410098 inject accept-encoding header for all http requests through SPDY as SPDY clients MUST support spdy. Also remove two new tests that have been to implementation agnostic and not needed anymore due to recent code changes

jetty-servlets/src/test/java/org/eclipse/jetty/servlets/GzipISETest.java

@@ -1,100 +0,0 @@
-//
-//  ========================================================================
-//  Copyright (c) 1995-2013 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.servlets;
-
-import java.io.BufferedReader;
-import java.io.IOException;
-import java.io.InputStreamReader;
-import java.net.Socket;
-import java.util.EnumSet;
-import javax.servlet.DispatcherType;
-
-import org.eclipse.jetty.http.HttpURI;
-import org.eclipse.jetty.servlet.DefaultServlet;
-import org.eclipse.jetty.servlet.FilterHolder;
-import org.eclipse.jetty.servlet.ServletHolder;
-import org.eclipse.jetty.servlet.ServletTester;
-import org.eclipse.jetty.toolchain.test.http.SimpleHttpParser;
-import org.eclipse.jetty.toolchain.test.http.SimpleHttpResponse;
-import org.eclipse.jetty.util.log.Log;
-import org.eclipse.jetty.util.log.Logger;
-import org.junit.Before;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
-
-import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertThat;
-
-@RunWith(JUnit4.class)
-public class GzipISETest
-{
-    private static final Logger LOG = Log.getLogger(GzipISETest.class);
-
-    private ServletTester servletTester = new ServletTester("/ctx");
-    private String host;
-    private int port;
-    private FilterHolder gzipFilterHolder;
-    private SimpleHttpParser httpParser = new SimpleHttpParser();
-
-    @Before
-    public void setUp() throws Exception
-    {
-        HttpURI uri = new HttpURI(servletTester.createConnector(true));
-        host = uri.getHost();
-        port = uri.getPort();
-        gzipFilterHolder = servletTester.addFilter(GzipFilter.class, "/*", EnumSet.allOf(DispatcherType.class));
-        gzipFilterHolder.start();
-        gzipFilterHolder.initialize();
-
-        ServletHolder servletHolder = servletTester.addServlet(DefaultServlet.class, "/*");
-        servletHolder.setInitParameter("resourceBase","src/test/resources/big_script.js");
-        servletHolder.setInitParameter("maxCachedFiles","10");
-        servletHolder.setInitParameter("dirAllowed","true");
-        servletHolder.start();
-        servletHolder.initialize();
-
-        servletTester.start();
-    }
-
-    /**
-     * This is a regression test for #409403. This test uses DefaultServlet + ResourceCache + GzipFilter to walk
-     * through a code path that writes every single byte individually into HttpOutput's _aggregate buffer. The bug
-     * never occured in plain http as the buffer gets passed around up to EndPoint.flush() where it gets cleared.
-     * This test is supposed to assure that future changes won't break this.
-     *
-     * @throws IOException
-     */
-    @Test
-    public void testISE() throws IOException
-    {
-        Socket socket = new Socket(host, port);
-        socket.setSoTimeout(10000);
-        BufferedReader reader = new BufferedReader(new InputStreamReader(socket.getInputStream(), "UTF-8"));
-        String request = "GET /ctx/ HTTP/1.0\r\n";
-        request += "Host: localhost:" + port + "\r\n";
-//        request += "accept-encoding: gzip\r\n";
-        request += "\r\n";
-        socket.getOutputStream().write(request.getBytes("UTF-8"));
-        socket.getOutputStream().flush();
-        SimpleHttpResponse response = httpParser.readResponse(reader);
-
-        assertThat("response body length is as expected", response.getBody().length(), is(76846));
-    }
-}

jetty-spdy/spdy-core/src/main/java/org/eclipse/jetty/spdy/parser/SynStreamBodyParser.java

