Copilot commented on code in PR #8035:
URL: https://github.com/apache/incubator-seata/pull/8035#discussion_r3025740314
##########
namingserver/src/main/java/org/apache/seata/namingserver/filter/ConsoleRemotingFilter.java:
##########
@@ -108,33 +153,89 @@ public void doFilter(ServletRequest servletRequest,
ServletResponse servletRespo
.forEach(headerName ->
headers.add(headerName, request.getHeader(headerName)));
Review Comment:
Request headers are copied wholesale into the proxied request, but only
`Content-Length`/`Transfer-Encoding` are removed for GET/HEAD (and empty
bodies). Hop-by-hop headers (e.g., `Connection`, `Keep-Alive`,
`Proxy-Authorization`, `TE`, `Trailer`, `Upgrade`) should not be forwarded on
requests either, and forwarding them can cause incorrect proxy behavior and
header injection issues. Consider filtering these headers out from `headers`
before building `HttpEntity` (and also consider excluding `Host` so the client
library sets it consistently).
##########
namingserver/src/main/java/org/apache/seata/namingserver/filter/ConsoleRemotingFilter.java:
##########
@@ -57,13 +58,57 @@ public class ConsoleRemotingFilter implements Filter {
private final Pattern urlPattern = Pattern.compile(CONSOLE_PATTERN);
- private final Logger logger =
LoggerFactory.getLogger(ConsoleRemotingFilter.class);
+ private static final Logger LOGGER =
LoggerFactory.getLogger(ConsoleRemotingFilter.class);
public ConsoleRemotingFilter(NamingManager namingManager, RestTemplate
restTemplate) {
this.namingManager = namingManager;
this.restTemplate = restTemplate;
}
+ /**
+ * Check whether the proxied Content-Type is safe (will not be rendered as
+ * HTML / XML by the browser). Only allow known-safe MIME types through
+ * (allowlist approach); everything else is replaced with
+ * {@code application/json}.
+ */
+ private static boolean isSafeContentType(String contentType) {
+ if (contentType == null) {
+ return false;
+ }
+ String lower = contentType.toLowerCase(Locale.ROOT);
+ // Extract the primary MIME type (ignore parameters such as charset)
+ int semicolonIdx = lower.indexOf(';');
+ String mimeType = (semicolonIdx >= 0 ? lower.substring(0,
semicolonIdx) : lower).trim();
+ return "application/json".equals(mimeType)
+ || "text/plain".equals(mimeType)
+ || "application/octet-stream".equals(mimeType);
+ }
+
+ /**
+ * Validate that the given byte array looks like well-formed JSON
+ * (starts with '{' or '[' after trimming leading whitespace).
+ * This is a lightweight sanity check to prevent forwarding
+ * arbitrary HTML / script payloads disguised as JSON.
+ */
+ private static boolean looksLikeJson(byte[] body) {
+ if (body == null || body.length == 0) {
+ return true;
+ }
+ int i = 0;
+ // skip leading whitespace
+ while (i < body.length && (body[i] == ' ' || body[i] == '\t'
+ || body[i] == '\r' || body[i] == '\n')) {
+ i++;
+ }
+ if (i >= body.length) {
+ return true;
+ }
+ byte first = body[i];
+ return first == '{' || first == '[' || first == '"'
+ || first == 't' || first == 'f' || first == 'n'
+ || (first >= '0' && first <= '9') || first == '-';
+ }
Review Comment:
The `looksLikeJson` check can false-negative valid JSON that starts with a
UTF-8 BOM (0xEF,0xBB,0xBF) or other leading whitespace characters beyond the
four handled. That would incorrectly convert legitimate upstream responses into
502. Consider explicitly skipping an optional UTF-8 BOM and expanding the
whitespace skip (or using a more robust JSON detection strategy that doesn’t
reject valid payloads).
##########
namingserver/src/main/java/org/apache/seata/namingserver/filter/ConsoleRemotingFilter.java:
##########
@@ -108,33 +153,89 @@ public void doFilter(ServletRequest servletRequest,
ServletResponse servletRespo
.forEach(headerName ->
headers.add(headerName, request.getHeader(headerName)));
// Create the HttpEntity with headers and body
- HttpEntity<byte[]> httpEntity = new
HttpEntity<>(request.getCachedBody(), headers);
HttpMethod httpMethod;
try {
httpMethod =
HttpMethod.valueOf(request.getMethod());
} catch (IllegalArgumentException ex) {
- logger.error("Unsupported HTTP method: {}",
request.getMethod(), ex);
+ LOGGER.error("Unsupported HTTP method: {}",
request.getMethod(), ex);
response.setStatus(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
return;
}
+
+ // GET/HEAD methods should not have a body; other
methods may include a body as needed.
+ HttpEntity<byte[]> httpEntity;
+ if (HttpMethod.GET.equals(httpMethod) ||
HttpMethod.HEAD.equals(httpMethod)) {
+ headers.remove(HttpHeaders.CONTENT_LENGTH);
+ headers.remove(HttpHeaders.TRANSFER_ENCODING);
+ // headers-only
+ httpEntity = new HttpEntity<>(headers);
Review Comment:
Request headers are copied wholesale into the proxied request, but only
`Content-Length`/`Transfer-Encoding` are removed for GET/HEAD (and empty
bodies). Hop-by-hop headers (e.g., `Connection`, `Keep-Alive`,
`Proxy-Authorization`, `TE`, `Trailer`, `Upgrade`) should not be forwarded on
requests either, and forwarding them can cause incorrect proxy behavior and
header injection issues. Consider filtering these headers out from `headers`
before building `HttpEntity` (and also consider excluding `Host` so the client
library sets it consistently).
##########
namingserver/src/test/java/org/apache/seata/namingserver/filter/ConsoleRemotingFilterTest.java:
##########
@@ -0,0 +1,242 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.seata.namingserver.filter;
+
+import jakarta.servlet.FilterChain;
+import org.apache.seata.common.metadata.Node;
+import org.apache.seata.common.metadata.namingserver.NamingServerNode;
+import org.apache.seata.namingserver.manager.NamingManager;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
+import org.springframework.http.HttpEntity;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpMethod;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.mock.web.MockHttpServletResponse;
+import org.springframework.web.client.RestTemplate;
+
+import java.net.URI;
+import java.nio.charset.StandardCharsets;
+import java.util.Collections;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+/**
+ * Unit tests for {@link ConsoleRemotingFilter}.
+ * <p>
+ * Covers the GET/HEAD body-stripping regression (OkHttp
IllegalArgumentException)
+ * and other proxy-forwarding behavior.
+ */
+class ConsoleRemotingFilterTest {
+
+ private NamingManager namingManager;
+ private RestTemplate restTemplate;
+ private ConsoleRemotingFilter filter;
+ private FilterChain filterChain;
+
+ private static final String NAMESPACE = "public";
+ private static final String CLUSTER = "default";
+ private static final String TARGET_HOST = "127.0.0.1";
+ private static final int TARGET_PORT = 7091;
+
+ @BeforeEach
+ void setUp() {
+ namingManager = mock(NamingManager.class);
+ restTemplate = mock(RestTemplate.class);
+ filterChain = mock(FilterChain.class);
+ filter = new ConsoleRemotingFilter(namingManager, restTemplate);
+
+ // Set up a NamingServerNode with a control endpoint
+ NamingServerNode node = new NamingServerNode();
+ node.setControl(new Node.Endpoint(TARGET_HOST, TARGET_PORT, "http"));
+
+ when(namingManager.getInstances(NAMESPACE, CLUSTER))
+ .thenReturn(Collections.singletonList(node));
+ }
+
+ /**
+ * Regression test: a GET request with a non-empty body should NOT forward
+ * the body to the upstream server (to avoid OkHttp's
IllegalArgumentException).
+ * The body, Content-Length, and Transfer-Encoding headers must be
stripped.
+ */
+ @Test
+ void getRequestWithBodyShouldStripBody() throws Exception {
+ // Prepare a GET request with a body (some clients/frameworks may
attach one)
+ MockHttpServletRequest request = createConsoleRequest("GET");
+
request.setContent("{\"key\":\"value\"}".getBytes(StandardCharsets.UTF_8));
+ request.addHeader(HttpHeaders.CONTENT_LENGTH, "15");
+
+ MockHttpServletResponse response = new MockHttpServletResponse();
+
+ // Stub RestTemplate to return a successful JSON response
+ HttpHeaders responseHeaders = new HttpHeaders();
+ responseHeaders.set(HttpHeaders.CONTENT_TYPE,
"application/json;charset=UTF-8");
+ ResponseEntity<byte[]> upstreamResponse = new ResponseEntity<>(
+ "{\"result\":\"ok\"}".getBytes(StandardCharsets.UTF_8),
+ responseHeaders,
+ HttpStatus.OK);
+
+ when(restTemplate.exchange(any(URI.class), eq(HttpMethod.GET),
any(HttpEntity.class), eq(byte[].class)))
+ .thenReturn(upstreamResponse);
+
+ filter.doFilter(request, response, filterChain);
+
+ // Capture the HttpEntity sent to RestTemplate
+ @SuppressWarnings("unchecked")
+ ArgumentCaptor<HttpEntity<byte[]>> entityCaptor =
ArgumentCaptor.forClass(HttpEntity.class);
+ verify(restTemplate).exchange(any(URI.class), eq(HttpMethod.GET),
entityCaptor.capture(), eq(byte[].class));
+
+ HttpEntity<byte[]> capturedEntity = entityCaptor.getValue();
+ // Body must be null (stripped for GET)
+ assertNull(capturedEntity.getBody(), "GET request body should be
stripped (null)");
+ // Content-Length and Transfer-Encoding headers must not be forwarded
+ assertNull(capturedEntity.getHeaders().get(HttpHeaders.CONTENT_LENGTH),
+ "Content-Length header should be removed for GET");
+
assertNull(capturedEntity.getHeaders().get(HttpHeaders.TRANSFER_ENCODING),
+ "Transfer-Encoding header should be removed for GET");
+
+ // Verify filterChain was NOT invoked (proxied)
+ verify(filterChain, never()).doFilter(any(), any());
+ assertEquals(200, response.getStatus());
+ }
+
+ /**
+ * HEAD request should also strip the body, same as GET.
+ */
+ @Test
+ void headRequestShouldStripBody() throws Exception {
+ MockHttpServletRequest request = createConsoleRequest("HEAD");
+ request.setContent("some body".getBytes(StandardCharsets.UTF_8));
+
+ MockHttpServletResponse response = new MockHttpServletResponse();
+
+ HttpHeaders responseHeaders = new HttpHeaders();
+ responseHeaders.set(HttpHeaders.CONTENT_TYPE, "application/json");
+ ResponseEntity<byte[]> upstreamResponse = new ResponseEntity<>(
+ null, responseHeaders, HttpStatus.OK);
+
+ when(restTemplate.exchange(any(URI.class), eq(HttpMethod.HEAD),
any(HttpEntity.class), eq(byte[].class)))
+ .thenReturn(upstreamResponse);
+
+ filter.doFilter(request, response, filterChain);
+
+ @SuppressWarnings("unchecked")
+ ArgumentCaptor<HttpEntity<byte[]>> entityCaptor =
ArgumentCaptor.forClass(HttpEntity.class);
+ verify(restTemplate).exchange(any(URI.class), eq(HttpMethod.HEAD),
entityCaptor.capture(), eq(byte[].class));
+
+ assertNull(entityCaptor.getValue().getBody(), "HEAD request body
should be stripped (null)");
+ verify(filterChain, never()).doFilter(any(), any());
+ }
+
+ /**
+ * POST request should forward the body as-is.
+ */
+ @Test
+ void postRequestShouldForwardBody() throws Exception {
+ byte[] bodyBytes =
"{\"data\":\"test\"}".getBytes(StandardCharsets.UTF_8);
+ MockHttpServletRequest request = createConsoleRequest("POST");
+ request.setContent(bodyBytes);
+
+ MockHttpServletResponse response = new MockHttpServletResponse();
+
+ HttpHeaders responseHeaders = new HttpHeaders();
+ responseHeaders.set(HttpHeaders.CONTENT_TYPE,
"application/json;charset=UTF-8");
+ ResponseEntity<byte[]> upstreamResponse = new ResponseEntity<>(
+ "{\"result\":\"created\"}".getBytes(StandardCharsets.UTF_8),
+ responseHeaders,
+ HttpStatus.OK);
+
+ when(restTemplate.exchange(any(URI.class), eq(HttpMethod.POST),
any(HttpEntity.class), eq(byte[].class)))
+ .thenReturn(upstreamResponse);
+
+ filter.doFilter(request, response, filterChain);
+
+ @SuppressWarnings("unchecked")
+ ArgumentCaptor<HttpEntity<byte[]>> entityCaptor =
ArgumentCaptor.forClass(HttpEntity.class);
+ verify(restTemplate).exchange(any(URI.class), eq(HttpMethod.POST),
entityCaptor.capture(), eq(byte[].class));
+
+ byte[] capturedBody = entityCaptor.getValue().getBody();
+ assert capturedBody != null : "POST body should not be null";
+ assertEquals(new String(bodyBytes, StandardCharsets.UTF_8),
+ new String(capturedBody, StandardCharsets.UTF_8),
+ "POST request body should be forwarded as-is");
Review Comment:
This test uses a Java `assert` statement, which is disabled by default
unless the JVM is run with `-ea`, so the null check may be silently skipped in
CI. Use a JUnit assertion (e.g., `assertNotNull`) so the test reliably fails
when `capturedBody` is null.
##########
namingserver/src/main/java/org/apache/seata/namingserver/filter/ConsoleRemotingFilter.java:
##########
@@ -108,33 +153,89 @@ public void doFilter(ServletRequest servletRequest,
ServletResponse servletRespo
.forEach(headerName ->
headers.add(headerName, request.getHeader(headerName)));
// Create the HttpEntity with headers and body
- HttpEntity<byte[]> httpEntity = new
HttpEntity<>(request.getCachedBody(), headers);
HttpMethod httpMethod;
try {
httpMethod =
HttpMethod.valueOf(request.getMethod());
} catch (IllegalArgumentException ex) {
- logger.error("Unsupported HTTP method: {}",
request.getMethod(), ex);
+ LOGGER.error("Unsupported HTTP method: {}",
request.getMethod(), ex);
response.setStatus(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
return;
}
+
+ // GET/HEAD methods should not have a body; other
methods may include a body as needed.
+ HttpEntity<byte[]> httpEntity;
+ if (HttpMethod.GET.equals(httpMethod) ||
HttpMethod.HEAD.equals(httpMethod)) {
+ headers.remove(HttpHeaders.CONTENT_LENGTH);
+ headers.remove(HttpHeaders.TRANSFER_ENCODING);
+ // headers-only
+ httpEntity = new HttpEntity<>(headers);
+ } else {
+ byte[] body = request.getCachedBody();
+ if (body == null || body.length == 0) {
+ headers.remove(HttpHeaders.CONTENT_LENGTH);
+
headers.remove(HttpHeaders.TRANSFER_ENCODING);
+ // headers-only for empty body
+ httpEntity = new HttpEntity<>(headers);
+ } else {
+ httpEntity = new HttpEntity<>(body,
headers);
+ }
+ }
+
try {
- ResponseEntity<byte[]> responseEntity =
restTemplate.exchange(
- URI.create(targetUrl), httpMethod,
httpEntity, byte[].class);
+ ResponseEntity<byte[]> responseEntity =
restTemplate.exchange(URI.create(targetUrl), httpMethod, httpEntity,
byte[].class);
+ //Copy headers from proxied response, skipping
hop-by-hop and headers we manage ourselves to mitigate
+ // security risks from Content-Type
manipulation
responseEntity.getHeaders().forEach((key,
value) -> {
- value.forEach(v -> response.addHeader(key,
v));
+ if
(!HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(key)
+ &&
!HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(key)
+ &&
!HttpHeaders.TRANSFER_ENCODING.equalsIgnoreCase(key)
+ &&
!"X-Content-Type-Options".equalsIgnoreCase(key)
+ &&
!HttpHeaders.CONNECTION.equalsIgnoreCase(key)
+ &&
!"Keep-Alive".equalsIgnoreCase(key)
+ &&
!HttpHeaders.PROXY_AUTHENTICATE.equalsIgnoreCase(key)
+ &&
!HttpHeaders.PROXY_AUTHORIZATION.equalsIgnoreCase(key)
+ &&
!HttpHeaders.TE.equalsIgnoreCase(key)
+ &&
!HttpHeaders.TRAILER.equalsIgnoreCase(key)
+ &&
!HttpHeaders.UPGRADE.equalsIgnoreCase(key)) {
+ value.forEach(v ->
response.addHeader(key, v));
+ }
});
- response.setStatus(
-
responseEntity.getStatusCode().value());
-
Optional.ofNullable(responseEntity.getBody()).ifPresent(body -> {
+ // Force a safe Content-Type: reject HTML/XML
types that could
+ // execute scripts; fall back to
application/json
+ String proxiedContentType =
responseEntity.getHeaders().getFirst(HttpHeaders.CONTENT_TYPE);
+ String safeContentType;
+ if (proxiedContentType != null &&
isSafeContentType(proxiedContentType)) {
+ safeContentType = proxiedContentType;
+ } else {
+ safeContentType =
"application/json;charset=UTF-8";
+ }
+ response.setContentType(safeContentType);
+ response.setHeader("X-Content-Type-Options",
"nosniff");
+
response.setStatus(responseEntity.getStatusCode().value());
+ byte[] responseBody = responseEntity.getBody();
+ if (responseBody != null &&
responseBody.length > 0) {
+ // For JSON content type, validate that
the body actually looks
+ // like JSON to prevent XSS via crafted
upstream responses
+ if
(safeContentType.toLowerCase(Locale.ROOT).contains("application/json")
+ && !looksLikeJson(responseBody)) {
+ LOGGER.warn("Upstream returned
non-JSON body for Content-Type {}, replacing with error response",
safeContentType);
+
response.setStatus(HttpServletResponse.SC_BAD_GATEWAY);
+
response.setContentType("application/json;charset=UTF-8");
+ responseBody = "{\"error\":\"Upstream
returned invalid response body\"}"
+
.getBytes(StandardCharsets.UTF_8);
+ }
try (ServletOutputStream outputStream =
response.getOutputStream()) {
- outputStream.write(body);
+ outputStream.write(responseBody);
outputStream.flush();
} catch (IOException e) {
- logger.error(e.getMessage(), e);
+ // Client likely disconnected (broken
pipe); log at debug
+ // level and do NOT attempt sendError
– the response may
+ // already be committed.
+ LOGGER.debug("Failed to write proxy
response body (client disconnect?): {}", e.getMessage());
Review Comment:
This debug log drops the exception stack trace entirely, which can make
diagnosing non-client-disconnect I/O failures harder (e.g., container issues,
output stream errors). Consider logging the throwable at debug as well (while
keeping it non-error level) so troubleshooting retains full context.
```suggestion
LOGGER.debug("Failed to write proxy
response body (client disconnect?): {}", e.getMessage(), e);
```
##########
namingserver/src/main/java/org/apache/seata/namingserver/filter/ConsoleRemotingFilter.java:
##########
@@ -108,33 +153,89 @@ public void doFilter(ServletRequest servletRequest,
ServletResponse servletRespo
.forEach(headerName ->
headers.add(headerName, request.getHeader(headerName)));
// Create the HttpEntity with headers and body
- HttpEntity<byte[]> httpEntity = new
HttpEntity<>(request.getCachedBody(), headers);
HttpMethod httpMethod;
try {
httpMethod =
HttpMethod.valueOf(request.getMethod());
} catch (IllegalArgumentException ex) {
- logger.error("Unsupported HTTP method: {}",
request.getMethod(), ex);
+ LOGGER.error("Unsupported HTTP method: {}",
request.getMethod(), ex);
response.setStatus(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
return;
}
+
+ // GET/HEAD methods should not have a body; other
methods may include a body as needed.
+ HttpEntity<byte[]> httpEntity;
+ if (HttpMethod.GET.equals(httpMethod) ||
HttpMethod.HEAD.equals(httpMethod)) {
+ headers.remove(HttpHeaders.CONTENT_LENGTH);
+ headers.remove(HttpHeaders.TRANSFER_ENCODING);
+ // headers-only
+ httpEntity = new HttpEntity<>(headers);
+ } else {
+ byte[] body = request.getCachedBody();
+ if (body == null || body.length == 0) {
+ headers.remove(HttpHeaders.CONTENT_LENGTH);
+
headers.remove(HttpHeaders.TRANSFER_ENCODING);
+ // headers-only for empty body
+ httpEntity = new HttpEntity<>(headers);
+ } else {
+ httpEntity = new HttpEntity<>(body,
headers);
+ }
+ }
+
try {
- ResponseEntity<byte[]> responseEntity =
restTemplate.exchange(
- URI.create(targetUrl), httpMethod,
httpEntity, byte[].class);
+ ResponseEntity<byte[]> responseEntity =
restTemplate.exchange(URI.create(targetUrl), httpMethod, httpEntity,
byte[].class);
+ //Copy headers from proxied response, skipping
hop-by-hop and headers we manage ourselves to mitigate
+ // security risks from Content-Type
manipulation
responseEntity.getHeaders().forEach((key,
value) -> {
- value.forEach(v -> response.addHeader(key,
v));
+ if
(!HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(key)
+ &&
!HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(key)
+ &&
!HttpHeaders.TRANSFER_ENCODING.equalsIgnoreCase(key)
+ &&
!"X-Content-Type-Options".equalsIgnoreCase(key)
+ &&
!HttpHeaders.CONNECTION.equalsIgnoreCase(key)
+ &&
!"Keep-Alive".equalsIgnoreCase(key)
+ &&
!HttpHeaders.PROXY_AUTHENTICATE.equalsIgnoreCase(key)
+ &&
!HttpHeaders.PROXY_AUTHORIZATION.equalsIgnoreCase(key)
+ &&
!HttpHeaders.TE.equalsIgnoreCase(key)
+ &&
!HttpHeaders.TRAILER.equalsIgnoreCase(key)
+ &&
!HttpHeaders.UPGRADE.equalsIgnoreCase(key)) {
+ value.forEach(v ->
response.addHeader(key, v));
+ }
});
- response.setStatus(
-
responseEntity.getStatusCode().value());
-
Optional.ofNullable(responseEntity.getBody()).ifPresent(body -> {
+ // Force a safe Content-Type: reject HTML/XML
types that could
+ // execute scripts; fall back to
application/json
+ String proxiedContentType =
responseEntity.getHeaders().getFirst(HttpHeaders.CONTENT_TYPE);
+ String safeContentType;
+ if (proxiedContentType != null &&
isSafeContentType(proxiedContentType)) {
+ safeContentType = proxiedContentType;
+ } else {
+ safeContentType =
"application/json;charset=UTF-8";
+ }
+ response.setContentType(safeContentType);
+ response.setHeader("X-Content-Type-Options",
"nosniff");
+
response.setStatus(responseEntity.getStatusCode().value());
+ byte[] responseBody = responseEntity.getBody();
+ if (responseBody != null &&
responseBody.length > 0) {
+ // For JSON content type, validate that
the body actually looks
+ // like JSON to prevent XSS via crafted
upstream responses
+ if
(safeContentType.toLowerCase(Locale.ROOT).contains("application/json")
+ && !looksLikeJson(responseBody)) {
+ LOGGER.warn("Upstream returned
non-JSON body for Content-Type {}, replacing with error response",
safeContentType);
+
response.setStatus(HttpServletResponse.SC_BAD_GATEWAY);
+
response.setContentType("application/json;charset=UTF-8");
+ responseBody = "{\"error\":\"Upstream
returned invalid response body\"}"
+
.getBytes(StandardCharsets.UTF_8);
+ }
try (ServletOutputStream outputStream =
response.getOutputStream()) {
- outputStream.write(body);
+ outputStream.write(responseBody);
outputStream.flush();
Review Comment:
For `HEAD` requests, the response must not include a message body. Currently
the code will write a body if the upstream returns one. Consider skipping the
write entirely when `httpMethod == HttpMethod.HEAD` (while still propagating
status and headers) to comply with HTTP semantics and avoid client/proxy
interoperability issues.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
