Block redirect-based SSRF bypass in Swagger imports (#6320)

* goalx: snapshot before shenyu-analysis

* Block redirect-based SSRF bypass in Swagger imports

Swagger import requests were validating only the initial URL while the
shared OkHttp client silently followed redirects. Disabling redirect
following at the HTTP utility layer prevents callers from reaching
unvalidated internal targets through 3xx responses.

Constraint: Keep the fix in the shared HTTP client so all callers inherit the safer default
Rejected: Add Swagger-specific redirect validation only | leaves other HttpUtils callers exposed to the same redirect class
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Do not re-enable redirect following without validating every redirect target before the request is sent
Tested: mvn -pl shenyu-admin -Dtest=HttpUtilsTest,SwaggerImportServiceTest test
Not-tested: Full shenyu-admin test suite and end-to-end Swagger import against live remote servers

* Scope redirect blocking to Swagger import requests

This follow-up preserves the default redirect-following behavior of the
shared HttpUtils client and moves no-redirect handling onto the
Swagger import path only. The regression tests now cover both the
explicit no-redirect mode and the original default behavior.

Constraint: Avoid changing shared HTTP client semantics for unrelated admin features
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: SSRF hardening for one feature should not silently redefine shared client behavior without an explicit compatibility review
Tested: mvn -pl shenyu-admin -Dtest=HttpUtilsTest,SwaggerImportServiceTest test
Not-tested: Full shenyu-admin test suite

* Replace redirect test literals with named constants

This follow-up removes repeated path, status, and body literals from the
HttpUtils redirect tests so the cases read in terms of the HTTP behavior
being verified instead of raw values.

Constraint: Keep the cleanup test-only and local to the redirect SSRF fix
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Name repeated HTTP semantics in tests when they describe protocol behavior rather than incidental values
Tested: mvn -pl shenyu-admin -Dtest=HttpUtilsTest,SwaggerImportServiceTest test
Not-tested: Full shenyu-admin test suite

* Fix redirect test URL construction

The no-redirect regression test was composing redirect URLs from a base
string that already ended with a slash, which produced invalid host/port
combinations in CI. This follow-up switches the test to a dedicated host
prefix constant so the dynamically allocated port is embedded correctly.

Constraint: Keep the fix limited to the test harness; production redirect handling is unchanged
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Avoid composing dynamic host:port URLs from request path fixtures that carry trailing slash semantics
Tested: mvn -pl shenyu-admin -Dtest=HttpUtilsTest,SwaggerImportServiceTest test
Not-tested: Full shenyu-admin test suite

---------

Co-authored-by: moremind <hefengen@apache.org>

Author: Aias00

shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/SwaggerImportServiceImpl.java

@@ -231,7 +231,7 @@ public boolean testConnection(final String swaggerUrl) {
         try {
             validateSwaggerUrl(swaggerUrl);
             try (Response response = httpUtils.requestForResponse(swaggerUrl, 
-                    Collections.emptyMap(), Collections.emptyMap(), HttpUtils.HTTPMethod.GET)) {
+                    Collections.emptyMap(), Collections.emptyMap(), HttpUtils.HTTPMethod.GET, false)) {
                 return response.code() == 200;
             }
         } catch (Exception e) {
@@ -247,7 +247,7 @@ private void validateSwaggerUrl(final String swaggerUrl) {
 
     private String fetchSwaggerDoc(final String swaggerUrl) throws IOException {
         try (Response response = httpUtils.requestForResponse(swaggerUrl,
-                Collections.emptyMap(), Collections.emptyMap(), HttpUtils.HTTPMethod.GET)) {
+                Collections.emptyMap(), Collections.emptyMap(), HttpUtils.HTTPMethod.GET, false)) {
 
             if (response.code() != 200) {
                 throw new RuntimeException("Failed to get Swagger document, HTTP status code: " + response.code());

shenyu-admin/src/main/java/org/apache/shenyu/admin/utils/HttpUtils.java

@@ -325,10 +325,30 @@ public Response requestCall(final String url, final Map<String, ?> form, final M
      */
     public Response requestForResponse(final String url, final Map<String, ?> form, final Map<String, String> header,
         final HTTPMethod method) throws IOException {
+        return requestForResponse(url, form, header, method, true);
+    }
+
+    /**
+     * request data with configurable redirect handling.
+     *
+     * @param url request url
+     * @param form request data
+     * @param header header
+     * @param method method
+     * @param followRedirects whether redirects should be followed
+     * @return Response
+     * @throws IOException IOException
+     */
+    public Response requestForResponse(final String url, final Map<String, ?> form, final Map<String, String> header,
+        final HTTPMethod method, final boolean followRedirects) throws IOException {
         Request.Builder requestBuilder = buildRequestBuilder(url, form, method);
         addHeader(requestBuilder, header);
         Request request = requestBuilder.build();
-        return httpClient
+        OkHttpClient client = followRedirects ? httpClient : httpClient.newBuilder()
+            .followRedirects(false)
+            .followSslRedirects(false)
+            .build();
+        return client
             .newCall(request)
             .execute();
     }

shenyu-admin/src/test/java/org/apache/shenyu/admin/utils/HttpUtilsTest.java

@@ -17,14 +17,19 @@
 
 package org.apache.shenyu.admin.utils;
 
+import com.sun.net.httpserver.HttpServer;
 import okhttp3.FormBody;
 import okhttp3.HttpUrl;
 import okhttp3.Request;
+import okhttp3.Response;
 import org.junit.Assert;
 import org.junit.Test;
 import org.junit.jupiter.api.Assertions;
+
+import java.net.InetSocketAddress;
 import java.io.File;
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -35,6 +40,18 @@ public class HttpUtilsTest {
 
     private static final String TEST_URL = "http://127.0.0.1/";
 
+    private static final String LOCALHOST_BASE_URL = "http://127.0.0.1:";
+
+    private static final String REDIRECT_PATH = "/swagger.json";
+
+    private static final String TARGET_PATH = "/internal";
+
+    private static final String TARGET_BODY = "target";
+
+    private static final int HTTP_STATUS_OK = 200;
+
+    private static final int HTTP_STATUS_REDIRECT = 302;
+
     private final Map<String, Object> formMap = new HashMap<>();
 
     {
@@ -113,4 +130,65 @@ public void fileUtilsToBytesByFileNotExistsTest() {
         File file = new File("");
         Assertions.assertThrows(IOException.class, () -> HttpUtils.FileUtils.toBytes(file));
     }
+
+    @Test
+    public void requestForResponseShouldNotFollowRedirectsWhenExplicitlyDisabled() throws IOException {
+        HttpServer targetServer = HttpServer.create(new InetSocketAddress(0), 0);
+        targetServer.createContext(TARGET_PATH, exchange -> {
+            byte[] body = TARGET_BODY.getBytes(StandardCharsets.UTF_8);
+            exchange.sendResponseHeaders(HTTP_STATUS_OK, body.length);
+            exchange.getResponseBody().write(body);
+            exchange.close();
+        });
+        targetServer.start();
+
+        HttpServer redirectServer = HttpServer.create(new InetSocketAddress(0), 0);
+        String redirectLocation = LOCALHOST_BASE_URL + targetServer.getAddress().getPort() + TARGET_PATH;
+        redirectServer.createContext(REDIRECT_PATH, exchange -> {
+            exchange.getResponseHeaders().add("Location", redirectLocation);
+            exchange.sendResponseHeaders(HTTP_STATUS_REDIRECT, -1);
+            exchange.close();
+        });
+        redirectServer.start();
+
+        HttpUtils httpUtils = new HttpUtils();
+        String redirectUrl = LOCALHOST_BASE_URL + redirectServer.getAddress().getPort() + REDIRECT_PATH;
+        try (Response response = httpUtils.requestForResponse(redirectUrl, new HashMap<>(), new HashMap<>(), HttpUtils.HTTPMethod.GET, false)) {
+            Assert.assertEquals(HTTP_STATUS_REDIRECT, response.code());
+            Assert.assertEquals(redirectLocation, response.header("Location"));
+        } finally {
+            redirectServer.stop(0);
+            targetServer.stop(0);
+        }
+    }
+
+    @Test
+    public void requestForResponseShouldFollowRedirectsByDefault() throws IOException {
+        HttpServer targetServer = HttpServer.create(new InetSocketAddress(0), 0);
+        targetServer.createContext(TARGET_PATH, exchange -> {
+            byte[] body = TARGET_BODY.getBytes(StandardCharsets.UTF_8);
+            exchange.sendResponseHeaders(HTTP_STATUS_OK, body.length);
+            exchange.getResponseBody().write(body);
+            exchange.close();
+        });
+        targetServer.start();
+
+        HttpServer redirectServer = HttpServer.create(new InetSocketAddress(0), 0);
+        String redirectLocation = LOCALHOST_BASE_URL + targetServer.getAddress().getPort() + TARGET_PATH;
+        redirectServer.createContext(REDIRECT_PATH, exchange -> {
+            exchange.getResponseHeaders().add("Location", redirectLocation);
+            exchange.sendResponseHeaders(HTTP_STATUS_REDIRECT, -1);
+            exchange.close();
+        });
+        redirectServer.start();
+
+        HttpUtils httpUtils = new HttpUtils();
+        String redirectUrl = LOCALHOST_BASE_URL + redirectServer.getAddress().getPort() + REDIRECT_PATH;
+        try (Response response = httpUtils.requestForResponse(redirectUrl, new HashMap<>(), new HashMap<>(), HttpUtils.HTTPMethod.GET)) {
+            Assert.assertEquals(HTTP_STATUS_OK, response.code());
+        } finally {
+            redirectServer.stop(0);
+            targetServer.stop(0);
+        }
+    }
 }