Issue #3537 - Bootstrapping WebSockets with HTTP/2.

Initial draft for the implementation of RFC 8441. Works and passes a single test, but more work is needed to polish the implementation and cover error cases. Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
jetty · Jun 5, 2019 · a485f03 · a485f03
1 parent db43840
commit a485f03
Show file tree

Hide file tree

Showing 25 changed files with 827 additions and 260 deletions.
diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java
@@ -87,6 +87,7 @@ public class HttpRequest implements Request
     private List<RequestListener> requestListeners;
     private BiFunction<Request, Request, Response.CompleteListener> pushListener;
     private Supplier<HttpFields> trailers;
+    private String upgradeProtocol;
 
     protected HttpRequest(HttpClient client, HttpConversation conversation, URI uri)
     {
@@ -607,6 +608,12 @@ public HttpRequest trailers(Supplier<HttpFields> trailers)
         return this;
     }
 
+    public HttpRequest upgradeProtocol(String upgradeProtocol)
+    {
+        this.upgradeProtocol = upgradeProtocol;
+        return this;
+    }
+
     @Override
     public ContentProvider getContent()
     {
@@ -763,6 +770,11 @@ public Supplier<HttpFields> getTrailers()
         return trailers;
     }
 
+    public String getUpgradeProtocol()
+    {
+        return upgradeProtocol;
+    }
+
     @Override
     public boolean abort(Throwable cause)
     {

diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpUpgrader.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpUpgrader.java
@@ -0,0 +1,34 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.client;
+
+import org.eclipse.jetty.http.HttpVersion;
+import org.eclipse.jetty.io.EndPoint;
+
+public interface HttpUpgrader
+{
+    public void prepare(HttpRequest request);
+
+    public void upgrade(HttpResponse response, EndPoint endPoint);
+
+    public interface Factory
+    {
+        public HttpUpgrader newHttpUpgrader(HttpVersion version);
+    }
+}
diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java b/jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java
@@ -28,11 +28,14 @@
 import org.eclipse.jetty.client.HttpConnection;
 import org.eclipse.jetty.client.HttpDestination;
 import org.eclipse.jetty.client.HttpExchange;
+import org.eclipse.jetty.client.HttpRequest;
+import org.eclipse.jetty.client.HttpUpgrader;
 import org.eclipse.jetty.client.IConnection;
 import org.eclipse.jetty.client.SendFailure;
 import org.eclipse.jetty.client.api.Connection;
 import org.eclipse.jetty.client.api.Request;
 import org.eclipse.jetty.client.api.Response;
+import org.eclipse.jetty.http.HttpVersion;
 import org.eclipse.jetty.io.AbstractConnection;
 import org.eclipse.jetty.io.EndPoint;
 import org.eclipse.jetty.util.Promise;
@@ -256,6 +259,18 @@ public SendFailure send(HttpExchange exchange)
             return send(channel, exchange);
         }
 
+        @Override
+        protected void normalizeRequest(Request request)
+        {
+            super.normalizeRequest(request);
+            if (request instanceof HttpUpgrader.Factory)
+            {
+                HttpUpgrader upgrader = ((HttpUpgrader.Factory)request).newHttpUpgrader(HttpVersion.HTTP_1_1);
+                request.attribute(HttpUpgrader.class.getName(), upgrader);
+                upgrader.prepare((HttpRequest)request);
+            }
+        }
+
         @Override
         public void close()
         {

diff --git a/...-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java b/...-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java
@@ -32,7 +32,9 @@
 import org.eclipse.jetty.client.HttpDestination;
 import org.eclipse.jetty.client.HttpExchange;
 import org.eclipse.jetty.client.HttpRequest;
+import org.eclipse.jetty.client.HttpUpgrader;
 import org.eclipse.jetty.client.SendFailure;
+import org.eclipse.jetty.client.api.Request;
 import org.eclipse.jetty.http.HttpVersion;
 import org.eclipse.jetty.http2.ErrorCode;
 import org.eclipse.jetty.http2.IStream;
@@ -88,6 +90,18 @@ public SendFailure send(HttpExchange exchange)
         return send(channel, exchange);
     }
 
