Merge pull request #785 from jaikiran/undertow-1565

UNDERTOW-1565 Don't encode query strings and URI paths while redirecting
undertow-io · Jul 29, 2019 · 112d763 · 112d763
2 parents fa9263d + 37b4f3c
commit 112d763
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 17 deletions.
diff --git a/core/src/main/java/io/undertow/security/handlers/SinglePortConfidentialityHandler.java b/core/src/main/java/io/undertow/security/handlers/SinglePortConfidentialityHandler.java
@@ -43,10 +43,12 @@ protected URI getRedirectURI(HttpServerExchange exchange) throws URISyntaxExcept
         return getRedirectURI(exchange, redirectPort);
     }
 
-    protected URI getRedirectURI(HttpServerExchange exchange, int port) throws URISyntaxException {
-        String host = exchange.getHostName();
-
-        String queryString = exchange.getQueryString();
+    protected URI getRedirectURI(final HttpServerExchange exchange, final int port) throws URISyntaxException {
+        final StringBuilder uriBuilder = new StringBuilder();
+        uriBuilder.append("https://").append(exchange.getHostName());
+        if (port > 0) {
+            uriBuilder.append(":").append(port);
+        }
         String uri = exchange.getRequestURI();
         if(exchange.isHostIncludedInRequestURI()) {
             int slashCount = 0;
@@ -60,8 +62,12 @@ protected URI getRedirectURI(HttpServerExchange exchange, int port) throws URISy
                 }
             }
         }
-        return new URI("https", null, host, port, uri,
-                queryString == null || queryString.length() == 0 ? null : queryString, null);
+        uriBuilder.append(uri);
+        final String queryString = exchange.getQueryString();
+        if (queryString != null && !queryString.isEmpty()) {
+            uriBuilder.append("?").append(queryString);
+        }
+        return new URI(uriBuilder.toString());
     }
 
 }
diff --git a/core/src/test/java/io/undertow/server/security/SimpleConfidentialRedirectTestCase.java b/core/src/test/java/io/undertow/server/security/SimpleConfidentialRedirectTestCase.java
@@ -49,6 +49,8 @@
 public class SimpleConfidentialRedirectTestCase {
 
 
+    private static int redirectPort = -1;
+
     @BeforeClass
     public static void setup() throws IOException {
         DefaultServer.startSSLServer();
@@ -58,11 +60,13 @@ public static void setup() throws IOException {
             public void handleRequest(final HttpServerExchange exchange) throws Exception {
                 exchange.getResponseHeaders().put(HttpString.tryFromString("scheme"), exchange.getRequestScheme());
                 exchange.getResponseHeaders().put(HttpString.tryFromString("uri"), exchange.getRequestURI());
+                exchange.getResponseHeaders().put(HttpString.tryFromString("queryString"), exchange.getQueryString());
+                exchange.getResponseHeaders().put(HttpString.tryFromString("redirectedToPort"), exchange.getHostPort());
                 exchange.endExchange();
             }
         };
-
-        current = new SinglePortConfidentialityHandler(current, DefaultServer.getHostSSLPort("default"));
+        redirectPort = DefaultServer.getHostSSLPort("default");
+        current = new SinglePortConfidentialityHandler(current, redirectPort);
 
         DefaultServer.setRootHandler(current);
     }
@@ -77,11 +81,11 @@ public void simpleRedirectTestCase() throws IOException, GeneralSecurityExceptio
         TestHttpClient client = new TestHttpClient();
         client.setSSLContext(DefaultServer.getClientSSLContext());
         try {
-            sendRequest(client, "/foo");
-            sendRequest(client, "/foo+bar");
-            sendRequest(client, "/foo+bar;aa");
-
-
+            sendRequest(client, "/foo", null);
+            sendRequest(client, "/foo+bar", null);
+            sendRequest(client, "/foo+bar;aa", null);
+            sendRequest(client, "/foo+bar;aa", "x=y");
+            sendRequest(client, "/foo+bar%3Aaa", "x=%3Ablah");
         } finally {
             client.getConnectionManager().shutdown();
         }
@@ -99,12 +103,21 @@ public void testRedirectWithFullURLInPath() throws IOException {
         }
     }
 
-    private void sendRequest(TestHttpClient client, String uri) throws IOException {
-        HttpGet get = new HttpGet(DefaultServer.getDefaultServerURL() + uri);
+    private void sendRequest(final TestHttpClient client, final String uri, final String queryString) throws IOException {
+        String targetURL = DefaultServer.getDefaultServerURL() + uri;
+        if (queryString != null) {
+            targetURL = targetURL + "?" + queryString;
+        }
+        final HttpGet get = new HttpGet(targetURL);
         HttpResponse result = client.execute(get);
         Assert.assertEquals(StatusCodes.OK, result.getStatusLine().getStatusCode());
-        Assert.assertEquals("https", result.getFirstHeader("scheme").getValue());
-        Assert.assertEquals(uri, result.getFirstHeader("uri").getValue());
+        Assert.assertEquals("Unexpected scheme in redirected URI", "https", result.getFirstHeader("scheme").getValue());
+        Assert.assertEquals("Unexpected port in redirected URI", String.valueOf(redirectPort), result.getFirstHeader("redirectedToPort").getValue());
+        Assert.assertEquals("Unexpected path in redirected URI", uri, result.getFirstHeader("uri").getValue());
+        if (queryString != null) {
+            Assert.assertEquals("Unexpected query string in redirected URI", queryString,
+                    result.getFirstHeader("queryString").getValue());
+        }
         HttpClientUtils.readResponse(result);
     }