Revert "Bjorncs/upgrade netty"

6 files changed, 76 insertions, 36 deletions

diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/Cookie.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/Cookie.java
index 8dcf61911dd..874bf35021b 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/Cookie.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/Cookie.java
@@ -1,6 +1,10 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.jdisc.http;
 
+import org.jboss.netty.handler.codec.http.CookieDecoder;
+import org.jboss.netty.handler.codec.http.CookieEncoder;
+import org.jboss.netty.handler.codec.http.DefaultCookie;
+
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
@@ -10,7 +14,6 @@ import java.util.concurrent.TimeUnit;
 /**
  * @author <a href="mailto:einarmr@yahoo-inc.com">Einar M R Rosenvinge</a>
  */
-@SuppressWarnings("deprecation")
 public class Cookie {
 
     private final Set<Integer> ports = new HashSet<>();
@@ -237,11 +240,10 @@ public class Cookie {
     }
 
     private static String encodeCookies(Iterable<? extends Cookie> cookies, boolean server) {
-        org.jboss.netty.handler.codec.http.CookieEncoder encoder =
-                new org.jboss.netty.handler.codec.http.CookieEncoder(server);
+        CookieEncoder encoder = new org.jboss.netty.handler.codec.http.CookieEncoder(server);
         for (Cookie cookie : cookies) {
             org.jboss.netty.handler.codec.http.Cookie nettyCookie =
-                    new org.jboss.netty.handler.codec.http.DefaultCookie(String.valueOf(cookie.getName()), String.valueOf(cookie.getValue()));
+                    new DefaultCookie(String.valueOf(cookie.getName()), String.valueOf(cookie.getValue()));
             nettyCookie.setComment(cookie.getComment());
             nettyCookie.setCommentUrl(cookie.getCommentUrl());
             nettyCookie.setDiscard(cookie.isDiscard());
@@ -258,8 +260,7 @@ public class Cookie {
     }
 
     private static List<Cookie> decodeCookies(String str) {
-        org.jboss.netty.handler.codec.http.CookieDecoder decoder =
-                new org.jboss.netty.handler.codec.http.CookieDecoder();
+        CookieDecoder decoder = new CookieDecoder();
         List<Cookie> ret = new LinkedList<>();
         for (org.jboss.netty.handler.codec.http.Cookie nettyCookie : decoder.decode(str)) {
             Cookie cookie = new Cookie();
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpRequest.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpRequest.java
index 6c4759915b2..ff80a8a845d 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpRequest.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpRequest.java
@@ -9,14 +9,12 @@ import com.yahoo.jdisc.handler.RequestHandler;
 import com.yahoo.jdisc.handler.ResponseHandler;
 import com.yahoo.jdisc.http.servlet.ServletOrJdiscHttpRequest;
 import com.yahoo.jdisc.service.CurrentContainer;
-import org.eclipse.jetty.http.HttpURI;
-import org.eclipse.jetty.util.MultiMap;
+import org.jboss.netty.handler.codec.http.QueryStringDecoder;
 
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.net.URI;
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.LinkedList;
@@ -86,7 +84,7 @@ public class HttpRequest extends Request implements ServletOrJdiscHttpRequest {
             this.method = method;
             this.version = version;
             this.remoteAddress = remoteAddress;
-            this.parameters.putAll(getUriQueryParameters(uri));
+            this.parameters.putAll(new QueryStringDecoder(uri.toString(), true).getParameters());
             if (connectedAtMillis != null) {
                 this.connectedAt = connectedAtMillis;
             } else {
@@ -104,7 +102,7 @@ public class HttpRequest extends Request implements ServletOrJdiscHttpRequest {
             this.method = method;
             this.version = version;
             this.remoteAddress = null;
-            this.parameters.putAll(getUriQueryParameters(uri));
+            this.parameters.putAll(new QueryStringDecoder(uri.toString(), true).getParameters());
             this.connectedAt = creationTime(TimeUnit.MILLISECONDS);
         } catch (RuntimeException e) {
             release();
@@ -112,18 +110,6 @@ public class HttpRequest extends Request implements ServletOrJdiscHttpRequest {
         }
     }
 
-    private static Map<String, List<String>> getUriQueryParameters(URI uri) {
-        MultiMap<String> queryParameters = new MultiMap<>();
-        new HttpURI(uri).decodeQueryTo(queryParameters);
-
-        // Do a deep copy so we do not leak Jetty classes outside
-        Map<String, List<String>> deepCopiedQueryParameters = new HashMap<>();
-        for (Map.Entry<String, List<String>> entry : queryParameters.entrySet()) {
-            deepCopiedQueryParameters.put(entry.getKey(), new ArrayList<>(entry.getValue()));
-        }
-        return deepCopiedQueryParameters;
-    }
-
     public Method getMethod() {
         return method;
     }
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
index 837f21519f2..300cf13bfa6 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
@@ -70,6 +70,7 @@ public class JettyHttpServer extends AbstractServerProvider {
         String NAME_DIMENSION = "serverName";
         String PORT_DIMENSION = "serverPort";
 
+        String NUM_ACTIVE_REQUESTS = "serverNumActiveRequests";
         String NUM_OPEN_CONNECTIONS = "serverNumOpenConnections";
         String NUM_CONNECTIONS_OPEN_MAX = "serverConnectionsOpenMax";
         String CONNECTION_DURATION_MAX = "serverConnectionDurationMax";
@@ -82,6 +83,8 @@ public class JettyHttpServer extends AbstractServerProvider {
         String MANHATTAN_NUM_BYTES_SENT     = "http.out.bytes";
 
         String NUM_CONNECTIONS = "serverNumConnections";
+        String NUM_CONNECTIONS_IDLE = "serverNumConnectionsIdle";
+        String NUM_UNEXPECTED_DISCONNECTS = "serverNumUnexpectedDisconnects";
 
         /* For historical reasons, these are all aliases for the same metric. 'jdisc.http' should ideally be the only one. */
         String JDISC_HTTP_REQUESTS = "jdisc.http.requests";
@@ -93,6 +96,7 @@ public class JettyHttpServer extends AbstractServerProvider {
         String NUM_SUCCESSFUL_WRITES = "serverNumSuccessfulResponseWrites";
         String NUM_FAILED_WRITES = "serverNumFailedResponseWrites";
 
+        String NETWORK_LATENCY = "serverNetworkLatency";
         String TOTAL_SUCCESSFUL_LATENCY = "serverTotalSuccessfulResponseLatency";
         String MANHATTAN_TOTAL_SUCCESSFUL_LATENCY = "http.latency";
         String TOTAL_FAILED_LATENCY = "serverTotalFailedResponseLatency";
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/CookieTestCase.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/CookieTestCase.java
index eabec6cd9e9..de2b0d453e6 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/CookieTestCase.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/CookieTestCase.java
@@ -1,6 +1,8 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.jdisc.http;
 
+import org.jboss.netty.handler.codec.http.CookieDecoder;
+import org.jboss.netty.handler.codec.http.DefaultCookie;
 import org.testng.annotations.Test;
 
 import java.util.Arrays;
@@ -21,13 +23,12 @@ import static org.testng.AssertJUnit.fail;
 /**
  * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen Hult</a>
  */
-@SuppressWarnings("deprecation")
 public class CookieTestCase {
 
     @Test
     public void requireThatDefaultValuesAreSane() {
-        assertCookie(new org.jboss.netty.handler.codec.http.DefaultCookie("foo", "bar"), new Cookie().setName("foo").setValue("bar"));
-        assertCookie(new org.jboss.netty.handler.codec.http.DefaultCookie("foo", "bar"), new Cookie("foo", "bar"));
+        assertCookie(new DefaultCookie("foo", "bar"), new Cookie().setName("foo").setValue("bar"));
+        assertCookie(new DefaultCookie("foo", "bar"), new Cookie("foo", "bar"));
     }
 
     @Test
@@ -133,17 +134,20 @@ public class CookieTestCase {
     @Test
     public void requireThatCookieCanBeEncoded() {
         assertEncodeCookie(
-                Collections.singletonList("foo.name=foo.value"),
+                Collections.singletonList("$Version=1; foo.name=foo.value; $Path=path; $Domain=domain; $Port=\"69\""),
                 Collections.singletonList(newCookie("foo")));
         assertEncodeCookie(
-                Collections.singletonList("bar.name=bar.value; foo.name=foo.value"),
+                Arrays.asList("$Version=1; bar.name=bar.value; $Path=path; $Domain=domain; $Port=\"69\"",
+                              "$Version=1; foo.name=foo.value; $Path=path; $Domain=domain; $Port=\"69\""),
                 Arrays.asList(newCookie("foo"), newCookie("bar")));
     }
 
     @Test
     public void requireThatSetCookieCanBeEncoded() {
         assertEncodeSetCookie(
-                Collections.singletonList("foo.name=foo.value; Path=path; Domain=domain; Secure; HTTPOnly"),
+                Collections.singletonList("foo.name=foo.value; Max-Age=0; Path=path; Domain=domain; Secure; " +
+                                          "HTTPOnly; Comment=comment; Version=1; CommentURL=\"commentUrl\"; " +
+                                          "Port=\"69\"; Discard"),
                 Collections.singletonList(newCookie("foo")));
     }
 
@@ -210,7 +214,7 @@ public class CookieTestCase {
 
     @Test
     public void requireThatCookieDecoderWorksForGenericValidCookies() {
-        new org.jboss.netty.handler.codec.http.CookieDecoder().decode("Y=v=1&n=8es5opih9ljtk&l=og0_iedeh0qqvqqr/o&p=m2g2rs6012000000&r=pv&lg=en-US&intl=" +
+        new CookieDecoder().decode("Y=v=1&n=8es5opih9ljtk&l=og0_iedeh0qqvqqr/o&p=m2g2rs6012000000&r=pv&lg=en-US&intl=" +
                                    "us&np=1; T=z=h.nzPBhSP4PBVd5JqacVnIbNjU1NAY2TjYzNzVOTjYzNzM0Mj&a=YAE&sk=DAALShmNQ" +
                                    "vhoZV&ks=EAABsibvMK6ejwn0uUoS4rC9w--~E&d=c2wBTVRJeU13RXhPVEUwTURJNU9URTBNRFF6TlRJ" +
                                    "NU5nLS0BYQFZQUUBZwE1VkNHT0w3VUVDTklJVEdRR1FXT0pOSkhEQQFzY2lkAWNOUnZIbEc3ZHZoVHlWZ" +
@@ -219,7 +223,7 @@ public class CookieTestCase {
 
     @Test
     public void requireThatCookieDecoderWorksForYInvalidCookies() {
-        new org.jboss.netty.handler.codec.http.CookieDecoder().decode("Y=v=1&n=77nkr5t7o4nqn&l=og0_iedeh0qqvqqr/o&p=m2g2rs6012000000&r=pv&lg=en-US&intl=" +
+        new CookieDecoder().decode("Y=v=1&n=77nkr5t7o4nqn&l=og0_iedeh0qqvqqr/o&p=m2g2rs6012000000&r=pv&lg=en-US&intl=" +
                                    "us&np=1; T=z=05nzPB0NP4PBN/n0gwc1AWGNjU1NAY2TjYzNzVOTjYzNzM0Mj&a=QAE&sk=DAA4R2svo" +
                                    "osjIa&ks=EAAj3nBQFkN4ZmuhqFxJdNoaQ--~E&d=c2wBTVRJeU13RXhPVEUwTURJNU9URTBNRFF6TlRJ" +
                                    "NU5nLS0BYQFRQUUBZwE1VkNHT0w3VUVDTklJVEdRR1FXT0pOSkhEQQFzY2lkAUpPalRXOEVsUDZrR3RHT" +
@@ -228,7 +232,7 @@ public class CookieTestCase {
 
     @Test
     public void requireThatCookieDecoderWorksForYValidCookies() {
-        new org.jboss.netty.handler.codec.http.CookieDecoder().decode("Y=v=1&n=3767k6te5aj2s&l=1v4u3001uw2ys00q0rw0qrw34q0x5s3u/o&p=030vvit012000000&iz=" +
+        new CookieDecoder().decode("Y=v=1&n=3767k6te5aj2s&l=1v4u3001uw2ys00q0rw0qrw34q0x5s3u/o&p=030vvit012000000&iz=" +
                                    "&r=pu&lg=en-US,it-IT,it&intl=it&np=1; T=z=m38yPBmLk3PBWvehTPBhBHYNU5OBjQ3NE5ONU5P" +
                                    "NDY0NzU0M0&a=IAE&sk=DAAAx5URYgbhQ6&ks=EAA4rTgdlAGeMQmdYeM_VehGg--~E&d=c2wBTWprNUF" +
                                    "UTXdNems1TWprNE16RXpNREl6TkRneAFhAUlBRQFnAUVJSlNMSzVRM1pWNVNLQVBNRkszQTRaWDZBAXNj" +
@@ -238,7 +242,7 @@ public class CookieTestCase {
 
     @Test
     public void requireThatCookieDecoderWorksForGenericInvalidCookies() {
-        new org.jboss.netty.handler.codec.http.CookieDecoder().decode("Y=v=1&n=e92s5cq8qbs6h&l=3kdb0f.3@i126be10b.d4j/o&p=m1f2qgmb13000107&r=g5&lg=en-US" +
+        new CookieDecoder().decode("Y=v=1&n=e92s5cq8qbs6h&l=3kdb0f.3@i126be10b.d4j/o&p=m1f2qgmb13000107&r=g5&lg=en-US" +
                                    "&intl=us; T=z=TXp3OBTrQ8OBFMcj3GBpFSyNk83TgY2MjMwN04zMDMw&a=YAE&sk=DAAVfaNwLeISrX" +
                                    "&ks=EAAOeNNgY8c5hV8YzPYmnrW7w--~E&d=c2wBTVRnd09RRXhOVFEzTURrME56UTMBYQFZQUUBZwFMQ" +
                                    "U5NT0Q2UjY2Q0I1STY0R0tKSUdVQVlRRQFvawFaVzAtAXRpcAFMTlRUdkMBenoBVFhwM09CQTdF&af=QU" +
@@ -278,7 +282,7 @@ public class CookieTestCase {
         }
     }
 
-    private static void assertCookie(final org.jboss.netty.handler.codec.http.DefaultCookie expected, final Cookie actual) {
+    private static void assertCookie(final DefaultCookie expected, final Cookie actual) {
         assertEquals(expected.getName(), actual.getName());
         assertEquals(expected.getValue(), actual.getValue());
         assertEquals(expected.getDomain(), actual.getDomain());
@@ -300,6 +304,7 @@ public class CookieTestCase {
         cookie.setPath("path");
         cookie.setComment("comment");
         cookie.setCommentUrl("commentUrl");
+        cookie.setMaxAge(69, TimeUnit.MILLISECONDS);
         cookie.setVersion(2);
         cookie.setSecure(true);
         cookie.setHttpOnly(true);
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/HttpRequestTestCase.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/HttpRequestTestCase.java
index 53291c850c2..8f26d9c38b0 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/HttpRequestTestCase.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/HttpRequestTestCase.java
@@ -4,6 +4,9 @@ package com.yahoo.jdisc.http;
 import com.yahoo.jdisc.Container;
 import com.yahoo.jdisc.Request;
 import com.yahoo.jdisc.service.CurrentContainer;
+import org.jboss.netty.handler.codec.http.HttpHeaders;
+import org.jboss.netty.handler.codec.http.HttpMethod;
+import org.jboss.netty.handler.codec.http.HttpVersion;
 import org.testng.annotations.Test;
 
 import java.net.InetSocketAddress;
@@ -27,6 +30,27 @@ import static org.testng.AssertJUnit.assertTrue;
 public class HttpRequestTestCase {
 
     @Test
+    public void requireThatMethodIsCompatibleWithNetty() {
+        assertMethod(HttpRequest.Method.OPTIONS, HttpMethod.OPTIONS);
+        assertMethod(HttpRequest.Method.GET, HttpMethod.GET);
+        assertMethod(HttpRequest.Method.HEAD, HttpMethod.HEAD);
+        assertMethod(HttpRequest.Method.POST, HttpMethod.POST);
+        assertMethod(HttpRequest.Method.PUT, HttpMethod.PUT);
+        assertMethod(HttpRequest.Method.PATCH, HttpMethod.PATCH);
+        assertMethod(HttpRequest.Method.DELETE, HttpMethod.DELETE);
+        assertMethod(HttpRequest.Method.TRACE, HttpMethod.TRACE);
+        assertMethod(HttpRequest.Method.CONNECT, HttpMethod.CONNECT);
+        assertEquals(9, HttpRequest.Method.values().length);
+    }
+
+    @Test
+    public void requireThatVersionIsCompatibleWithNetty() {
+        assertVersion(HttpRequest.Version.HTTP_1_0, HttpVersion.HTTP_1_0);
+        assertVersion(HttpRequest.Version.HTTP_1_1, HttpVersion.HTTP_1_1);
+        assertEquals(2, HttpRequest.Version.values().length);
+    }
+
+    @Test
     public void requireThatSimpleServerConstructorsUseReasonableDefaults() {
         final URI uri = URI.create("http://localhost/");
         HttpRequest request = HttpRequest.newServerRequest(mockContainer(), uri);
@@ -190,6 +214,18 @@ public class HttpRequestTestCase {
         assertEquals(cookies, request.decodeCookieHeader());
     }
 
+    private static void assertMethod(final HttpRequest.Method discMethod, final HttpMethod nettyMethod) {
+        assertEquals(discMethod, HttpRequest.Method.valueOf(nettyMethod.getName()));
+        assertEquals(discMethod, HttpRequest.Method.valueOf(nettyMethod.toString()));
+        assertEquals(nettyMethod, HttpMethod.valueOf(discMethod.toString()));
+    }
+
+    private static void assertVersion(final HttpRequest.Version discVersion, final HttpVersion nettyVersion) {
+        assertEquals(discVersion, HttpRequest.Version.fromString(nettyVersion.getText()));
+        assertEquals(discVersion, HttpRequest.Version.fromString(nettyVersion.toString()));
+        assertEquals(nettyVersion, HttpVersion.valueOf(discVersion.toString()));
+    }
+
     private static HttpRequest newRequest(final HttpRequest.Version version) throws Exception {
         return HttpRequest.newServerRequest(
                 mockContainer(),
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java
index 6416d11f523..1fb1f3658ee 100644
--- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java
+++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java
@@ -353,13 +353,21 @@ public class HttpServerTest {
                                             .setDiscard(true)
                                             .setDomain(".localhost")
                                             .setHttpOnly(true)
+                                            .setMaxAge(5000, TimeUnit.SECONDS)
                                             .setPath("/foopath")
                                             .setSecure(true)
                                             .setVersion(2)));
         driver.client().get("/status.html")
               .expectStatusCode(is(OK))
-              .expectHeader("Set-Cookie",
-                      is("foo=bar; Path=/foopath; Domain=.localhost; Secure; HTTPOnly"));
+              .expectHeader("Set-Cookie", is("foo=bar; " +
+                                             "Max-Age=5000; " +
+                                             "Path=\"/foopath\"; " +
+                                             "Domain=.localhost; " +
+                                             "Secure; HTTPOnly; " +
+                                             "Comment=\"comment yeah\"; " +
+                                             "Version=1; " +
+                                             "CommentURL=\"http://comment.yes/\"; " +
+                                             "Discard"));
         assertThat(driver.close(), is(true));
     }