+    @Override
+    protected void normalizeRequest(Request request)
+    {
+        super.normalizeRequest(request);
+        if (request instanceof HttpUpgrader.Factory)
+        {
+            HttpUpgrader upgrader = ((HttpUpgrader.Factory)request).newHttpUpgrader(HttpVersion.HTTP_2);
+            request.attribute(HttpUpgrader.class.getName(), upgrader);
+            upgrader.prepare((HttpRequest)request);
+        }
+    }
+
     protected HttpChannelOverHTTP2 acquireHttpChannel()
     {
         HttpChannelOverHTTP2 channel = idleChannels.poll();

diff --git a/...nt-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java b/...nt-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java
@@ -31,6 +31,7 @@
 import org.eclipse.jetty.client.HttpReceiver;
 import org.eclipse.jetty.client.HttpRequest;
 import org.eclipse.jetty.client.HttpResponse;
+import org.eclipse.jetty.client.HttpUpgrader;
 import org.eclipse.jetty.client.api.Request;
 import org.eclipse.jetty.client.api.Response;
 import org.eclipse.jetty.http.HttpField;
@@ -98,7 +99,7 @@ void onHeaders(Stream stream, HeadersFrame frame)
                 if (isTunnel(exchange))
                 {
                     HttpRequest httpRequest = exchange.getRequest();
-                    ClientHTTP2StreamEndPoint endPoint = new ClientHTTP2StreamEndPoint((IStream)stream);
+                    EndPoint endPoint = new ClientHTTP2StreamEndPoint((IStream)stream);
                     long idleTimeout = httpRequest.getIdleTimeout();
                     if (idleTimeout < 0)
                         idleTimeout = getHttpDestination().getHttpClient().getIdleTimeout();
@@ -107,6 +108,12 @@ void onHeaders(Stream stream, HeadersFrame frame)
                         LOG.debug("Successful HTTP2 tunnel on {} via {}", stream, endPoint);
                     ((IStream)stream).setAttachment(endPoint);
                     httpRequest.getConversation().setAttribute(EndPoint.class.getName(), endPoint);
+
+                    // TODO: can we make this similar to the proxy case where the after tunnel
+                    //  logic is done in the onHeaders listener, rather than here?
+                    HttpUpgrader upgrader = (HttpUpgrader)httpRequest.getAttributes().get(HttpUpgrader.class.getName());
+                    if (upgrader != null)
+                        upgrader.upgrade(httpResponse, endPoint);
                 }
 
                 if (responseHeaders(exchange))

diff --git a/...ient-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpSenderOverHTTP2.java b/...ient-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpSenderOverHTTP2.java
@@ -58,7 +58,18 @@ protected void sendHeaders(HttpExchange exchange, final HttpContent content, fin
         MetaData.Request metaData;
         if (isTunnel)
         {
-            metaData = new MetaData.Request(request.getMethod(), null, new HostPortHttpField(request.getPath()), null, HttpVersion.HTTP_2, request.getHeaders());
+            String upgradeProtocol = request.getUpgradeProtocol();
+            if (upgradeProtocol == null)
+            {
+                metaData = new MetaData.Request(request.getMethod(), null, new HostPortHttpField(request.getPath()), null, HttpVersion.HTTP_2, request.getHeaders());
+            }
+            else
+            {
+                // TODO: merge with below? needs relativize()?
+                HttpURI uri = HttpURI.createHttpURI(request.getScheme(), request.getHost(), request.getPort(), request.getPath(), null, request.getQuery(), null);
+                metaData = new MetaData.Request(request.getMethod(), uri, HttpVersion.HTTP_2, request.getHeaders());
+                metaData.setProtocol(upgradeProtocol);
+            }
         }
         else
         {

diff --git a/...ttp2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnection.java b/...ttp2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnection.java
@@ -372,7 +372,7 @@ public Runnable onRequest(HeadersFrame frame)
         }
 
         @Override
-        protected void prepareUpgrade()
+        protected void tryUpgrade()
         {
             if (isTunnel())
                 getHttpTransport().prepareUpgrade();

diff --git a/...tp2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java b/...tp2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java
@@ -305,6 +305,8 @@ public void onCompleted()
         Object attachment = stream.getAttachment();
         if (attachment instanceof HttpChannelOverHTTP2)
         {
+            // TODO: we used to "fake" a 101 response to upgrade the endpoint
+            //  but we don't anymore, so this code should be deleted.
             HttpChannelOverHTTP2 channel = (HttpChannelOverHTTP2)attachment;
             if (channel.getResponse().getStatus() == HttpStatus.SWITCHING_PROTOCOLS_101)
             {

diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
@@ -505,7 +505,8 @@ public boolean handle()
                                 if (hasContent && !_response.isContentComplete(_response.getHttpOutput().getWritten()))
                                     sendErrorOrAbort("Insufficient content written");
                             }
-                            prepareUpgrade();
+                            // TODO: why tryUpgrade() is before closeOutput()? Add a comment for clarity.
+                            tryUpgrade();
                             _response.closeOutput();
                         }
                         finally
@@ -751,7 +752,7 @@ public boolean onRequestComplete()
         return result;
     }
 
-    protected void prepareUpgrade()
+    protected void tryUpgrade()
     {
     }
 

diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java
@@ -408,6 +408,12 @@ boolean onIdleTimeout(Throwable timeout)
         return true;
     }
 
+    @Override
+    protected void tryUpgrade()
+    {
+        // TODO: move the code from HttpConnection.upgrade() here?
+    }
+
     /**
      * <p>Attempts to perform a HTTP/1.1 upgrade.</p>
      * <p>The upgrade looks up a {@link ConnectionFactory.Upgrading} from the connector

diff --git a/...ket/jetty-websocket-api/src/main/java/org/eclipse/jetty/websocket/api/UpgradeRequest.java b/...ket/jetty-websocket-api/src/main/java/org/eclipse/jetty/websocket/api/UpgradeRequest.java
@@ -125,7 +125,6 @@ public interface UpgradeRequest
      * @return the HTTP method used
      */
     String getMethod();
-
     /**
      * The WebSocket Origin of this Upgrade Request
      * <p>
@@ -255,34 +254,6 @@ public interface UpgradeRequest
      */
     void setHeaders(Map<String, List<String>> headers);
 
-    /**
-     * Set the HTTP Version to use.
-     * <p>
-     * As of <a href="http://tools.ietf.org/html/rfc6455">RFC6455 (December 2011)</a> this should always be
-     * {@code HTTP/1.1}
-     *
-     * @param httpVersion the HTTP version to use.
-     */
-    void setHttpVersion(String httpVersion);
-
-    /**
-     * Set the HTTP method to use.
-     * <p>
-     * As of <a href="http://tools.ietf.org/html/rfc6455">RFC6455 (December 2011)</a> this is always {@code GET}
-     *
-     * @param method the HTTP method to use.
-     */
-    void setMethod(String method);
-
-    /**
-     * Set the Request URI to use for this request.
-     * <p>
-     * Must be an absolute URI with scheme {@code 'ws'} or {@code 'wss'}
-     *
-     * @param uri the Request URI
-     */
-    void setRequestURI(URI uri);
-
     /**
      * Set the Session associated with this request.
      * <p>

diff --git a/...bsocket-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeRequest.java b/...bsocket-client/src/main/java/org/eclipse/jetty/websocket/client/ClientUpgradeRequest.java
@@ -18,10 +18,6 @@
 
 package org.eclipse.jetty.websocket.client;
 
-import org.eclipse.jetty.http.HttpHeader;
-import org.eclipse.jetty.websocket.api.UpgradeRequest;
-import org.eclipse.jetty.websocket.api.extensions.ExtensionConfig;
-
 import java.net.HttpCookie;
 import java.net.URI;
 import java.security.Principal;
@@ -30,8 +26,14 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.TreeMap;
 
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.HttpScheme;
+import org.eclipse.jetty.websocket.api.UpgradeRequest;
+import org.eclipse.jetty.websocket.api.extensions.ExtensionConfig;
+
 /**
  * Client based UpgradeRequest API
  */
@@ -46,7 +48,6 @@ public final class ClientUpgradeRequest implements UpgradeRequest
     private String httpVersion;
     private String method;
     private String host;
-    private boolean secure;
 
     public ClientUpgradeRequest()
     {
@@ -57,12 +58,9 @@ public ClientUpgradeRequest(URI uri)
     {
         this.requestURI = uri;
         String scheme = uri.getScheme();
-        if (!"ws".equalsIgnoreCase(scheme) && !"wss".equalsIgnoreCase(scheme))
-        {
+        if (!HttpScheme.WS.is(scheme) || !HttpScheme.WSS.is(scheme))
             throw new IllegalArgumentException("URI scheme must be 'ws' or 'wss'");
-        }
         this.host = this.requestURI.getHost();
-        this.parameters.clear();
     }
 
     @Override
@@ -193,11 +191,7 @@ public Map<String, List<String>> getParameterMap()
     public String getProtocolVersion()
     {
         String version = getHeader("Sec-WebSocket-Version");
-        if (version == null)
-        {
-            return "13"; // Default
-        }
-        return version;
+        return Objects.requireNonNullElse(version, "13");
     }
 
     @Override
@@ -278,8 +272,7 @@ public void setHeader(String name, String value)
     @Override
     public void setHeaders(Map<String, List<String>> headers)
     {
-        headers.clear();
-
+        this.headers.clear();
         for (Map.Entry<String, List<String>> entry : headers.entrySet())
         {
             String name = entry.getKey();
@@ -288,24 +281,6 @@ public void setHeaders(Map<String, List<String>> headers)
         }
     }
 
-    @Override
-    public void setHttpVersion(String httpVersion)
-    {
-        this.httpVersion = httpVersion;
-    }
-
-    @Override
-    public void setMethod(String method)
-    {
-        this.method = method;
-    }
-
-    @Override
-    public void setRequestURI(URI uri)
-    {
-        this.requestURI = uri;
-    }
-
     @Override
     public void setSession(Object session)
     {

diff --git a/...main/java/org/eclipse/jetty/websocket/client/impl/DelegatedJettyClientUpgradeRequest.java b/...main/java/org/eclipse/jetty/websocket/client/impl/DelegatedJettyClientUpgradeRequest.java
@@ -231,24 +231,6 @@ public void setHeaders(Map<String, List<String>> headers)
         // TODO
     }
 
-    @Override
-    public void setHttpVersion(String httpVersion)
-    {
-        // TODO
-    }
-
-    @Override
-    public void setMethod(String method)
-    {
-
-    }
-
-    @Override
-    public void setRequestURI(URI uri)
-    {
-        // TODO
-    }
-
     @Override
     public void setSession(Object session)
     {