Skip to content

Status code change in response from 4XX to 500 #2949

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
predic8 wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Status code change in response from 4XX to 500 #2949

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
aab6e3b
Improve error handling and status code consistency across interceptor…
predic8 May 18, 2026
8be0335
Remove redundant `final` modifier from static methods in tests
predic8 May 18, 2026
2f08ef5
Add request-response flow handling to correct status code logic
predic8 May 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions core/src/main/java/com/predic8/membrane/core/cli/RouterCLI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@
import java.security.SecureRandom;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.Scanner;
import java.util.stream.Collectors;

import static com.predic8.membrane.annot.Constants.*;
import static com.predic8.membrane.core.cli.util.JwkGenerator.generateJWK;
Expand Down
60 changes: 42 additions & 18 deletions core/src/main/java/com/predic8/membrane/core/exceptions/ProblemDetails.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,32 @@

package com.predic8.membrane.core.exceptions;

import com.fasterxml.jackson.core.*;
import com.fasterxml.jackson.databind.*;
import com.predic8.membrane.core.exchange.*;
import com.predic8.membrane.core.http.*;
import com.predic8.membrane.core.interceptor.*;
import org.jetbrains.annotations.*;
import org.slf4j.*;

import java.util.*;

import static com.predic8.membrane.core.exceptions.ProblemDetailsXML.*;
import static com.predic8.membrane.core.http.MimeType.*;
import static com.predic8.membrane.core.http.Response.*;
import static com.predic8.membrane.core.util.ExceptionUtil.*;
import static java.nio.charset.StandardCharsets.*;
import static java.util.Locale.*;
import static java.util.UUID.*;
import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.ObjectWriter;
import com.predic8.membrane.core.exchange.Exchange;
import com.predic8.membrane.core.http.Response;
import com.predic8.membrane.core.interceptor.Interceptor;
import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.MDC;

import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Set;

import static com.predic8.membrane.core.exceptions.ProblemDetailsXML.createXMLContent;
import static com.predic8.membrane.core.http.MimeType.APPLICATION_PROBLEM_JSON;
import static com.predic8.membrane.core.http.MimeType.TEXT_PLAIN_UTF8;
import static com.predic8.membrane.core.http.Response.statusCode;
import static com.predic8.membrane.core.interceptor.Interceptor.Flow.REQUEST;
import static com.predic8.membrane.core.interceptor.Interceptor.Flow.RESPONSE;
import static com.predic8.membrane.core.util.ExceptionUtil.concatMessageAndCauseMessages;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.Locale.ROOT;
import static java.util.UUID.randomUUID;


