Avoid too-clever method that removes port 443 for https

2 files changed, 53 insertions, 17 deletions

diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java
index 86aa9b994f9..f24f2f62771 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java
@@ -5,7 +5,6 @@ import com.yahoo.jdisc.Response;
 import com.yahoo.jdisc.http.HttpRequest;
 import com.yahoo.jdisc.http.servlet.ServletRequest;
 import com.yahoo.jdisc.service.CurrentContainer;
-import org.eclipse.jetty.util.URIUtil;
 import org.eclipse.jetty.util.Utf8Appendable;
 
 import javax.servlet.http.HttpServletRequest;
@@ -14,6 +13,7 @@ import java.net.URI;
 import java.security.cert.X509Certificate;
 import java.util.Enumeration;
 
+import static com.yahoo.jdisc.Response.Status.BAD_REQUEST;
 import static com.yahoo.jdisc.http.core.HttpServletRequestUtils.getConnection;
 import static com.yahoo.jdisc.http.core.HttpServletRequestUtils.getConnectorLocalPort;
 
@@ -48,16 +48,15 @@ class HttpRequestFactory {
             String path = servletRequest.getRequestURI();
             String query = servletRequest.getQueryString();
 
-            StringBuffer builder = new StringBuffer(128);
-            URIUtil.appendSchemeHostPort(builder, scheme, host, port);
-            builder.append(path);
-            if (query != null) {
-                builder.append('?').append(query);
-            }
-            URI uri = URI.create(builder.toString());
+            URI uri = URI.create(scheme + "://" +
+                                 host + ":" + port +
+                                 (path != null ? path : "") +
+                                 (query != null ? "?" + query : ""));
+
             validateSchemeHostPort(scheme, host, port, uri);
             return uri;
-        } catch (IllegalArgumentException e) {
+        }
+        catch (IllegalArgumentException e) {
             throw createBadQueryException(e);
         }
     }
@@ -66,15 +65,12 @@ class HttpRequestFactory {
         if ( ! scheme.equals(uri.getScheme()))
             throw new IllegalArgumentException("Bad scheme: " + scheme);
 
-        if ( ! host.equals(uri.getHost()))
-            throw new IllegalArgumentException("Bad host: " + host);
-
-        if (port != uri.getPort() && ! (port == 80 && scheme.equals("http")) && ! (port == 443 && scheme.equals("https")))
-            throw new IllegalArgumentException("Bad port: " + port);
+        if ( ! host.equals(uri.getHost()) || port != uri.getPort())
+            throw new IllegalArgumentException("Bad authority: " + uri.getRawAuthority() + " != " + host + ":" + port);
     }
 
     private static RequestException createBadQueryException(IllegalArgumentException e) {
-        return new RequestException(Response.Status.BAD_REQUEST, "Query violates RFC 2396: " + e.getMessage(), e);
+        return new RequestException(BAD_REQUEST, "URL violates RFC 2396: " + e.getMessage(), e);
     }
 
     public static void copyHeaders(HttpServletRequest from, HttpRequest to) {
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactoryTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactoryTest.java
index 588bbaf73fb..984d93042ab 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactoryTest.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactoryTest.java
@@ -20,6 +20,7 @@ import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
@@ -33,7 +34,46 @@ public class HttpRequestFactoryTest {
     private static final int LOCAL_PORT = 80;
 
     @Test
-    public final void testIllegalQuery() {
+    public void testLegalURIs() {
+        {
+            URI uri = HttpRequestFactory.getUri(createMockRequest("https", "host", null, null));
+            assertEquals("https", uri.getScheme());
+            assertEquals("host", uri.getHost());
+            assertEquals("", uri.getRawPath());
+            assertNull(uri.getRawQuery());
+        }
+        {
+            URI uri = HttpRequestFactory.getUri(createMockRequest("https", "host", "", ""));
+            assertEquals("https", uri.getScheme());
+            assertEquals("host", uri.getHost());
+            assertEquals("", uri.getRawPath());
+            assertEquals("", uri.getRawQuery());
+        }
+        {
+            URI uri = HttpRequestFactory.getUri(createMockRequest("http", "host.a1-2-3", "", ""));
+            assertEquals("http", uri.getScheme());
+            assertEquals("host.a1-2-3", uri.getHost());
+            assertEquals("", uri.getRawPath());
+            assertEquals("", uri.getRawQuery());
+        }
+        {
+            URI uri = HttpRequestFactory.getUri(createMockRequest("https", "host", "/:1/../1=.", ""));
+            assertEquals("https", uri.getScheme());
+            assertEquals("host", uri.getHost());
+            assertEquals("/:1/../1=.", uri.getRawPath());
+            assertEquals("", uri.getRawQuery());
+        }
+        {
+            URI uri = HttpRequestFactory.getUri(createMockRequest("https", "host", "", "a=/../&?="));
+            assertEquals("https", uri.getScheme());
+            assertEquals("host", uri.getHost());
+            assertEquals("", uri.getRawPath());
+            assertEquals("a=/../&?=", uri.getRawQuery());
+        }
+    }
+
+    @Test
+    public void testIllegalQuery() {
         try {
             HttpRequestFactory.newJDiscRequest(
                     new MockContainer(),
@@ -89,7 +129,7 @@ public class HttpRequestFactoryTest {
             fail("Above statement should throw");
         } catch (RequestException e) {
             assertThat(e.getResponseStatus(), is(Response.Status.BAD_REQUEST));
-            assertThat(e.getMessage(), equalTo("Query violates RFC 2396: Not valid UTF8! byte C0 in state 0"));
+            assertThat(e.getMessage(), equalTo("URL violates RFC 2396: Not valid UTF8! byte C0 in state 0"));
         }
     }