Skip to content

Commit 2f20108

Browse files
predic8t-burch
predic8
and
t-burch
authored
Status code change in response from 4XX to 500 (#2949)
* Improve error handling and status code consistency across interceptors and tests * Remove redundant `final` modifier from static methods in tests * Add request-response flow handling to correct status code logic * Sync HTTP status code with ProblemDetails body Extract effective status code to ensure consistency between HTTP response status and the `status` field in the ProblemDetails body. --------- Co-authored-by: t-burch <119930761+t-burch@users.noreply.github.com>
1 parent c5aff4a commit 2f20108

9 files changed

Lines changed: 193 additions & 128 deletions

File tree

‎core/src/main/java/com/predic8/membrane/core/cli/RouterCLI.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,7 @@
4747
import java.security.SecureRandom;
4848
import java.util.Arrays;
4949
import java.util.List;
50-
import java.util.Optional;
5150
import java.util.Scanner;
52-
import java.util.stream.Collectors;
5351

5452
import static com.predic8.membrane.annot.Constants.*;
5553
import static com.predic8.membrane.core.cli.util.JwkGenerator.generateJWK;

‎core/src/main/java/com/predic8/membrane/core/exceptions/ProblemDetails.java‎

Lines changed: 44 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,23 +11,32 @@
1111

1212
package com.predic8.membrane.core.exceptions;
1313

14-
import com.fasterxml.jackson.core.*;
15-
import com.fasterxml.jackson.databind.*;
16-
import com.predic8.membrane.core.exchange.*;
17-
import com.predic8.membrane.core.http.*;
18-
import com.predic8.membrane.core.interceptor.*;
19-
import org.jetbrains.annotations.*;
20-
import org.slf4j.*;
21-
22-
import java.util.*;
23-
24-
import static com.predic8.membrane.core.exceptions.ProblemDetailsXML.*;
25-
import static com.predic8.membrane.core.http.MimeType.*;
26-
import static com.predic8.membrane.core.http.Response.*;
27-
import static com.predic8.membrane.core.util.ExceptionUtil.*;
28-
import static java.nio.charset.StandardCharsets.*;
29-
import static java.util.Locale.*;
30-
import static java.util.UUID.*;
14+
import com.fasterxml.jackson.core.JsonProcessingException;
15+
import com.fasterxml.jackson.databind.ObjectMapper;
16+
import com.fasterxml.jackson.databind.ObjectWriter;
17+
import com.predic8.membrane.core.exchange.Exchange;
18+
import com.predic8.membrane.core.http.Response;
19+
import com.predic8.membrane.core.interceptor.Interceptor;
20+
import org.jetbrains.annotations.NotNull;
21+
import org.slf4j.Logger;
22+
import org.slf4j.LoggerFactory;
23+
import org.slf4j.MDC;
24+
25+
import java.util.HashMap;
26+
import java.util.LinkedHashMap;
27+
import java.util.Map;
28+
import java.util.Set;
29+
30+
import static com.predic8.membrane.core.exceptions.ProblemDetailsXML.createXMLContent;
31+
import static com.predic8.membrane.core.http.MimeType.APPLICATION_PROBLEM_JSON;
32+
import static com.predic8.membrane.core.http.MimeType.TEXT_PLAIN_UTF8;
33+
import static com.predic8.membrane.core.http.Response.statusCode;
34+
import static com.predic8.membrane.core.interceptor.Interceptor.Flow.REQUEST;
35+
import static com.predic8.membrane.core.interceptor.Interceptor.Flow.RESPONSE;
36+
import static com.predic8.membrane.core.util.ExceptionUtil.concatMessageAndCauseMessages;
37+
import static java.nio.charset.StandardCharsets.UTF_8;
38+
import static java.util.Locale.ROOT;
39+
import static java.util.UUID.randomUUID;
3140

3241

3342
/**
@@ -344,7 +353,9 @@ private String getFullType(String type) {
344353
}
345354

346355
private Response createContent(Map<String, Object> root, Exchange exchange) {
347-
Response.ResponseBuilder builder = statusCode(status);
356+
var effectiveStatus = correctStatusCodeForResponse(exchange, status);
357+
root.put(STATUS, effectiveStatus); // keep body in sync with HTTP status
358+
var builder = statusCode(effectiveStatus);
348359
try {
349360
if (exchange != null && (acceptXML(exchange) || exchange.getRequest().isXML())) {
350361
createXMLContent(root, builder);
@@ -358,6 +369,21 @@ private Response createContent(Map<String, Object> root, Exchange exchange) {
358369
return builder.build();
359370
}
360371

372+
/**
373+
* If a user error (4XX) occurs in the response flow, convert error code to 500.
374+
*/
375+
private int correctStatusCodeForResponse(Exchange exc, int status) {
376+
if (flow == REQUEST)
377+
return status;
378+
if (flow == RESPONSE && status >= 400 && status < 500)
379+
return 500;
380+
381+
// flow is not set. If there is already a response, it is probably the response flow.
382+
if (exc != null && exc.getResponse() != null && status >= 400 && status < 500)
383+
return 500;
384+
return status;
385+
}
386+
361387
private boolean acceptXML(Exchange exchange) {
362388
String accept = exchange.getRequest().getHeader().getAccept();
363389
if (accept == null)

‎core/src/main/java/com/predic8/membrane/core/interceptor/xml/Xml2JsonInterceptor.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ private void handleException(Exchange exc, Flow flow, Exception e, String msg) {
146146
log.info(msg, e);
147147
log.debug("", e);
148148
}
149-
internal(router.getConfiguration().isProduction(), getDisplayName()).flow(flow).status(flow == REQUEST ? 400 : 500)
149+
internal(router.getConfiguration().isProduction(), getDisplayName()).flow(flow).status(400)
150150
.detail(msg)
151151
.exception(e)
152152
.topLevel("charset-from-header", exc.getMessage(flow).getHeader().getCharset())

‎core/src/main/java/com/predic8/membrane/core/interceptor/xslt/XSLTInterceptor.java‎

Lines changed: 26 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -13,29 +13,33 @@
1313
limitations under the License. */
1414
package com.predic8.membrane.core.interceptor.xslt;
1515

16-
import com.predic8.membrane.annot.*;
17-
import com.predic8.membrane.core.exceptions.*;
18-
import com.predic8.membrane.core.exchange.*;
19-
import com.predic8.membrane.core.http.*;
20-
import com.predic8.membrane.core.interceptor.*;
21-
import com.predic8.membrane.core.multipart.*;
22-
import com.predic8.membrane.core.util.*;
23-
import org.jetbrains.annotations.*;
24-
import org.slf4j.*;
25-
26-
import javax.xml.transform.*;
27-
import javax.xml.transform.stream.*;
28-
import java.io.*;
29-
import java.util.*;
30-
31-
import static com.predic8.membrane.core.exceptions.ProblemDetails.*;
16+
import com.predic8.membrane.annot.MCAttribute;
17+
import com.predic8.membrane.annot.MCElement;
18+
import com.predic8.membrane.core.exchange.Exchange;
19+
import com.predic8.membrane.core.http.Message;
20+
import com.predic8.membrane.core.interceptor.AbstractInterceptor;
21+
import com.predic8.membrane.core.interceptor.Outcome;
22+
import com.predic8.membrane.core.multipart.XOPReconstitutor;
23+
import com.predic8.membrane.core.util.ConfigurationException;
24+
import org.jetbrains.annotations.NotNull;
25+
import org.slf4j.Logger;
26+
import org.slf4j.LoggerFactory;
27+
28+
import javax.xml.transform.TransformerException;
29+
import javax.xml.transform.stream.StreamSource;
30+
import java.util.Map;
31+
3232
import static com.predic8.membrane.core.exceptions.ProblemDetails.internal;
33-
import static com.predic8.membrane.core.interceptor.Interceptor.Flow.*;
34-
import static com.predic8.membrane.core.interceptor.Outcome.*;
33+
import static com.predic8.membrane.core.exceptions.ProblemDetails.user;
34+
import static com.predic8.membrane.core.interceptor.Interceptor.Flow.REQUEST;
35+
import static com.predic8.membrane.core.interceptor.Interceptor.Flow.RESPONSE;
3536
import static com.predic8.membrane.core.interceptor.Outcome.ABORT;
37+
import static com.predic8.membrane.core.interceptor.Outcome.CONTINUE;
3638
import static com.predic8.membrane.core.util.ExceptionUtil.getRootCause;
37-
import static com.predic8.membrane.core.util.text.StringUtil.*;
38-
import static com.predic8.membrane.core.util.text.TextUtil.*;
39+
import static com.predic8.membrane.core.util.text.StringUtil.tail;
40+
import static com.predic8.membrane.core.util.text.StringUtil.truncateAfter;
41+
import static com.predic8.membrane.core.util.text.TextUtil.linkURL;
42+
import static com.predic8.membrane.core.util.text.TextUtil.removeFinalChar;
3943

4044
/**
4145
* @description <p>
@@ -75,7 +79,8 @@ private Outcome handleInternal(Exchange exc, Flow flow) {
7579
} catch (TransformerException e) {
7680
log.debug("", e);
7781
var cause = getRootCause(e);
78-
if (cause.getMessage() != null && cause.getMessage().contains("not allowed in prolog")) {
82+
// rolog matches Prolog and prolog
83+
if (cause.getMessage() != null && cause.getMessage().contains("rolog")) {
7984
user(router.getConfiguration().isProduction(), getDisplayName())
8085
.title("Content not allowed in prolog of XML input.")
8186
.detail("Check for extra characters before the XML declaration <?xml ... ?>")

‎core/src/main/java/com/predic8/membrane/core/lang/xpath/XPathExchangeExpression.java‎

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,23 +14,31 @@
1414

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

17-
import com.predic8.membrane.core.config.xml.*;
18-
import com.predic8.membrane.core.exchange.*;
19-
import com.predic8.membrane.core.http.*;
20-
import com.predic8.membrane.core.interceptor.*;
21-
import com.predic8.membrane.core.lang.*;
22-
import com.predic8.membrane.core.router.*;
23-
import com.predic8.membrane.core.util.xml.*;
24-
import com.predic8.membrane.core.util.xml.parser.*;
25-
import org.jetbrains.annotations.*;
26-
import org.slf4j.*;
27-
import org.w3c.dom.*;
28-
29-
import javax.xml.namespace.*;
30-
import javax.xml.xpath.*;
31-
32-
import static com.predic8.membrane.core.util.text.StringUtil.*;
33-
import static javax.xml.xpath.XPathConstants.*;
17+
import com.predic8.membrane.core.config.xml.XmlConfig;
18+
import com.predic8.membrane.core.exchange.Exchange;
19+
import com.predic8.membrane.core.http.Message;
20+
import com.predic8.membrane.core.interceptor.Interceptor;
21+
import com.predic8.membrane.core.interceptor.XMLSupport;
22+
import com.predic8.membrane.core.lang.AbstractExchangeExpression;
23+
import com.predic8.membrane.core.lang.ExchangeExpressionException;
24+
import com.predic8.membrane.core.router.Router;
25+
import com.predic8.membrane.core.util.xml.XMLUtil;
26+
import com.predic8.membrane.core.util.xml.XPathUtil;
27+
import com.predic8.membrane.core.util.xml.parser.HardenedXmlParser;
28+
import com.predic8.membrane.core.util.xml.parser.XmlParser;
29+
import org.jetbrains.annotations.NotNull;
30+
import org.slf4j.Logger;
31+
import org.slf4j.LoggerFactory;
32+
import org.w3c.dom.NodeList;
33+
34+
import javax.xml.namespace.QName;
35+
import javax.xml.xpath.XPathConstants;
36+
import javax.xml.xpath.XPathEvaluationResult;
37+
import javax.xml.xpath.XPathExpressionException;
38+
39+
import static com.predic8.membrane.core.util.text.StringUtil.tail;
40+
import static com.predic8.membrane.core.util.text.StringUtil.truncateAfter;
41+
import static javax.xml.xpath.XPathConstants.NODESET;
3442

3543
public class XPathExchangeExpression extends AbstractExchangeExpression {
3644

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

126-
if (causeMessageContains(e, "is not allowed in trailing section")) {
135+
// Matches: Content and content
136+
if (causeMessageContains(e, "ontent")) {
127137
throw new ExchangeExpressionException(expression, e, "Content not allowed in trailing section of XML input.")
128138
.detail("There are extra characters after the XML root element (after the final closing tag like </root>).")
129139
.body(tail(msg.getBodyAsStringDecoded(), 50))

‎core/src/test/java/com/predic8/membrane/core/interceptor/xml/Xml2JsonInterceptorTest.java‎

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,21 +14,30 @@
1414
package com.predic8.membrane.core.interceptor.xml;
1515

1616

17-
import com.fasterxml.jackson.databind.*;
18-
import com.predic8.membrane.core.exchange.*;
19-
import com.predic8.membrane.core.http.*;
20-
import com.predic8.membrane.core.interceptor.*;
21-
import com.predic8.membrane.core.router.*;
22-
import org.apache.commons.io.*;
23-
import org.jetbrains.annotations.*;
24-
import org.junit.jupiter.api.*;
25-
import org.slf4j.*;
26-
27-
import java.io.*;
28-
import java.nio.charset.*;
17+
import com.fasterxml.jackson.databind.JsonNode;
18+
import com.fasterxml.jackson.databind.ObjectMapper;
19+
import com.predic8.membrane.core.exchange.Exchange;
20+
import com.predic8.membrane.core.http.MimeType;
21+
import com.predic8.membrane.core.http.Request;
22+
import com.predic8.membrane.core.http.Response;
23+
import com.predic8.membrane.core.interceptor.Outcome;
24+
import com.predic8.membrane.core.router.DefaultRouter;
25+
import org.apache.commons.io.IOUtils;
26+
import org.junit.jupiter.api.BeforeAll;
27+
import org.junit.jupiter.api.Test;
28+
import org.slf4j.Logger;
29+
import org.slf4j.LoggerFactory;
30+
31+
import java.io.ByteArrayInputStream;
32+
import java.io.IOException;
33+
import java.io.InputStream;
34+
import java.net.URISyntaxException;
35+
import java.nio.charset.Charset;
2936

3037
import static com.predic8.membrane.core.http.MimeType.*;
31-
import static java.nio.charset.StandardCharsets.*;
38+
import static com.predic8.membrane.core.http.Request.get;
39+
import static java.nio.charset.StandardCharsets.ISO_8859_1;
40+
import static java.nio.charset.StandardCharsets.UTF_8;
3241
import static org.junit.jupiter.api.Assertions.*;
3342

3443

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

107+
@Test
108+
void statusCodeOfResponseError() throws URISyntaxException {
109+
var exc = get("/foo").buildExchange();
110+
exc.setResponse(Response.ok().contentType(TEXT_XML).body("<unclosed>").build());
111+
interceptor.handleResponse(exc);
112+
assertEquals(500, exc.getResponse().getStatusCode());
113+
}
114+
98115
private JsonNode getJsonRootFromStream(InputStream stream) throws IOException {
99116
return new ObjectMapper().readTree(stream);
100117
}

‎core/src/test/java/com/predic8/membrane/core/interceptor/xslt/XSLTInterceptorTest.java‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,20 @@
1313
limitations under the License. */
1414
package com.predic8.membrane.core.interceptor.xslt;
1515

16-
import com.predic8.membrane.core.exchange.*;
17-
import com.predic8.membrane.core.http.*;
18-
import com.predic8.membrane.core.interceptor.*;
19-
import com.predic8.membrane.core.router.*;
20-
import org.hamcrest.*;
21-
import org.junit.jupiter.api.*;
22-
import org.xml.sax.*;
23-
24-
import javax.xml.xpath.*;
25-
import java.io.*;
26-
import java.net.*;
16+
import com.predic8.membrane.core.exchange.Exchange;
17+
import com.predic8.membrane.core.router.DummyTestRouter;
18+
import org.junit.jupiter.api.Test;
19+
import org.xml.sax.InputSource;
20+
21+
import javax.xml.xpath.XPath;
22+
import javax.xml.xpath.XPathExpressionException;
23+
import javax.xml.xpath.XPathFactory;
2724

2825
import static com.predic8.membrane.core.http.Request.get;
29-
import static com.predic8.membrane.core.http.Response.*;
26+
import static com.predic8.membrane.core.http.Response.ok;
3027
import static com.predic8.membrane.core.interceptor.Outcome.ABORT;
31-
import static org.junit.jupiter.api.Assertions.*;
28+
import static org.junit.jupiter.api.Assertions.assertEquals;
29+
import static org.junit.jupiter.api.Assertions.assertTrue;
3230

3331
public class XSLTInterceptorTest {
3432

@@ -83,7 +81,7 @@ void noContentInProlog() throws Exception {
8381
i.init(new DummyTestRouter());
8482
assertEquals(ABORT, i.handleRequest(exc));
8583
assertEquals(400, exc.getResponse().getStatusCode());
86-
String body = exc.getResponse().getBodyAsStringDecoded();
84+
var body = exc.getResponse().getBodyAsStringDecoded();
8785
assertTrue(body.contains("rubbish"));
8886
assertTrue(body.contains("not allowed in prolog"));
8987
}

0 commit comments

Comments
 (0)