/**
Expand Down Expand Up @@ -344,7 +353,7 @@ private String getFullType(String type) {
}

private Response createContent(Map<String, Object> root, Exchange exchange) {
Response.ResponseBuilder builder = statusCode(status);
var builder = statusCode(correctStatusCodeForResponse(exchange,status));
try {
Comment on lines +356 to 357
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

HTTP status and problem payload status can diverge after remapping.

Line 354 applies corrected HTTP status, but createMap() already wrote the original status field. This can return HTTP 500 with body "status": 400, which is inconsistent for Problem Details consumers.

Proposed fix 
-    private Response createContent(Map<String, Object> root, Exchange exchange) {
-        var builder = statusCode(correctStatusCodeForResponse(exchange,status));
+    private Response createContent(Map<String, Object> root, Exchange exchange) {
+        var effectiveStatus = correctStatusCodeForResponse(exchange, status);
+        root.put(STATUS, effectiveStatus);
+        var builder = statusCode(effectiveStatus);
         try {
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/src/main/java/com/predic8/membrane/core/exceptions/ProblemDetails.java`
around lines 354 - 355, The problem is that createMap() is using the original
status while you call statusCode(correctStatusCodeForResponse(...)) afterward,
causing HTTP status and the "status" field to diverge; fix by computing the
corrected status first (call correctStatusCodeForResponse(exchange, status) once
into a local correctedStatus), use that correctedStatus when creating the
builder (statusCode(correctedStatus)) and ensure createMap() is invoked
with/after that correctedStatus (or accept correctedStatus as a parameter) so
the map's "status" field matches the HTTP response status; update references
around statusCode(...), correctStatusCodeForResponse(...), createMap(), and the
builder variable accordingly.

Copy link
Copy Markdown
Member Author

@predic8 predic8 May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

statusCode(..) sets the statusCode on the builder

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@predic8, you're correct that statusCode(correctStatusCodeForResponse(exchange, status)) properly sets the HTTP response line (e.g., HTTP/1.1 500). My concern is about a different thing — the "status" field inside the JSON problem-details body.

createMap() is called before createContent, and it writes:

root.put(STATUS, status);   // e.g. 400 — the *original* field value

Then createContent receives that already-populated map and sets the corrected HTTP status on the builder. The result is:

HTTP/1.1 500 Internal Server Error          ← builder: corrected to 500 ✅
Content-Type: application/problem+json

{
  "status": 400,                             ← body field: still original 400 ❌
  ...
}

Per RFC 9457 §3.1, the "status" member "is advisory; it conveys the HTTP status code used for the convenience of the consumer", so it should match the actual HTTP status. A Problem Details consumer relying on that field would see an inconsistency here.

The minimal fix is to update the "status" entry in root inside createContent after the remapping:

 private Response createContent(Map<String, Object> root, Exchange exchange) {
-    var builder = statusCode(correctStatusCodeForResponse(exchange, status));
+    var effectiveStatus = correctStatusCodeForResponse(exchange, status);
+    root.put(STATUS, effectiveStatus);   // keep body in sync with HTTP status
+    var builder = statusCode(effectiveStatus);
     try {

Would you like to address this, or should I open a follow-up issue for it?

🧠 Learnings used 
Learnt from: rrayst
Repo: membrane/api-gateway PR: 2863
File: core/src/main/java/com/predic8/membrane/core/interceptor/server/WSDLPublisherInterceptor.java:107-107
Timestamp: 2026-03-11T21:11:47.936Z
Learning: When reviewing refactors that extract logic from a synchronized block into a new method, verify that the synchronization guard (the GuardedBy target, e.g., the lock on paths) is carried into the extracted method and not dropped. Ensure shared fields annotated with GuardedBy(

if (exchange != null && (acceptXML(exchange) || exchange.getRequest().isXML())) {
createXMLContent(root, builder);
Expand All @@ -358,6 +367,21 @@ private Response createContent(Map<String, Object> root, Exchange exchange) {
return builder.build();
}

/**
* If a user error (4XX) occurs in the response flow, convert error code to 500.
*/
private int correctStatusCodeForResponse(Exchange exc, int status) {
if (flow == REQUEST)
return status;
if (flow == RESPONSE && status >= 400 && status < 500)
return 500;

// flow is not set. If there is already a response, it is probably the response flow.
if (exc != null && exc.getResponse() != null && status >= 400 && status < 500)
return 500;
Comment thread
predic8 marked this conversation as resolved.
return status;
}

