You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by am...@apache.org on 2023/06/22 08:04:50 UTC

[knox] branch master updated: KNOX-2928 - For malformed url should return 400 bad request instead of 500 (#766)

This is an automated email from the ASF dual-hosted git repository.

amagyar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git


The following commit(s) were added to refs/heads/master by this push:
     new 8f1fe56e1 KNOX-2928 - For malformed url should return 400 bad request instead of 500 (#766)
8f1fe56e1 is described below

commit 8f1fe56e1a48b40e7ea3b41a34fb48dd586aad84
Author: Attila Magyar <m....@gmail.com>
AuthorDate: Thu Jun 22 10:04:44 2023 +0200

    KNOX-2928 - For malformed url should return 400 bad request instead of 500 (#766)
---
 .../apache/knox/gateway/service/knoxsso/WebSSOResource.java    |  8 +++++++-
 .../knox/gateway/service/knoxsso/WebSSOResourceTest.java       |  7 ++++++-
 .../main/java/org/apache/knox/gateway/SpiGatewayMessages.java  |  3 +++
 .../apache/knox/gateway/dispatch/GatewayDispatchFilter.java    |  8 +++++++-
 .../src/main/java/org/apache/knox/gateway/util/RegExUtils.java | 10 +++-------
 5 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java b/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java
index 531524d11..a4ed8e1b1 100644
--- a/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java
+++ b/gateway-service-knoxsso/src/main/java/org/apache/knox/gateway/service/knoxsso/WebSSOResource.java
@@ -22,6 +22,7 @@ import static javax.ws.rs.core.MediaType.APPLICATION_XML;
 import static org.apache.knox.gateway.services.GatewayServices.GATEWAY_CLUSTER_ATTRIBUTE;
 
 import java.io.IOException;
+import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.security.Principal;
@@ -249,7 +250,12 @@ public class WebSSOResource {
       // If there is a whitelist defined, then the original URL must be validated against it.
       // If there is no whitelist, then everything is valid.
       if (whitelist != null) {
-        validRedirect = RegExUtils.checkBaseUrlAgainstWhitelist(whitelist, original);
+        try {
+          validRedirect = RegExUtils.checkBaseUrlAgainstWhitelist(whitelist, original);
+        } catch (MalformedURLException e) {
+          throw new WebApplicationException("Malformed original URL: " + original,
+                  Response.Status.BAD_REQUEST);
+        }
       }
 
       if (!validRedirect) {
diff --git a/gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java b/gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java
index 33be1c79c..bb31f4fb8 100644
--- a/gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java
+++ b/gateway-service-knoxsso/src/test/java/org/apache/knox/gateway/service/knoxsso/WebSSOResourceTest.java
@@ -28,6 +28,7 @@ import static org.junit.Assert.assertTrue;
 
 import java.lang.reflect.Field;
 import java.net.HttpCookie;
+import java.net.MalformedURLException;
 import java.net.URLEncoder;
 import java.nio.charset.StandardCharsets;
 import java.security.KeyPair;
@@ -149,7 +150,7 @@ public class WebSSOResourceTest {
   }
 
   @Test
-  public void testWhitelistMatchingAgainstBaseURL() {
+  public void testWhitelistMatchingAgainstBaseURL() throws MalformedURLException {
     Assert.assertTrue("Failed to match whitelist",
             RegExUtils.checkBaseUrlAgainstWhitelist("^https?:\\/\\/(.*KNOX_GW_DOMAIN)(?::[0-9]+)?(?:\\/.*)?$",
                     "https://KNOX_GW_DOMAIN"));
@@ -170,6 +171,10 @@ public class WebSSOResourceTest {
                     "https://google.com/https://KNOX_GW_DOMAIN"));
   }
 
+  @Test(expected = MalformedURLException.class)
+  public void testMalformedOriginalUrl() throws MalformedURLException {
+            RegExUtils.checkBaseUrlAgainstWhitelist(".*", "https://localhost:5003gateway/homepage/home/");
+  }
 
   private void configureCommonExpectations(Map<String, String> contextExpectations) throws Exception {
     configureCommonExpectations(contextExpectations, false, false, true);
diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/SpiGatewayMessages.java b/gateway-spi/src/main/java/org/apache/knox/gateway/SpiGatewayMessages.java
index b8745474a..4eaa6cfac 100644
--- a/gateway-spi/src/main/java/org/apache/knox/gateway/SpiGatewayMessages.java
+++ b/gateway-spi/src/main/java/org/apache/knox/gateway/SpiGatewayMessages.java
@@ -114,4 +114,7 @@ public interface SpiGatewayMessages {
 
   @Message( level = MessageLevel.INFO, text = "HTTP client retry non safe request is set to {0} for {1}" )
   void setRetryNonIndependent(boolean retryNonIndependent, String serviceRole);
+
+  @Message( level = MessageLevel.DEBUG, text = "Malformed dispatch URL: {0}" )
+  void malformedDispatchUrl(String url);
 }
diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilter.java b/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilter.java
index 2b56a0660..870d3f788 100644
--- a/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilter.java
+++ b/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/GatewayDispatchFilter.java
@@ -32,6 +32,7 @@ import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
+import java.net.MalformedURLException;
 import java.net.URISyntaxException;
 import java.util.Collections;
 import java.util.HashMap;
@@ -140,7 +141,12 @@ public class GatewayDispatchFilter extends AbstractGatewayFilter {
       }
 
       if (whitelist != null) {
-        isAllowed = RegExUtils.checkBaseUrlAgainstWhitelist(whitelist, request.getRequestURI());
+        try {
+          isAllowed = RegExUtils.checkBaseUrlAgainstWhitelist(whitelist, request.getRequestURI());
+        } catch (MalformedURLException e) {
+          LOG.malformedDispatchUrl(request.getRequestURI());
+          isAllowed = false;
+        }
 
         if (!isAllowed) {
           LOG.dispatchDisallowed(request.getRequestURI());
diff --git a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/RegExUtils.java b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/RegExUtils.java
index 7a644fdc0..06a01414b 100644
--- a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/RegExUtils.java
+++ b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/RegExUtils.java
@@ -45,7 +45,7 @@ public class RegExUtils {
     return false;
   }
 
-  public static boolean checkBaseUrlAgainstWhitelist(String whitelist, String fullUrl) {
+  public static boolean checkBaseUrlAgainstWhitelist(String whitelist, String fullUrl) throws MalformedURLException {
     String decodedURL = fullUrl;
     try {
       decodedURL = URLDecoder.decode(fullUrl, StandardCharsets.UTF_8.name());
@@ -53,12 +53,8 @@ public class RegExUtils {
       //
     }
     String baseUrl;
-    try {
-      URL url = new URL(decodedURL);
-      baseUrl = new URL(url.getProtocol(), url.getHost(), url.getPort(), "").toString();
-    } catch (MalformedURLException e) {
-      throw new RuntimeException(e);
-    }
+    URL url = new URL(decodedURL);
+    baseUrl = new URL(url.getProtocol(), url.getHost(), url.getPort(), "").toString();
     return checkWhitelist(whitelist, baseUrl);
   }