@@ -137,9 +137,11 @@ public class SynStreamBodyParser extends ControlFrameBodyParser
                     {
                         byte flags = controlFrameParser.getFlags();
                         if (flags > (SynInfo.FLAG_CLOSE | PushSynInfo.FLAG_UNIDIRECTIONAL))
-                            throw new IllegalArgumentException("Invalid flag " + flags + " for frame " + ControlFrameType.SYN_STREAM);
+                            throw new IllegalArgumentException("Invalid flag " + flags + " for frame " +
+                                    ControlFrameType.SYN_STREAM);
 
-                        SynStreamFrame frame = new SynStreamFrame(version, flags, streamId, associatedStreamId, priority, slot, new Fields(headers, true));
+                        SynStreamFrame frame = new SynStreamFrame(version, flags, streamId, associatedStreamId,
+                                priority, slot, new Fields(headers, false));
                         controlFrameParser.onControlFrame(frame);
 
                         reset();

jetty-spdy/spdy-http-server/src/main/java/org/eclipse/jetty/spdy/server/http/HTTPSPDYServerConnectionFactory.java

@@ -97,6 +97,14 @@ public class HTTPSPDYServerConnectionFactory extends SPDYServerConnectionFactory
             logger.debug("Received {} on {}", synInfo, stream);
 
             Fields headers = synInfo.getHeaders();
+            // According to SPDY/3 spec section 3.2.1 user-agents MUST support gzip compression. Firefox omits the
+            // accep-encoding header as it is redundant to negotiate gzip compression support with the server,
+            // if clients have to accept it.
+            // So we inject the accept-encoding header here, even if not set by the client. This will enforce SPDY
+            // clients to follow the spec and enable gzip compression if GzipFilter or the like is enabled.
+            if (!(headers.get("accept-encoding") != null && headers.get("accept-encoding").value().contains
+                    ("gzip")))
+                headers.add("accept-encoding", "gzip");
             HttpTransportOverSPDY transport = new HttpTransportOverSPDY(connector, httpConfiguration, endPoint, pushStrategy, stream, headers);
             HttpInputOverSPDY input = new HttpInputOverSPDY();
             HttpChannelOverSPDY channel = new HttpChannelOverSPDY(connector, httpConfiguration, endPoint, transport, input, stream);

jetty-spdy/spdy-http-server/src/test/java/org/eclipse/jetty/spdy/server/http/ReferrerPushStrategyTest.java

@@ -505,7 +505,10 @@ public class ReferrerPushStrategyTest extends AbstractHTTPSPDYTest
         else
             assertThat("No push data is received", pushDataLatch.await(1, TimeUnit.SECONDS), is(false));
         if (validateHeaders)
+        {
             assertThat("Push push headers valid", pushSynHeadersValid.await(5, TimeUnit.SECONDS), is(true));
+            assertThat("Push response headers are valid", pushResponseHeaders.await(5, TimeUnit.SECONDS), is(true));
+        }
     }
 
     private static final Logger LOG = Log.getLogger(ReferrerPushStrategyTest.class);

jetty-spdy/spdy-http-server/src/test/java/org/eclipse/jetty/spdy/server/http/ServerHTTPSPDYTest.java

@@ -54,6 +54,10 @@ import org.junit.Assert;
 import org.junit.Ignore;
 import org.junit.Test;
 
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
 public class ServerHTTPSPDYTest extends AbstractHTTPSPDYTest
 {
     public ServerHTTPSPDYTest(short version)
@@ -61,6 +65,7 @@ public class ServerHTTPSPDYTest extends AbstractHTTPSPDYTest
         super(version);
     }
 
+    @Test
     public void testSimpleGET() throws Exception
     {
         final String path = "/foo";
@@ -75,12 +80,14 @@ public class ServerHTTPSPDYTest extends AbstractHTTPSPDYTest
                 Assert.assertEquals("GET", httpRequest.getMethod());
                 Assert.assertEquals(path, target);
                 Assert.assertEquals(path, httpRequest.getRequestURI());
-                Assert.assertEquals("localhost:" + connector.getLocalPort(), httpRequest.getHeader("host"));
+                assertThat("accept-encoding is set to gzip, even if client didn't set it",
+                        httpRequest.getHeader("accept-encoding"), containsString("gzip"));
+                assertThat(httpRequest.getHeader("host"), is("localhost:" + connector.getLocalPort()));
                 handlerLatch.countDown();
             }
         }), null);
 