private boolean acceptXML(Exchange exchange) {
String accept = exchange.getRequest().getHeader().getAccept();
if (accept == null)
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/com/predic8/membrane/core/interceptor/xml/Xml2JsonInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ private void handleException(Exchange exc, Flow flow, Exception e, String msg) {
log.info(msg, e);
log.debug("", e);
}
internal(router.getConfiguration().isProduction(), getDisplayName()).flow(flow).status(flow == REQUEST ? 400 : 500)
internal(router.getConfiguration().isProduction(), getDisplayName()).flow(flow).status(400)
.detail(msg)
.exception(e)
.topLevel("charset-from-header", exc.getMessage(flow).getHeader().getCharset())
Expand Down
47 changes: 26 additions & 21 deletions core/src/main/java/com/predic8/membrane/core/interceptor/xslt/XSLTInterceptor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,29 +13,33 @@
limitations under the License. */
package com.predic8.membrane.core.interceptor.xslt;

import com.predic8.membrane.annot.*;
import com.predic8.membrane.core.exceptions.*;
import com.predic8.membrane.core.exchange.*;
import com.predic8.membrane.core.http.*;
import com.predic8.membrane.core.interceptor.*;
import com.predic8.membrane.core.multipart.*;
import com.predic8.membrane.core.util.*;
import org.jetbrains.annotations.*;
import org.slf4j.*;

import javax.xml.transform.*;
import javax.xml.transform.stream.*;
import java.io.*;
import java.util.*;

import static com.predic8.membrane.core.exceptions.ProblemDetails.*;
import com.predic8.membrane.annot.MCAttribute;
import com.predic8.membrane.annot.MCElement;
import com.predic8.membrane.core.exchange.Exchange;
import com.predic8.membrane.core.http.Message;
import com.predic8.membrane.core.interceptor.AbstractInterceptor;
import com.predic8.membrane.core.interceptor.Outcome;
import com.predic8.membrane.core.multipart.XOPReconstitutor;
import com.predic8.membrane.core.util.ConfigurationException;
import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.xml.transform.TransformerException;
import javax.xml.transform.stream.StreamSource;
import java.util.Map;

import static com.predic8.membrane.core.exceptions.ProblemDetails.internal;
import static com.predic8.membrane.core.interceptor.Interceptor.Flow.*;
import static com.predic8.membrane.core.interceptor.Outcome.*;
import static com.predic8.membrane.core.exceptions.ProblemDetails.user;
import static com.predic8.membrane.core.interceptor.Interceptor.Flow.REQUEST;
import static com.predic8.membrane.core.interceptor.Interceptor.Flow.RESPONSE;
import static com.predic8.membrane.core.interceptor.Outcome.ABORT;
import static com.predic8.membrane.core.interceptor.Outcome.CONTINUE;
import static com.predic8.membrane.core.util.ExceptionUtil.getRootCause;
import static com.predic8.membrane.core.util.text.StringUtil.*;
import static com.predic8.membrane.core.util.text.TextUtil.*;
import static com.predic8.membrane.core.util.text.StringUtil.tail;
import static com.predic8.membrane.core.util.text.StringUtil.truncateAfter;
import static com.predic8.membrane.core.util.text.TextUtil.linkURL;
import static com.predic8.membrane.core.util.text.TextUtil.removeFinalChar;

/**
* @description <p>
Expand Down Expand Up @@ -75,7 +79,8 @@ private Outcome handleInternal(Exchange exc, Flow flow) {
} catch (TransformerException e) {
log.debug("", e);
var cause = getRootCause(e);
if (cause.getMessage() != null && cause.getMessage().contains("not allowed in prolog")) {
// rolog matches Prolog and prolog
if (cause.getMessage() != null && cause.getMessage().contains("rolog")) {
user(router.getConfiguration().isProduction(), getDisplayName())
.title("Content not allowed in prolog of XML input.")
.detail("Check for extra characters before the XML declaration <?xml ... ?>")
Expand Down
48 changes: 29 additions & 19 deletions core/src/main/java/com/predic8/membrane/core/lang/xpath/XPathExchangeExpression.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,23 +14,31 @@

package com.predic8.membrane.core.lang.xpath;

import com.predic8.membrane.core.config.xml.*;
import com.predic8.membrane.core.exchange.*;
import com.predic8.membrane.core.http.*;
import com.predic8.membrane.core.interceptor.*;
import com.predic8.membrane.core.lang.*;
import com.predic8.membrane.core.router.*;
import com.predic8.membrane.core.util.xml.*;
import com.predic8.membrane.core.util.xml.parser.*;
import org.jetbrains.annotations.*;
import org.slf4j.*;
import org.w3c.dom.*;

import javax.xml.namespace.*;
import javax.xml.xpath.*;

import static com.predic8.membrane.core.util.text.StringUtil.*;
import static javax.xml.xpath.XPathConstants.*;
import com.predic8.membrane.core.config.xml.XmlConfig;
import com.predic8.membrane.core.exchange.Exchange;
import com.predic8.membrane.core.http.Message;
import com.predic8.membrane.core.interceptor.Interceptor;
import com.predic8.membrane.core.interceptor.XMLSupport;
import com.predic8.membrane.core.lang.AbstractExchangeExpression;
import com.predic8.membrane.core.lang.ExchangeExpressionException;
import com.predic8.membrane.core.router.Router;
import com.predic8.membrane.core.util.xml.XMLUtil;
import com.predic8.membrane.core.util.xml.XPathUtil;
import com.predic8.membrane.core.util.xml.parser.HardenedXmlParser;
import com.predic8.membrane.core.util.xml.parser.XmlParser;
import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.w3c.dom.NodeList;

import javax.xml.namespace.QName;
import javax.xml.xpath.XPathConstants;
import javax.xml.xpath.XPathEvaluationResult;
import javax.xml.xpath.XPathExpressionException;

import static com.predic8.membrane.core.util.text.StringUtil.tail;
import static com.predic8.membrane.core.util.text.StringUtil.truncateAfter;
import static javax.xml.xpath.XPathConstants.NODESET;

public class XPathExchangeExpression extends AbstractExchangeExpression {

Expand Down Expand Up @@ -116,14 +124,16 @@ private Object evaluateAndCast(Message msg, QName xmlType) throws XPathExpressio
}
} catch (RuntimeException e) {
// Parser errors may escape as unchecked exceptions.
if (causeMessageContains(e, "not allowed in prolog")) {
// Matches: prolog and Prolog
if (causeMessageContains(e, "rolog")) {
throw new ExchangeExpressionException(expression, e, "Content not allowed in prolog of XML input.")
.detail("There are extra characters before the XML declaration <?xml ... ?>")
.body(truncateAfter(msg.getBodyAsStringDecoded(), 50))
.excludeException();
}

if (causeMessageContains(e, "is not allowed in trailing section")) {
// Matches: Content and content
if (causeMessageContains(e, "ontent")) {
throw new ExchangeExpressionException(expression, e, "Content not allowed in trailing section of XML input.")
.detail("There are extra characters after the XML root element (after the final closing tag like </root>).")
.body(tail(msg.getBodyAsStringDecoded(), 50))
Expand Down
43 changes: 30 additions & 13 deletions core/src/test/java/com/predic8/membrane/core/interceptor/xml/Xml2JsonInterceptorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,30 @@
package com.predic8.membrane.core.interceptor.xml;


import com.fasterxml.jackson.databind.*;
import com.predic8.membrane.core.exchange.*;
import com.predic8.membrane.core.http.*;
import com.predic8.membrane.core.interceptor.*;
import com.predic8.membrane.core.router.*;
import org.apache.commons.io.*;
import org.jetbrains.annotations.*;
import org.junit.jupiter.api.*;
import org.slf4j.*;

import java.io.*;
import java.nio.charset.*;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.predic8.membrane.core.exchange.Exchange;
import com.predic8.membrane.core.http.MimeType;
import com.predic8.membrane.core.http.Request;
import com.predic8.membrane.core.http.Response;
import com.predic8.membrane.core.interceptor.Outcome;
import com.predic8.membrane.core.router.DefaultRouter;
import org.apache.commons.io.IOUtils;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.URISyntaxException;
import java.nio.charset.Charset;

import static com.predic8.membrane.core.http.MimeType.*;
import static java.nio.charset.StandardCharsets.*;
import static com.predic8.membrane.core.http.Request.get;
import static java.nio.charset.StandardCharsets.ISO_8859_1;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.jupiter.api.Assertions.*;


Expand Down Expand Up @@ -95,6 +104,14 @@ void validTestxml2jsonResponse() throws Exception {
getJsonRootFromStream(processThroughInterceptorResponse(loadResource("/xml/content-utf-8-encoding-utf-8.xml"))).get("bar").get("futf").asText());
}

@Test
void statusCodeOfResponseError() throws URISyntaxException {
var exc = get("/foo").buildExchange();
exc.setResponse(Response.ok().contentType(TEXT_XML).body("<unclosed>").build());
interceptor.handleResponse(exc);
assertEquals(500, exc.getResponse().getStatusCode());
}

private JsonNode getJsonRootFromStream(InputStream stream) throws IOException {
return new ObjectMapper().readTree(stream);
}
Expand Down
26 changes: 12 additions & 14 deletions core/src/test/java/com/predic8/membrane/core/interceptor/xslt/XSLTInterceptorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,20 @@
limitations under the License. */
package com.predic8.membrane.core.interceptor.xslt;

import com.predic8.membrane.core.exchange.*;
import com.predic8.membrane.core.http.*;
import com.predic8.membrane.core.interceptor.*;
import com.predic8.membrane.core.router.*;
import org.hamcrest.*;
import org.junit.jupiter.api.*;
import org.xml.sax.*;

import javax.xml.xpath.*;
import java.io.*;
import java.net.*;
import com.predic8.membrane.core.exchange.Exchange;
import com.predic8.membrane.core.router.DummyTestRouter;
import org.junit.jupiter.api.Test;
import org.xml.sax.InputSource;

import javax.xml.xpath.XPath;
import javax.xml.xpath.XPathExpressionException;
import javax.xml.xpath.XPathFactory;

import static com.predic8.membrane.core.http.Request.get;
import static com.predic8.membrane.core.http.Response.*;
import static com.predic8.membrane.core.http.Response.ok;
import static com.predic8.membrane.core.interceptor.Outcome.ABORT;
import static org.junit.jupiter.api.Assertions.*;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class XSLTInterceptorTest {

Expand Down Expand Up @@ -83,7 +81,7 @@ void noContentInProlog() throws Exception {
i.init(new DummyTestRouter());
assertEquals(ABORT, i.handleRequest(exc));
assertEquals(400, exc.getResponse().getStatusCode());
String body = exc.getResponse().getBodyAsStringDecoded();
var body = exc.getResponse().getBodyAsStringDecoded();
assertTrue(body.contains("rubbish"));
assertTrue(body.contains("not allowed in prolog"));
}
Expand Down
Loading
Loading