You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shenyu.apache.org by he...@apache.org on 2026/04/14 11:33:31 UTC
(shenyu) branch master updated: Align SSRF URL validation with OkHttp parsing (#6321)
This is an automated email from the ASF dual-hosted git repository.
hefengen 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 bec32f3559 Align SSRF URL validation with OkHttp parsing (#6321)
bec32f3559 is described below
commit bec32f355972e095ef57289e84a30578b71b60ee
Author: aias00 <li...@apache.org>
AuthorDate: Tue Apr 14 19:33:26 2026 +0800
Align SSRF URL validation with OkHttp parsing (#6321)
* goalx: snapshot before shenyu-analysis
* Align SSRF URL validation with the OkHttp request target
The Swagger import SSRF guard was parsing URLs with java.net.URL while
the actual request path was interpreted by OkHttp. This mismatch let
parser-confusion payloads pass validation but resolve to internal
hosts during request execution. The fix validates against OkHttp's
HttpUrl semantics and adds regression coverage for the known payload.
Constraint: Keep validation semantics aligned with the HTTP client actually used for outbound requests
Rejected: Add ad-hoc blacklist checks for backslash or @ combinations | brittle and would miss future parser edge cases
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Any future SSRF validation must use the same parser and canonicalization rules as the outbound HTTP client
Tested: mvn -pl shenyu-admin -Dtest=UrlSecurityUtilsTest,SwaggerImportServiceTest test
Not-tested: Full shenyu-admin test suite and live end-to-end Swagger import against a running admin instance
* Name parser-confusion SSRF payload in tests
This follow-up replaces the repeated parser-confusion exploit string with
a single named constant in the SSRF regression tests.
Constraint: Limit the cleanup to the dedicated parser-confusion test surface
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Keep security regression payloads named when reused across multiple assertions
Tested: mvn -pl shenyu-admin -Dtest=UrlSecurityUtilsTest,SwaggerImportServiceTest test
Not-tested: Full shenyu-admin test suite
* Name allowed URL schemes in SSRF validation
This follow-up replaces the raw http/https string checks in
UrlSecurityUtils with named protocol constants.
Constraint: Keep the cleanup local to the SSRF validation utility without changing behavior
Confidence: high
Scope-risk: narrow
Reversibility: clean
Directive: Protocol allowlists should use named constants when shared across security-sensitive validation logic
Tested: mvn -pl shenyu-admin -Dtest=UrlSecurityUtilsTest,SwaggerImportServiceTest test
Not-tested: Full shenyu-admin test suite
---
.gitignore | 3 ++
.../shenyu/admin/utils/UrlSecurityUtils.java | 31 ++++++++++---------
.../admin/service/SwaggerImportServiceTest.java | 13 +++++++-
.../shenyu/admin/utils/UrlSecurityUtilsTest.java | 36 ++++++++++++++++++++++
4 files changed, 68 insertions(+), 15 deletions(-)
diff --git a/.gitignore b/.gitignore
index ed771e556c..e266500fd7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -50,3 +50,6 @@ Thumbs.db
# Private individual user cursor rules
.cursor/rules/_*.mdc
+
+# local worktrees
+.worktrees/
diff --git a/shenyu-admin/src/main/java/org/apache/shenyu/admin/utils/UrlSecurityUtils.java b/shenyu-admin/src/main/java/org/apache/shenyu/admin/utils/UrlSecurityUtils.java
index 9ac69800ec..cbdad61455 100644
--- a/shenyu-admin/src/main/java/org/apache/shenyu/admin/utils/UrlSecurityUtils.java
+++ b/shenyu-admin/src/main/java/org/apache/shenyu/admin/utils/UrlSecurityUtils.java
@@ -17,9 +17,9 @@
package org.apache.shenyu.admin.utils;
+import okhttp3.HttpUrl;
+
import java.net.InetAddress;
-import java.net.MalformedURLException;
-import java.net.URL;
import java.net.UnknownHostException;
import java.util.Arrays;
import java.util.HashSet;
@@ -31,6 +31,10 @@ import java.util.Set;
*/
public final class UrlSecurityUtils {
+ private static final String HTTP_PROTOCOL = "http";
+
+ private static final String HTTPS_PROTOCOL = "https";
+
/**
* Private constructor to prevent instantiation.
*/
@@ -48,21 +52,20 @@ public final class UrlSecurityUtils {
throw new IllegalArgumentException("URL cannot be empty");
}
- try {
- URL parsedUrl = new URL(url);
- String protocol = parsedUrl.getProtocol();
-
- // Only allow HTTP and HTTPS protocols
- if (!"http".equals(protocol) && !"https".equals(protocol)) {
- throw new IllegalArgumentException("Only HTTP and HTTPS protocols are allowed");
- }
+ HttpUrl parsedUrl = HttpUrl.parse(url);
+ if (Objects.isNull(parsedUrl)) {
+ throw new IllegalArgumentException("Invalid URL format");
+ }
- // Validate host for SSRF protection
- validateHostForSSRF(parsedUrl.getHost(), parsedUrl.getPort());
+ String protocol = parsedUrl.scheme();
- } catch (MalformedURLException e) {
- throw new IllegalArgumentException("Invalid URL format: " + e.getMessage());
+ // Only allow HTTP and HTTPS protocols
+ if (!HTTP_PROTOCOL.equals(protocol) && !HTTPS_PROTOCOL.equals(protocol)) {
+ throw new IllegalArgumentException("Only HTTP and HTTPS protocols are allowed");
}
+
+ // Validate host for SSRF protection using the same URL parser as request execution.
+ validateHostForSSRF(parsedUrl.host(), parsedUrl.port());
}
/**
diff --git a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/SwaggerImportServiceTest.java b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/SwaggerImportServiceTest.java
index e3a4f653d7..2ba839f58e 100644
--- a/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/SwaggerImportServiceTest.java
+++ b/shenyu-admin/src/test/java/org/apache/shenyu/admin/service/SwaggerImportServiceTest.java
@@ -26,12 +26,15 @@ import org.mockito.junit.jupiter.MockitoExtension;
import org.apache.shenyu.admin.service.manager.DocManager;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.mockito.Mockito.verifyNoInteractions;
/**
* Test for SwaggerImportService.
*/
@ExtendWith(MockitoExtension.class)
public class SwaggerImportServiceTest {
+
+ private static final String PARSER_CONFUSION_PAYLOAD = "http://127.0.0.1:6666\\@1.1.1.1";
@Mock
private DocManager docManager;
@@ -51,4 +54,12 @@ public class SwaggerImportServiceTest {
// Test valid URL format but may fail to connect
assertFalse(service.testConnection("http://invalid.example.com/swagger.json"));
}
-}
+
+ @Test
+ public void testConnectionShouldRejectParserConfusionPayloadBeforeRequest() {
+ SwaggerImportService service = new SwaggerImportServiceImpl(docManager, httpUtils);
+
+ assertFalse(service.testConnection(PARSER_CONFUSION_PAYLOAD));
+ verifyNoInteractions(httpUtils);
+ }
+}
diff --git a/shenyu-admin/src/test/java/org/apache/shenyu/admin/utils/UrlSecurityUtilsTest.java b/shenyu-admin/src/test/java/org/apache/shenyu/admin/utils/UrlSecurityUtilsTest.java
new file mode 100644
index 0000000000..850055d47a
--- /dev/null
+++ b/shenyu-admin/src/test/java/org/apache/shenyu/admin/utils/UrlSecurityUtilsTest.java
@@ -0,0 +1,36 @@
+/*
+ * 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.shenyu.admin.utils;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/**
+ * Test for {@link UrlSecurityUtils}.
+ */
+public class UrlSecurityUtilsTest {
+
+ private static final String PARSER_CONFUSION_PAYLOAD = "http://127.0.0.1:6666\\@1.1.1.1";
+
+ @Test
+ public void validateUrlForSSRFShouldRejectParserConfusionPayload() {
+ assertThrows(IllegalArgumentException.class,
+ () -> UrlSecurityUtils.validateUrlForSSRF(PARSER_CONFUSION_PAYLOAD));
+ }
+}