-        Fields headers = SPDYTestUtils.createHeaders("localhost", connector.getPort(), version, "GET", path);
+        Fields headers = SPDYTestUtils.createHeaders("localhost", connector.getLocalPort(), version, "GET", path);
         final CountDownLatch replyLatch = new CountDownLatch(1);
         session.syn(new SynInfo(headers, true), new StreamFrameListener.Adapter()
         {

jetty-spdy/spdy-http-server/src/test/java/org/eclipse/jetty/spdy/server/http/WriteThroughAggregateBufferTest.java

@@ -1,111 +0,0 @@
-//
-//  ========================================================================
-//  Copyright (c) 1995-2013 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.spdy.server.http;
-
-import java.util.EnumSet;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicInteger;
-import javax.servlet.DispatcherType;
-
-import org.eclipse.jetty.servlet.DefaultServlet;
-import org.eclipse.jetty.servlet.FilterHolder;
-import org.eclipse.jetty.servlet.ServletContextHandler;
-import org.eclipse.jetty.servlet.ServletHolder;
-import org.eclipse.jetty.servlets.GzipFilter;
-import org.eclipse.jetty.spdy.api.DataInfo;
-import org.eclipse.jetty.spdy.api.ReplyInfo;
-import org.eclipse.jetty.spdy.api.Session;
-import org.eclipse.jetty.spdy.api.Stream;
-import org.eclipse.jetty.spdy.api.StreamFrameListener;
-import org.eclipse.jetty.spdy.api.SynInfo;
-import org.eclipse.jetty.util.Fields;
-import org.eclipse.jetty.util.log.Log;
-import org.eclipse.jetty.util.log.Logger;
-import org.junit.Assert;
-import org.junit.Test;
-
-import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertThat;
-
-public class WriteThroughAggregateBufferTest extends AbstractHTTPSPDYTest
-{
-    private static final Logger LOG = Log.getLogger(WriteThroughAggregateBufferTest.class);
-    public WriteThroughAggregateBufferTest(short version)
-    {
-        super(version);
-    }
-
-    /**
-     * This test was created due to bugzilla #409403. It tests a specific code path through DefaultServlet leading to
-     * every byte of the response being sent one by one through BufferUtil.writeTo(). To do this,
-     * we need to use DefaultServlet + ResourceCache + GzipFilter. As this bug only affected SPDY,
-     * we test using SPDY. The accept-encoding header must not be set to replicate this issue.
-     * @throws Exception
-     */
-    @Test
-    public void testGetBigJavaScript() throws Exception
-    {
-        final CountDownLatch replyLatch = new CountDownLatch(1);
-        final CountDownLatch dataLatch = new CountDownLatch(1);
-
-        ServletContextHandler contextHandler = new ServletContextHandler(getServer(), "/ctx");
-
-        FilterHolder gzipFilterHolder = new FilterHolder(GzipFilter.class);
-        contextHandler.addFilter(gzipFilterHolder, "/*", EnumSet.allOf(DispatcherType.class));
-
-        ServletHolder servletHolder = new ServletHolder(DefaultServlet.class);
-        servletHolder.setInitParameter("resourceBase", "src/test/resources/");
-        servletHolder.setInitParameter("dirAllowed", "true");
-        servletHolder.setInitParameter("maxCachedFiles", "10");
-        contextHandler.addServlet(servletHolder, "/*");
-        Session session = startClient(version, startHTTPServer(version, contextHandler), null);
-
-        Fields headers = SPDYTestUtils.createHeaders("localhost", connector.getPort(), version, "GET",
-                "/ctx/big_script.js");
-//        headers.add("accept-encoding","gzip");
-        session.syn(new SynInfo(headers, true), new StreamFrameListener.Adapter()
-        {
-            AtomicInteger bytesReceived= new AtomicInteger(0);
-            @Override
-            public void onReply(Stream stream, ReplyInfo replyInfo)
-            {
-                Fields replyHeaders = replyInfo.getHeaders();
-                Assert.assertTrue(replyHeaders.get(HTTPSPDYHeader.STATUS.name(version)).value().contains("200"));
-                replyLatch.countDown();
-            }
-
-            @Override
-            public void onData(Stream stream, DataInfo dataInfo)
-            {
-                bytesReceived.addAndGet(dataInfo.available());
-                dataInfo.consume(dataInfo.available());
-                if (dataInfo.isClose())
-                {
-                    assertThat("bytes received matches file size: 76847", bytesReceived.get(), is(76847));
-                    dataLatch.countDown();
-                }
-            }
-
-        });
-
-        assertThat("reply is received", replyLatch.await(5, TimeUnit.SECONDS), is(true));
-        assertThat("all data is sent", dataLatch.await(5, TimeUnit.SECONDS), is(true));
-    }
-}