Fixed bug where the 'Accept-Encoding' header could be improperly form…

…atted after removing the 'sdch' encoding

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

@@ -1076,7 +1076,8 @@ private void modifyRequestHeadersToReflectProxying(HttpRequest httpRequest) {
 
             HttpHeaders headers = httpRequest.headers();
 
-            removeSDCHEncoding(headers);
+            // Remove sdch from encodings we accept since we can't decode it.
+            ProxyUtils.removeSdchEncoding(headers);
             switchProxyConnectionHeader(headers);
             stripConnectionTokens(headers);
             stripHopByHopHeaders(headers);
@@ -1113,22 +1114,6 @@ private void modifyResponseHeadersToReflectProxying(
         }
     }
 
-    /**
-     * Remove sdch from encodings we accept since we can't decode it.
-     * 
-     * @param headers
-     *            The headers to modify
-     */
-    private void removeSDCHEncoding(HttpHeaders headers) {
-        String ae = headers.get(HttpHeaders.Names.ACCEPT_ENCODING);
-        if (StringUtils.isNotBlank(ae)) {
-            //
-            String noSdch = ae.replace(",sdch", "").replace("sdch", "");
-            headers.set(HttpHeaders.Names.ACCEPT_ENCODING, noSdch);
-            LOG.debug("Removed sdch and inserted: {}", noSdch);
-        }
-    }
-
     /**
      * Switch the de-facto standard "Proxy-Connection" header to "Connection"
      * when we pass it along to the remote host. This is largely undocumented

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

@@ -637,4 +637,30 @@ public static FullHttpResponse createFullHttpResponse(HttpVersion httpVersion,
 
         return response;
     }
+
+    /**
+     * Given an HttpHeaders instance, removes 'sdch' from the 'Accept-Encoding'
+     * header list (if it exists) and returns the modified instance.
+     *
+     * Removes all occurrences of 'sdch' from the 'Accept-Encoding' header.
+     * @param headers The headers to modify.
+     */
+    public static HttpHeaders removeSdchEncoding(HttpHeaders headers) {
+        if (headers == null) {
+            throw new IllegalArgumentException("Headers is null");
+        }
+
+        String ae = headers.get(HttpHeaders.Names.ACCEPT_ENCODING);
+        if (StringUtils.isNotBlank(ae)) {
+            // The former regex should remove occurrences of 'sdch' while the
+            // latter regex should take care of the dangling comma case when
+            // 'sdch' was the first element in the list and there are other
+            // encodings.
+            String noSdch = ae.replaceAll(",? *(sdch|SDCH)", "").replace("^ *, *", "");
+            headers.set(HttpHeaders.Names.ACCEPT_ENCODING, noSdch);
+            LOG.debug("Removed sdch and inserted: {}", noSdch);
+        }
+
+        return headers;
+    }
 }

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

@@ -1,6 +1,7 @@
 package org.littleshoot.proxy.impl;
 
 import io.netty.handler.codec.http.DefaultFullHttpRequest;
+import io.netty.handler.codec.http.DefaultHttpHeaders;
 import io.netty.handler.codec.http.DefaultHttpMessage;
 import io.netty.handler.codec.http.DefaultHttpResponse;
 import io.netty.handler.codec.http.HttpHeaders;
@@ -235,4 +236,43 @@ public void testSplitCommaSeparatedHeaderValues() {
         assertThat("Expected no header tokens", ProxyUtils.splitCommaSeparatedHeaderValues("  \t  \t  "), empty());
         assertThat("Expected no header tokens", ProxyUtils.splitCommaSeparatedHeaderValues(" ,  ,\t, "), empty());
     }
+
+    /**
+     * Verifies that 'sdch' is removed from the 'Accept-Encoding' header list.
+     */
+    @Test
+    public void testRemoveSdchEncoding() {
+        // Various cases where 'sdch' is not present within the accepted
+        // encodings list
+        assertRemoveSdchEncoding("", "");
+        assertRemoveSdchEncoding("gzip", "gzip");
+        assertRemoveSdchEncoding("gzip, deflate, br", "gzip, deflate, br");
+
+        // Various cases where 'sdch' is present within the accepted encodings
+        // list
+        assertRemoveSdchEncoding("sdch", "");
+        assertRemoveSdchEncoding("SDCH", "");
+        assertRemoveSdchEncoding("sdch, gzip", "gzip");
+        assertRemoveSdchEncoding("gzip,sdch,delate", "gzip,deflate");
+        assertRemoveSdchEncoding("gzip, sdch, deflate", "gzip, deflate");
+        assertRemoveSdchEncoding("gzip,deflate,sdch", "gzip,deflate");
+        assertRemoveSdchEncoding("gzip, deflate, sdch", "gzip, deflate");
+    }
+
+    /**
+     * Helper method that asserts that 'sdch' is removed from the
+     * 'Accept-Encoding' header.
+     *
+     * @param inputEncodings The input value of the 'Accept-Encoding' header
+     *        that should be used as the basis for the assertion check.
+     * @param expectedEncodings The expected value of the 'Accept-Encoding'
+     *        header after the 'sdch' encoding is removed.
+     */
+    private void assertRemoveSdchEncoding(String inputEncodings, String expectedEncodings) {
+        HttpHeaders headers = new DefaultHttpHeaders();
+
+        headers.add(HttpHeaders.Names.ACCEPT_ENCODING, inputEncodings);
+        assertEquals(expectedEncodings,
+                ProxyUtils.removeSdchEncoding(headers).get(HttpHeaders.Names.ACCEPT_ENCODING));
+    }
 }