Merge remote-tracking branch 'upstream/master' into avoid-closing-con…

…nections-for-short-circuits

src/main/java/org/littleshoot/proxy/impl/ClientToProxyConnection.java

@@ -36,9 +36,9 @@
 import java.net.UnknownHostException;
 import java.nio.channels.ClosedChannelException;
 import java.nio.charset.Charset;
-import java.util.Arrays;
-import java.util.HashSet;
+import java.util.Date;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
@@ -77,11 +77,10 @@
 public class ClientToProxyConnection extends ProxyConnection<HttpRequest> {
     private static final HttpResponseStatus CONNECTION_ESTABLISHED = new HttpResponseStatus(
             200, "HTTP/1.1 200 Connection established");
-
-    private static final Set<String> HOP_BY_HOP_HEADERS = new HashSet<String>(
-            Arrays.asList(new String[] { "connection", "keep-alive",
-                    "proxy-authenticate", "proxy-authorization", "te",
-                    "trailers", "upgrade" }));
+    /**
+     * Used for case-insensitive comparisons when parsing Connection header values.
+     */
+    private static final String LOWERCASE_TRANSFER_ENCODING_HEADER = HttpHeaders.Names.TRANSFER_ENCODING.toLowerCase(Locale.US);
 
     /**
      * Keep track of all ProxyToServerConnections by host+port.
@@ -428,8 +427,8 @@ protected Future<?> execute() {
             LOG.debug("Responding with CONNECT successful");
             HttpResponse response = responseFor(HttpVersion.HTTP_1_1,
                     CONNECTION_ESTABLISHED);
-            response.headers().set("Connection", "Keep-Alive");
-            response.headers().set("Proxy-Connection", "Keep-Alive");
+            response.headers().set(HttpHeaders.Names.CONNECTION, HttpHeaders.Values.KEEP_ALIVE);
+            response.headers().set("Proxy-Connection", HttpHeaders.Values.KEEP_ALIVE);
             ProxyUtils.addVia(response);
             return writeToChannel(response);
         };
@@ -926,10 +925,9 @@ private void writeAuthenticationRequired() {
                 + "the credentials required.</p>\n" + "</body></html>\n";
         DefaultFullHttpResponse response = responseFor(HttpVersion.HTTP_1_1,
                 HttpResponseStatus.PROXY_AUTHENTICATION_REQUIRED, body);
-        response.headers().set("Date", ProxyUtils.httpDate());
+        HttpHeaders.setDate(response, new Date());
         response.headers().set("Proxy-Authenticate",
                 "Basic realm=\"Restricted Files\"");
-        response.headers().set("Date", ProxyUtils.httpDate());
         write(response);
     }
 
@@ -1014,6 +1012,7 @@ private void modifyResponseHeadersToReflectProxying(
             HttpResponse httpResponse) {
         if (!proxyServer.isTransparent()) {
             HttpHeaders headers = httpResponse.headers();
+
             stripConnectionTokens(headers);
             stripHopByHopHeaders(headers);
             ProxyUtils.addVia(httpResponse);
@@ -1025,8 +1024,8 @@ private void modifyResponseHeadersToReflectProxying(
              * assigned one by the recipient if the message will be cached by
              * that recipient or gatewayed via a protocol which requires a Date.
              */
-            if (!headers.contains("Date")) {
-                headers.set("Date", ProxyUtils.httpDate());
+            if (!headers.contains(HttpHeaders.Names.DATE)) {
+                HttpHeaders.setDate(httpResponse, new Date());
             }
         }
     }
@@ -1060,7 +1059,7 @@ private void switchProxyConnectionHeader(HttpHeaders headers) {
         if (headers.contains(proxyConnectionKey)) {
             String header = headers.get(proxyConnectionKey);
             headers.remove(proxyConnectionKey);
-            headers.set("Connection", header);
+            headers.set(HttpHeaders.Names.CONNECTION, header);
         }
     }
 
