This is an automated email from the ASF dual-hosted git repository.
Aias00 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shenyu.git
The following commit(s) were added to refs/heads/master by this push:
new 0f61285cde fix(admin): strip hop-by-hop headers in cluster forward
filter (#7807) (#7822)
0f61285cde is described below
commit 0f61285cde56e920316720321f8400324c88b9e1
Author: Harsh Raj <[email protected]>
AuthorDate: Tue Oct 20 09:56:43 2026 +0530
fix(admin): strip hop-by-hop headers in cluster forward filter (#7807)
(#7822)
Co-authored-by: Harsh <[email protected]>
---
.../mode/cluster/filter/ClusterForwardFilter.java | 111 ++++++++++-
.../cluster/filter/ClusterForwardFilterTest.java | 212 ++++++++++++++++++++-
2 files changed, 313 insertions(+), 10 deletions(-)
diff --git
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/mode/cluster/filter/ClusterForwardFilter.java
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/mode/cluster/filter/ClusterForwardFilter.java
index 7cc1445739..6e598b1dad 100644
---
a/shenyu-admin/src/main/java/org/apache/shenyu/admin/mode/cluster/filter/ClusterForwardFilter.java
+++
b/shenyu-admin/src/main/java/org/apache/shenyu/admin/mode/cluster/filter/ClusterForwardFilter.java
@@ -22,6 +22,7 @@ import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
+import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.shenyu.admin.config.properties.ClusterProperties;
@@ -46,7 +47,12 @@ import java.io.IOException;
import java.io.InputStream;
import java.net.URI;
import java.util.Collections;
+import java.util.Enumeration;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Locale;
import java.util.Objects;
+import java.util.Set;
/**
* Cluster forward filter.
@@ -56,6 +62,17 @@ public class ClusterForwardFilter extends
OncePerRequestFilter {
private static final Logger LOG =
LoggerFactory.getLogger(ClusterForwardFilter.class);
private static final PathMatcher PATH_MATCHER = new AntPathMatcher();
+
+ private static final Set<String> HOP_BY_HOP_HEADERS = Set.of(
+ "connection",
+ "keep-alive",
+ "proxy-authenticate",
+ "proxy-authorization",
+ "te",
+ "trailer",
+ "transfer-encoding",
+ "upgrade"
+ );
private final RestTemplate restTemplate;
@@ -68,6 +85,14 @@ public class ClusterForwardFilter extends
OncePerRequestFilter {
public ClusterForwardFilter(final RestTemplate restTemplate) {
this.restTemplate = restTemplate;
}
+
+ public ClusterForwardFilter(final RestTemplate restTemplate,
+ final ClusterSelectMasterService
clusterSelectMasterService,
+ final ClusterProperties clusterProperties) {
+ this.restTemplate = restTemplate;
+ this.clusterSelectMasterService = clusterSelectMasterService;
+ this.clusterProperties = clusterProperties;
+ }
@Override
protected void doFilterInternal(@NotNull final HttpServletRequest request,
@@ -138,15 +163,30 @@ public class ClusterForwardFilter extends
OncePerRequestFilter {
ResponseEntity<byte[]> responseEntity =
restTemplate.exchange(targetUrl, HttpMethod.valueOf(request.getMethod()),
requestEntity, byte[].class);
// Set response status and headers
- response.setStatus(responseEntity.getStatusCode().value());
+ int statusCode = responseEntity.getStatusCode().value();
+ response.setStatus(statusCode);
// Copy response headers
copyHeaders(responseEntity.getHeaders(), response);
// fix cors error
- response.addHeader("Access-Control-Allow-Origin", "*");
- response.addHeader("Access-Control-Allow-Methods", "GET, POST, PUT,
DELETE");
+ if (!response.containsHeader("Access-Control-Allow-Origin")) {
+ response.setHeader("Access-Control-Allow-Origin", "*");
+ }
+ if (!response.containsHeader("Access-Control-Allow-Methods")) {
+ response.setHeader("Access-Control-Allow-Methods", "GET, POST,
PUT, DELETE");
+ }
// write response back
- IOUtils.copy(new
ByteArrayInputStream(Objects.requireNonNull(responseEntity.getBody())),
response.getOutputStream());
- response.getOutputStream().flush();
+ byte[] body = Objects.nonNull(responseEntity.getBody()) ?
responseEntity.getBody() : new byte[0];
+ if (statusCode != HttpServletResponse.SC_NO_CONTENT
+ && statusCode != HttpServletResponse.SC_NOT_MODIFIED
+ && (statusCode < 100 || statusCode >= 200)) {
+ response.setContentLength(body.length);
+ }
+ if (body.length > 0) {
+ IOUtils.copy(new ByteArrayInputStream(body),
response.getOutputStream());
+ response.getOutputStream().flush();
+ } else {
+ response.flushBuffer();
+ }
}
@NotNull
@@ -178,19 +218,64 @@ public class ClusterForwardFilter extends
OncePerRequestFilter {
return builder.toUriString();
}
- private void copyHeaders(final HttpServletRequest request, final
HttpHeaders headers) {
+ void copyHeaders(final HttpServletRequest request, final HttpHeaders
headers) {
+ Set<String> hopByHopHeaders = buildHopByHopHeaders(request);
Collections.list(request.getHeaderNames())
- .forEach(headerName -> headers.add(headerName,
removeSpecial(request.getHeader(headerName))));
+ .forEach(headerName -> {
+ String name = removeSpecial(headerName);
+ String lowerName = name.toLowerCase(Locale.ROOT);
+ if (!hopByHopHeaders.contains(lowerName)
+ && !HttpHeaders.HOST.equalsIgnoreCase(lowerName)
+ &&
!HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(lowerName)) {
+ Collections.list(request.getHeaders(headerName))
+ .forEach(headerValue -> headers.add(name,
removeSpecial(headerValue)));
+ }
+ });
}
- private void copyHeaders(final HttpHeaders sourceHeaders, final
HttpServletResponse response) {
+ void copyHeaders(final HttpHeaders sourceHeaders, final
HttpServletResponse response) {
+ Set<String> hopByHopHeaders = buildHopByHopHeaders(sourceHeaders);
sourceHeaders.forEach((headerName, headerValues) -> {
String name = removeSpecial(headerName);
- if (!response.containsHeader(name)) {
+ String lowerName = name.toLowerCase(Locale.ROOT);
+ if (!hopByHopHeaders.contains(lowerName)
+ && !HttpHeaders.CONTENT_LENGTH.equalsIgnoreCase(lowerName)
+ && !response.containsHeader(name)) {
headerValues.forEach(headerValue -> response.addHeader(name,
removeSpecial(headerValue)));
}
});
}
+
+ private Set<String> buildHopByHopHeaders(final HttpServletRequest request)
{
+ Set<String> hopByHopHeaders = new HashSet<>(HOP_BY_HOP_HEADERS);
+ Enumeration<String> connectionHeaders =
request.getHeaders(HttpHeaders.CONNECTION);
+ if (Objects.nonNull(connectionHeaders)) {
+ while (connectionHeaders.hasMoreElements()) {
+ String value = connectionHeaders.nextElement();
+ if (StringUtils.isNotEmpty(value)) {
+ for (String token : value.split(",")) {
+
hopByHopHeaders.add(token.trim().toLowerCase(Locale.ROOT));
+ }
+ }
+ }
+ }
+ return hopByHopHeaders;
+ }
+
+ private Set<String> buildHopByHopHeaders(final HttpHeaders sourceHeaders) {
+ Set<String> hopByHopHeaders = new HashSet<>(HOP_BY_HOP_HEADERS);
+ List<String> connectionValues =
sourceHeaders.get(HttpHeaders.CONNECTION);
+ if (CollectionUtils.isNotEmpty(connectionValues)) {
+ for (String value : connectionValues) {
+ if (StringUtils.isNotEmpty(value)) {
+ for (String token : value.split(",")) {
+
hopByHopHeaders.add(token.trim().toLowerCase(Locale.ROOT));
+ }
+ }
+ }
+ }
+ return hopByHopHeaders;
+ }
private String removeSpecial(final String str) {
return str.replaceAll("\r", "").replaceAll("\n", "");
@@ -206,5 +291,13 @@ public class ClusterForwardFilter extends
OncePerRequestFilter {
}
return baos.toByteArray();
}
+
+ public void setClusterSelectMasterService(final ClusterSelectMasterService
clusterSelectMasterService) {
+ this.clusterSelectMasterService = clusterSelectMasterService;
+ }
+
+ public void setClusterProperties(final ClusterProperties
clusterProperties) {
+ this.clusterProperties = clusterProperties;
+ }
}
diff --git
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/mode/cluster/filter/ClusterForwardFilterTest.java
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/mode/cluster/filter/ClusterForwardFilterTest.java
index 0436d20af9..7fb8b5eb6b 100644
---
a/shenyu-admin/src/test/java/org/apache/shenyu/admin/mode/cluster/filter/ClusterForwardFilterTest.java
+++
b/shenyu-admin/src/test/java/org/apache/shenyu/admin/mode/cluster/filter/ClusterForwardFilterTest.java
@@ -17,9 +17,13 @@
package org.apache.shenyu.admin.mode.cluster.filter;
+import com.sun.net.httpserver.HttpServer;
+import jakarta.servlet.DispatcherType;
import org.apache.shenyu.admin.config.properties.ClusterProperties;
import org.apache.shenyu.admin.model.dto.ClusterMasterDTO;
import org.apache.shenyu.admin.mode.cluster.service.ClusterSelectMasterService;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@@ -28,7 +32,10 @@ import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import org.mockito.junit.jupiter.MockitoSettings;
import org.mockito.quality.Strictness;
+import
org.springframework.boot.web.embedded.tomcat.TomcatServletWebServerFactory;
+import org.springframework.boot.web.server.WebServer;
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;
@@ -39,25 +46,37 @@ import org.springframework.test.util.ReflectionTestUtils;
import org.springframework.web.client.RestTemplate;
import org.yaml.snakeyaml.Yaml;
+import java.io.BufferedReader;
import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStream;
+import java.net.HttpURLConnection;
+import java.net.InetSocketAddress;
+import java.net.Socket;
+import java.net.URI;
import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
import java.util.Collections;
+import java.util.EnumSet;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
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;
/**
- * Test case for {@link ClusterForwardFilter}.
+ * Test cases for {@link ClusterForwardFilter}.
*/
@ExtendWith(MockitoExtension.class)
@MockitoSettings(strictness = Strictness.LENIENT)
@@ -67,6 +86,16 @@ public final class ClusterForwardFilterTest {
private static final String MASTER_PORT = "9095";
+ private static final String MASTER_BODY =
"{\"code\":200,\"message\":\"success\",\"data\":{\"id\":\"123\"}}";
+
+ private static HttpServer masterServer;
+
+ private static WebServer followerServer;
+
+ private static int masterPort;
+
+ private static int followerPort;
+
@Mock
private RestTemplate restTemplate;
@@ -91,6 +120,56 @@ public final class ClusterForwardFilterTest {
when(clusterSelectMasterService.getMaster()).thenReturn(master);
}
+ @BeforeAll
+ public static void setUpServers() throws Exception {
+ masterServer = HttpServer.create(new InetSocketAddress(0), 0);
+ masterServer.createContext("/selector", exchange -> {
+ byte[] bytes = MASTER_BODY.getBytes(StandardCharsets.UTF_8);
+ exchange.getResponseHeaders().set("Content-Type",
"application/json");
+ // sendResponseHeaders with 0 triggers HTTP/1.1 chunked transfer
encoding in JDK HttpServer
+ exchange.sendResponseHeaders(200, 0);
+ OutputStream os = exchange.getResponseBody();
+ os.write(bytes);
+ os.close();
+ });
+ masterServer.start();
+ masterPort = masterServer.getAddress().getPort();
+
+ ClusterSelectMasterService selectMasterService =
mock(ClusterSelectMasterService.class);
+ when(selectMasterService.isMaster()).thenReturn(false);
+ ClusterMasterDTO masterDTO = new ClusterMasterDTO();
+ masterDTO.setMasterHost("127.0.0.1");
+ masterDTO.setMasterPort(String.valueOf(masterPort));
+ masterDTO.setContextPath("");
+ when(selectMasterService.getMaster()).thenReturn(masterDTO);
+
+ ClusterProperties forwardingProperties = new ClusterProperties();
+ forwardingProperties.setForwardList(Collections.singletonList("/**"));
+ forwardingProperties.setIgnoredList(Collections.emptyList());
+
+ ClusterForwardFilter filter = new ClusterForwardFilter(new
RestTemplate(), selectMasterService, forwardingProperties);
+
+ TomcatServletWebServerFactory factory = new
TomcatServletWebServerFactory(0);
+ followerServer = factory.getWebServer(servletContext -> {
+ servletContext.addFilter("clusterForwardFilter", filter)
+
.addMappingForUrlPatterns(EnumSet.of(DispatcherType.REQUEST), true, "/*");
+ servletContext.addServlet("testServlet", new
jakarta.servlet.http.HttpServlet() { })
+ .addMapping("/*");
+ });
+ followerServer.start();
+ followerPort = followerServer.getPort();
+ }
+
+ @AfterAll
+ public static void tearDownServers() {
+ if (Objects.nonNull(followerServer)) {
+ followerServer.stop();
+ }
+ if (Objects.nonNull(masterServer)) {
+ masterServer.stop(0);
+ }
+ }
+
@Test
public void testMasterHandlesRequestsLocally() throws Exception {
when(clusterSelectMasterService.isMaster()).thenReturn(true);
@@ -153,6 +232,137 @@ public final class ClusterForwardFilterTest {
}
}
+ @Test
+ public void
testCopyRequestHeadersExcludesHopByHopAndHostAndContentLength() {
+ MockHttpServletRequest request = new MockHttpServletRequest();
+ request.addHeader(HttpHeaders.CONNECTION, "close, X-Custom-Hop");
+ request.addHeader(HttpHeaders.HOST, "follower.shenyu.apache.org:9096");
+ request.addHeader(HttpHeaders.CONTENT_LENGTH, "100");
+ request.addHeader(HttpHeaders.TRANSFER_ENCODING, "chunked");
+ request.addHeader(HttpHeaders.UPGRADE, "websocket");
+ request.addHeader("X-Custom-Hop", "remove-this");
+ request.addHeader(HttpHeaders.AUTHORIZATION, "Bearer test-token");
+ request.addHeader("X-Forwarded-For", "192.168.1.1");
+
+ HttpHeaders headers = new HttpHeaders();
+ ClusterForwardFilter filter = new
ClusterForwardFilter(mock(RestTemplate.class));
+ filter.copyHeaders(request, headers);
+
+ assertFalse(headers.containsKey(HttpHeaders.CONNECTION));
+ assertFalse(headers.containsKey(HttpHeaders.HOST));
+ assertFalse(headers.containsKey(HttpHeaders.CONTENT_LENGTH));
+ assertFalse(headers.containsKey(HttpHeaders.TRANSFER_ENCODING));
+ assertFalse(headers.containsKey(HttpHeaders.UPGRADE));
+ assertFalse(headers.containsKey("X-Custom-Hop"));
+
+ assertTrue(headers.containsKey(HttpHeaders.AUTHORIZATION));
+ assertEquals("Bearer test-token",
headers.getFirst(HttpHeaders.AUTHORIZATION));
+ assertTrue(headers.containsKey("X-Forwarded-For"));
+ assertEquals("192.168.1.1", headers.getFirst("X-Forwarded-For"));
+ }
+
+ @Test
+ public void testCopyResponseHeadersExcludesHopByHopAndContentLength() {
+ HttpHeaders sourceHeaders = new HttpHeaders();
+ sourceHeaders.add(HttpHeaders.TRANSFER_ENCODING, "chunked");
+ sourceHeaders.add(HttpHeaders.CONNECTION, "keep-alive, X-Resp-Hop");
+ sourceHeaders.add("Keep-Alive", "timeout=5");
+ sourceHeaders.add(HttpHeaders.CONTENT_LENGTH, "456");
+ sourceHeaders.add("X-Resp-Hop", "strip-this");
+ sourceHeaders.add(HttpHeaders.CONTENT_TYPE, "application/json");
+ sourceHeaders.add("X-Custom-Header", "allowed-value");
+
+ MockHttpServletResponse response = new MockHttpServletResponse();
+ ClusterForwardFilter filter = new
ClusterForwardFilter(mock(RestTemplate.class));
+ filter.copyHeaders(sourceHeaders, response);
+
+ assertFalse(response.containsHeader(HttpHeaders.TRANSFER_ENCODING));
+ assertFalse(response.containsHeader(HttpHeaders.CONNECTION));
+ assertFalse(response.containsHeader("Keep-Alive"));
+ assertFalse(response.containsHeader(HttpHeaders.CONTENT_LENGTH));
+ assertFalse(response.containsHeader("X-Resp-Hop"));
+
+ assertTrue(response.containsHeader(HttpHeaders.CONTENT_TYPE));
+ assertEquals("application/json",
response.getHeader(HttpHeaders.CONTENT_TYPE));
+ assertTrue(response.containsHeader("X-Custom-Header"));
+ assertEquals("allowed-value", response.getHeader("X-Custom-Header"));
+ }
+
+ @Test
+ public void testRealHttpFollowerForwardingWithConnectionClose() throws
Exception {
+ URI uri = URI.create("http://127.0.0.1:"; + followerPort + "/selector");
+ HttpURLConnection connection = (HttpURLConnection)
uri.toURL().openConnection();
+ connection.setRequestMethod("GET");
+ connection.setRequestProperty(HttpHeaders.CONNECTION, "close");
+ connection.setConnectTimeout(5000);
+ connection.setReadTimeout(5000);
+
+ int responseCode = connection.getResponseCode();
+ assertEquals(200, responseCode);
+
+ // Verify that follower response does NOT advertise chunked encoding
when Connection: close
+ assertNull(connection.getHeaderField(HttpHeaders.TRANSFER_ENCODING));
+
+ // Content-Length must be present and match the forwarded payload bytes
+ String contentLengthHeader =
connection.getHeaderField(HttpHeaders.CONTENT_LENGTH);
+ byte[] expectedBytes = MASTER_BODY.getBytes(StandardCharsets.UTF_8);
+ assertEquals(String.valueOf(expectedBytes.length),
contentLengthHeader);
+
+ // Reading the body must succeed completely without IncompleteRead
+ InputStream is = connection.getInputStream();
+ byte[] actualBytes = is.readAllBytes();
+ assertEquals(new String(expectedBytes, StandardCharsets.UTF_8), new
String(actualBytes, StandardCharsets.UTF_8));
+ connection.disconnect();
+ }
+
+ @Test
+ public void testRealHttpFollowerForwardingWithConnectionKeepAlive() throws
Exception {
+ try (Socket socket = new Socket("127.0.0.1", followerPort)) {
+ socket.setSoTimeout(5000);
+ OutputStream out = socket.getOutputStream();
+ String rawRequest = "GET /selector HTTP/1.1\r\n"
+ + "Host: 127.0.0.1:" + followerPort + "\r\n"
+ + "Connection: keep-alive\r\n\r\n";
+ out.write(rawRequest.getBytes(StandardCharsets.UTF_8));
+ out.flush();
+
+ BufferedReader reader = new BufferedReader(new
InputStreamReader(socket.getInputStream(), StandardCharsets.UTF_8));
+ String statusLine = reader.readLine();
+ assertTrue(statusLine.contains("200"));
+
+ List<String> transferEncodingHeaders = new ArrayList<>();
+ List<String> connectionHeaders = new ArrayList<>();
+ String line;
+ int contentLength = -1;
+ while (Objects.nonNull(line = reader.readLine()) &&
!line.isEmpty()) {
+ String lower = line.toLowerCase();
+ if (lower.startsWith("transfer-encoding:")) {
+ transferEncodingHeaders.add(line);
+ } else if (lower.startsWith("connection:")) {
+ connectionHeaders.add(line);
+ } else if (lower.startsWith("content-length:")) {
+ contentLength =
Integer.parseInt(line.split(":")[1].trim());
+ }
+ }
+
+ // Must NOT have duplicate Transfer-Encoding headers
+ assertEquals(0, transferEncodingHeaders.size());
+
+ // Connection header must not have duplicated tokens like
"keep-alive, keep-alive"
+ assertEquals(1, connectionHeaders.size());
+ assertFalse(connectionHeaders.get(0).contains("keep-alive,
keep-alive"));
+
+ // Content-Length must match the forwarded payload
+ byte[] expectedBytes =
MASTER_BODY.getBytes(StandardCharsets.UTF_8);
+ assertEquals(expectedBytes.length, contentLength);
+
+ char[] bodyChars = new char[contentLength];
+ int read = reader.read(bodyChars, 0, contentLength);
+ assertEquals(contentLength, read);
+ assertEquals(MASTER_BODY, new String(bodyChars));
+ }
+ }
+
private MockHttpServletRequest beatRequest() {
final MockHttpServletRequest request = new MockHttpServletRequest();
request.setMethod("POST");
