Issue jetty#5104 - Fix protocol version in Via header to work with H2…

… and other protocols Bug: jetty#5104 Signed-off-by: Travis Spencer <[email protected]>
travisspencer · Aug 1, 2020 · 2625ebe · 2625ebe
1 parent 66ec160
commit 2625ebe
Show file tree

Hide file tree

Showing 2 changed files with 114 additions and 1 deletion.
diff --git a/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/AbstractProxyServlet.java b/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/AbstractProxyServlet.java
@@ -167,6 +167,9 @@ public String getHostHeader()
 
     public String getViaHost()
     {
+        if (_viaHost == null)
+            _viaHost = viaHost();
+
         return _viaHost;
     }
 
@@ -509,15 +512,49 @@ protected Set<String> findConnectionHeaders(HttpServletRequest clientRequest)
 
     protected void addProxyHeaders(HttpServletRequest clientRequest, Request proxyRequest)
     {
-        addViaHeader(proxyRequest);
+        addViaHeader(clientRequest, proxyRequest);
         addXForwardedHeaders(clientRequest, proxyRequest);
     }
 
+    /**
+     * Adds the HTTP Via header to the proxied request.
+     *
+     * @deprecated Use {@link #addViaHeader(HttpServletRequest, Request)} instead.
+     * @param proxyRequest the request being proxied
+     */
+    @Deprecated
     protected void addViaHeader(Request proxyRequest)
     {
         proxyRequest.header(HttpHeader.VIA, "http/1.1 " + getViaHost());
     }
 
+    /**
+     * Adds the HTTP Via header to the proxied request, taking into account data present in the client request.
+     * This method considers the protocol of the client request when forming the proxied request. If it
+     * is HTTP, then the protocol name will not be included in the Via header that is sent by the proxy, and only
+     * the protocol version will be sent. If it is not, the entire protocol (name and version) will be included. 
+     * If the client request includes a Via header, the result will be appended to that to form a chain.
+     *
+     * @param clientRequest the client request
+     * @param proxyRequest the request being proxied
+     * @see <a href="https://tools.ietf.org/html/rfc7230#section-5.7.1">RFC 7230 section 5.7.1</a>
+     */
+    protected void addViaHeader(HttpServletRequest clientRequest, Request proxyRequest)
+    {
+        String protocol = clientRequest.getProtocol();
+        String[] parts = protocol.split("/", 2);
+        String protocolName = parts.length == 2 && "HTTP".equals(parts[0]) ? parts[1] : protocol;
+        String viaHeaderValue = "";
+        String clientViaHeader = clientRequest.getHeader(HttpHeader.VIA.name());
+
+        if (clientViaHeader != null)
+            viaHeaderValue = clientViaHeader;
+
+        viaHeaderValue += protocolName + " " + getViaHost();
+
+        proxyRequest.header(HttpHeader.VIA, viaHeaderValue);
+    }
+
     protected void addXForwardedHeaders(HttpServletRequest clientRequest, Request proxyRequest)
     {
         proxyRequest.header(HttpHeader.X_FORWARDED_FOR, clientRequest.getRemoteAddr());

diff --git a/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyServletTest.java b/jetty-proxy/src/test/java/org/eclipse/jetty/proxy/ProxyServletTest.java
@@ -26,6 +26,8 @@
 import java.io.PrintWriter;
 import java.net.ConnectException;
 import java.net.HttpCookie;
+import java.net.InetAddress;
+import java.net.URI;
 import java.nio.ByteBuffer;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -63,6 +65,7 @@
 import org.eclipse.jetty.client.DuplexConnectionPool;
 import org.eclipse.jetty.client.HttpClient;
 import org.eclipse.jetty.client.HttpProxy;
+import org.eclipse.jetty.client.HttpRequest;
 import org.eclipse.jetty.client.api.ContentResponse;
 import org.eclipse.jetty.client.api.Request;
 import org.eclipse.jetty.client.api.Response;
@@ -116,6 +119,15 @@ public static Stream<Arguments> impls()
         ).map(Arguments::of);
     }
 
+    public static Stream<Arguments> implsWithProtocols()
+    {
+        String[] protocols = {"HTTP/1.1", "HTTP/2.0", "OTHER/0.9"};
+
+        return impls()
+                .flatMap(impl -> Arrays.stream(protocols)
+                        .flatMap(p -> Stream.of(Arguments.of(impl.get()[0], p))));
+    }
+
     private HttpClient client;
     private Server proxy;
     private ServerConnector proxyConnector;
@@ -185,6 +197,26 @@ private HttpClient prepareClient() throws Exception
         return result;
     }
 
+    private static HttpServletRequest mockClientRequest(String protocol)
+    {
+        return new org.eclipse.jetty.server.Request(null, null)
+        {
+            @Override
+            public String getProtocol()
+            {
+                return protocol;
+            }
+        };
+    }
+
+    private static HttpRequest mockProxyRequest()
+    {
+        return new HttpRequest(new HttpClient(), null, URI.create("https://example.com"))
+        {
+
+        };
+    }
+
     @AfterEach
     public void dispose() throws Exception
     {
@@ -549,6 +581,50 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se
             Matchers.equalTo("localhost:" + serverConnector.getLocalPort()));
     }
 
+    @ParameterizedTest
+    @MethodSource("impls")
+    public void testProxyViaHeaderIsPresent(Class<? extends ProxyServlet> proxyServletClass) throws Exception
+    {
+        startServer(new HttpServlet()
+        {
+            @Override
+            protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException
+            {
+                PrintWriter writer = resp.getWriter();
+                writer.write(req.getHeader("Via"));
+                writer.flush();
+            }
+        });
+        String viaHost = "my-good-via-host.example.org";
+        startProxy(proxyServletClass, Collections.singletonMap("viaHost", viaHost));
+        startClient();
+
+        ContentResponse response = client.GET("http://localhost:" + serverConnector.getLocalPort());
+        assertThat("Response expected to contain content of Via Header from the request",
+                   response.getContentAsString(),
+                   Matchers.equalTo("1.1 " + viaHost));
+    }
+
+    @ParameterizedTest
+    @MethodSource("implsWithProtocols")
+    public void testProxyViaHeaderForVariousProtocols(Class<? extends ProxyServlet> proxyServletClass, String protocol) throws Exception
+    {
+        AbstractProxyServlet proxyServlet = proxyServletClass.getDeclaredConstructor().newInstance();
+        String host = InetAddress.getLocalHost().getHostName();
+        HttpServletRequest clientRequest = mockClientRequest(protocol);
+        HttpRequest proxyRequest = mockProxyRequest();
+
+        proxyServlet.addViaHeader(clientRequest, proxyRequest);
+
+        String expectedProtocol = protocol.startsWith("HTTP") ? protocol.split("/", 2)[1] : protocol;
+        String expectedVia = expectedProtocol + " " + host;
+        String expectedViaWithLocalhost = expectedProtocol + " localhost";
+
+        assertThat("Response expected to contain a Via header with the right protocol version and host",
+                   proxyRequest.getHeaders().getField("Via").getValue(),
+                   Matchers.anyOf(Matchers.equalTo(expectedVia), Matchers.equalTo(expectedViaWithLocalhost)));
+    }
+
     @ParameterizedTest
     @MethodSource("impls")
     public void testProxyWhiteList(Class<? extends ProxyServlet> proxyServletClass) throws Exception