@@ -1076,10 +1075,14 @@ private void switchProxyConnectionHeader(HttpHeaders headers) {
      *            The headers to modify
      */
     private void stripConnectionTokens(HttpHeaders headers) {
-        if (headers.contains("Connection")) {
-            for (String headerValue : headers.getAll("Connection")) {
-                for (String connectionToken : headerValue.split(",")) {
-                    headers.remove(connectionToken);
+        if (headers.contains(HttpHeaders.Names.CONNECTION)) {
+            for (String headerValue : headers.getAll(HttpHeaders.Names.CONNECTION)) {
+                for (String connectionToken : ProxyUtils.splitCommaSeparatedHeaderValues(headerValue)) {
+                    // do not strip out the Transfer-Encoding header if it is specified in the Connection header, since LittleProxy does not
+                    // normally modify the Transfer-Encoding of the message.
+                    if (!LOWERCASE_TRANSFER_ENCODING_HEADER.equals(connectionToken.toLowerCase(Locale.US))) {
+                        headers.remove(connectionToken);
+                    }
                 }
             }
         }
@@ -1094,9 +1097,9 @@ private void stripConnectionTokens(HttpHeaders headers) {
      */
     private void stripHopByHopHeaders(HttpHeaders headers) {
         Set<String> headerNames = headers.names();
-        for (String name : headerNames) {
-            if (HOP_BY_HOP_HEADERS.contains(name.toLowerCase())) {
-                headers.remove(name);
+        for (String headerName : headerNames) {
+            if (ProxyUtils.shouldRemoveHopByHopHeader(headerName)) {
+                headers.remove(headerName);
             }
         }
     }

src/main/java/org/littleshoot/proxy/impl/ProxyUtils.java

@@ -2,6 +2,7 @@
 
 import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import io.netty.buffer.ByteBuf;
 import io.netty.handler.codec.http.DefaultFullHttpResponse;
 import io.netty.handler.codec.http.DefaultHttpResponse;
@@ -26,20 +27,39 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Properties;
+import java.util.Set;
 import java.util.TimeZone;
 import java.util.regex.Pattern;
 
 /**
  * Utilities for the proxy.
  */
 public class ProxyUtils {
+    /**
+     * Hop-by-hop headers that should be removed when proxying, as defined by the HTTP 1.1 spec, section 13.5.1
+     * (http://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.5.1). Transfer-Encoding is NOT included in this list, since LittleProxy
+     * does not typically modify the transfer encoding. See also {@link #shouldRemoveHopByHopHeader(String)}.
+     *
+     * Header names are stored as lowercase to make case-insensitive comparisons easier.
+     */
+    private static final Set<String> SHOULD_NOT_PROXY_HOP_BY_HOP_HEADERS = ImmutableSet.of(
+            HttpHeaders.Names.CONNECTION.toLowerCase(Locale.US),
+            HttpHeaders.Names.PROXY_AUTHENTICATE.toLowerCase(Locale.US),
+            HttpHeaders.Names.PROXY_AUTHORIZATION.toLowerCase(Locale.US),
+            HttpHeaders.Names.TE.toLowerCase(Locale.US),
+            HttpHeaders.Names.TRAILER.toLowerCase(Locale.US),
+            /*  Note: Not removing Transfer-Encoding since LittleProxy does not normally re-chunk content.
+                HttpHeaders.Names.TRANSFER_ENCODING.toLowerCase(Locale.US), */
+            HttpHeaders.Names.UPGRADE.toLowerCase(Locale.US),
+            "Keep-Alive".toLowerCase(Locale.US)
+    );
 
     private static final Logger LOG = LoggerFactory.getLogger(ProxyUtils.class);
 
     private static final TimeZone GMT = TimeZone.getTimeZone("GMT");
 
     /**
-     * Splits comma-separated header values into their individual values.
+     * Splits comma-separated header values (such as Connection) into their individual tokens.
      */
     private static final Splitter COMMA_SEPARATED_HEADER_VALUE_SPLITTER = Splitter.on(',').trimResults().omitEmptyStrings();
 
@@ -123,15 +143,6 @@ public static String formatDate(final Date date, final String pattern) {
         return formatter.format(date);
     }
 
-    /**
-     * Creates a Date formatted for HTTP headers for the current time.
-     * 
-     * @return The formatted HTTP date.
-     */
-    public static String httpDate() {
-        return formatDate(new Date());
-    }
-
     /**
      * If an HttpObject implements the market interface LastHttpContent, it
      * represents the last chunk of a transfer.
@@ -466,7 +477,7 @@ public static List<String> getAllCommaSeparatedHeaderValues(String headerName, H
 
         ImmutableList.Builder<String> headerValues = ImmutableList.builder();
         for (String header : allHeaders) {
-            Iterable<String> commaSeparatedValues = COMMA_SEPARATED_HEADER_VALUE_SPLITTER.split(header);
+            List<String> commaSeparatedValues = splitCommaSeparatedHeaderValues(header);
             headerValues.addAll(commaSeparatedValues);
         }
 
@@ -501,4 +512,44 @@ public static String getHostName() throws IllegalStateException {
             throw new IllegalStateException("Could not determine host!", e);
         }
     }
+
+    /**
+     * Determines if the specified header should be removed from the proxied response because it is a hop-by-hop header, as defined by the
+     * HTTP 1.1 spec in section 13.5.1. The comparison is case-insensitive, so "Connection" will be treated the same as "connection" or "CONNECTION".
+     * From http://www.w3.org/Protocols/rfc2616/rfc2616-sec13.html#sec13.5.1 :
+     * <pre>
+       The following HTTP/1.1 headers are hop-by-hop headers:
+        - Connection
+        - Keep-Alive
+        - Proxy-Authenticate
+        - Proxy-Authorization
+        - TE
+        - Trailers [LittleProxy note: actual header name is Trailer]
+        - Transfer-Encoding [LittleProxy note: this header is not normally removed when proxying, since the proxy does not re-chunk
+                            responses. The exception is when an HttpObjectAggregator is enabled, which aggregates chunked content and removes
+                            the 'Transfer-Encoding: chunked' header itself.]
+        - Upgrade
+
+       All other headers defined by HTTP/1.1 are end-to-end headers.
+     * </pre>
+     *
+     * @param headerName the header name
+     * @return true if this header is a hop-by-hop header and should be removed when proxying, otherwise false
+     */
+    public static boolean shouldRemoveHopByHopHeader(String headerName) {
+        return SHOULD_NOT_PROXY_HOP_BY_HOP_HEADERS.contains(headerName.toLowerCase(Locale.US));
+    }
+
+    /**
+     * Splits comma-separated header values into tokens. For example, if the value of the Connection header is "Transfer-Encoding, close",
+     * this method will return "Transfer-Encoding" and "close". This method strips trims any optional whitespace from
+     * the tokens. Unlike {@link #getAllCommaSeparatedHeaderValues(String, HttpMessage)}, this method only operates on
+     * a single header value, rather than all instances of the header in a message.
+     *
+     * @param headerValue the un-tokenized header value (must not be null)
+     * @return all tokens within the header value, or an empty list if there are no values
+     */
+    public static List<String> splitCommaSeparatedHeaderValues(String headerValue) {
+        return ImmutableList.copyOf(COMMA_SEPARATED_HEADER_VALUE_SPLITTER.split(headerValue));
+    }
 }

src/test/java/org/littleshoot/proxy/ProxyHeadersTest.java

@@ -0,0 +1,94 @@
+package org.littleshoot.proxy;
+
+import org.apache.http.Header;
+import org.apache.http.HttpResponse;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.util.EntityUtils;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.littleshoot.proxy.impl.DefaultHttpProxyServer;
+import org.mockserver.integration.ClientAndServer;
+import org.mockserver.matchers.Times;
+import org.mockserver.model.ConnectionOptions;
+
+import static org.hamcrest.Matchers.emptyArray;
+import static org.hamcrest.Matchers.not;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+import static org.mockserver.model.HttpRequest.request;
+import static org.mockserver.model.HttpResponse.response;
+
+/**
+ * Tests the proxy's handling and manipulation of headers.
+ */
+public class ProxyHeadersTest {
+    private HttpProxyServer proxyServer;
+
+    private ClientAndServer mockServer;
+    private int mockServerPort;
+
+    @Before
+    public void setUp() throws Exception {
+        mockServer = new ClientAndServer(0);
+        mockServerPort = mockServer.getPort();
+    }
+
+    @After
+    public void tearDown() throws Exception {
+        try {
+            if (proxyServer != null) {
+                proxyServer.abort();
+            }
+        } finally {
+            if (mockServer != null) {
+                mockServer.stop();
+            }
+        }
+    }
+
+    @Test
+    public void testProxyRemovesConnectionHeadersFromServer() throws Exception {
+        // the proxy should remove all Connection headers, since it is a hop-by-hop header. however, since the proxy does not
+        // generally modify the Transfer-Encoding of the message, it should not remove the Transfer-Encoding header.
+        mockServer.when(request()
+                        .withMethod("GET")
+                        .withPath("/connectionheaders"),
+                Times.exactly(1))
+                .respond(response()
+                        .withStatusCode(200)
+                        .withBody("success")
+                        .withHeader("Connection", "Transfer-Encoding, Dummy-Header")
+                        .withHeader("Transfer-Encoding", "identity")
+                        .withHeader("Dummy-Header", "dummy-value")
+                        .withConnectionOptions(new ConnectionOptions()
+                                .withSuppressConnectionHeader(true))
+                );
+
+        this.proxyServer = DefaultHttpProxyServer.bootstrap()
+                .withPort(0)
+                .start();
+
+        HttpClient httpClient = TestUtils.createProxiedHttpClient(proxyServer.getListenAddress().getPort());
+        HttpResponse response = httpClient.execute(new HttpGet("http://localhost:" + mockServerPort + "/connectionheaders"));
+        EntityUtils.consume(response.getEntity());
+
+        Header[] dummyHeaders = response.getHeaders("Dummy-Header");
+        assertThat("Expected proxy to remove the Dummy-Header specified in the Connection header", dummyHeaders, emptyArray());
+
+        Header[] transferEncodingHeaders = response.getHeaders("Transfer-Encoding");
+        assertThat("Expected proxy to keep the Transfer-Encoding header, even when specified in the Connection header", transferEncodingHeaders, not(emptyArray()));
+
+        // make sure we find the "identity" header, which should not be removed
+        boolean foundIdentity = false;
+        for (Header transferEncodingHeader : transferEncodingHeaders) {
+            if ("identity".equals(transferEncodingHeader.getValue())) {
+                foundIdentity = true;
+                break;
+            }
+        }
+
+        assertTrue("Expected to find Transfer-Encoding: identity header value specified in response", foundIdentity);
+    }
+}

src/test/java/org/littleshoot/proxy/impl/ProxyUtilsTest.java

@@ -203,4 +203,20 @@ public void testAddNewViaHeaderToExistingViaHeader() {
         String expectedViaHeader = "1.1 " + hostname;
         assertEquals(expectedViaHeader, viaHeaders.get(1));
     }
+
+    @Test
+    public void testSplitCommaSeparatedHeaderValues() {
+        assertThat("Incorrect header tokens", ProxyUtils.splitCommaSeparatedHeaderValues("one"), contains("one"));
+        assertThat("Incorrect header tokens", ProxyUtils.splitCommaSeparatedHeaderValues("one,two,three"), contains("one", "two", "three"));
+        assertThat("Incorrect header tokens", ProxyUtils.splitCommaSeparatedHeaderValues("one, two, three"), contains("one", "two", "three"));
+        assertThat("Incorrect header tokens", ProxyUtils.splitCommaSeparatedHeaderValues(" one,two,  three "), contains("one", "two", "three"));
+        assertThat("Incorrect header tokens", ProxyUtils.splitCommaSeparatedHeaderValues("\t\tone ,\t two,  three\t"), contains("one", "two", "three"));
+
+        assertThat("Expected no header tokens", ProxyUtils.splitCommaSeparatedHeaderValues(""), empty());
+        assertThat("Expected no header tokens", ProxyUtils.splitCommaSeparatedHeaderValues(","), empty());
+        assertThat("Expected no header tokens", ProxyUtils.splitCommaSeparatedHeaderValues(" "), empty());
+        assertThat("Expected no header tokens", ProxyUtils.splitCommaSeparatedHeaderValues("\t"), empty());
+        assertThat("Expected no header tokens", ProxyUtils.splitCommaSeparatedHeaderValues("  \t  \t  "), empty());
+        assertThat("Expected no header tokens", ProxyUtils.splitCommaSeparatedHeaderValues(" ,  ,\t, "), empty());
+    }
 }