jetty-9 improved range request processing and tests

7 files changed, 300 insertions, 114 deletions

diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java
index 1a0b845f58..3db61eb24e 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java
@@ -183,6 +183,7 @@ public class HttpOutput extends ServletOutputStream
         if (_aggregate == null)
             _aggregate = _channel.getByteBufferPool().acquire(getBufferSize(), false);
 
+        System.err.println(BufferUtil.toDetailString(_aggregate));
         BufferUtil.append(_aggregate, (byte)b);
         _written++;
 
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/InclusiveByteRange.java b/jetty-server/src/main/java/org/eclipse/jetty/server/InclusiveByteRange.java
index ffd7055907..fd06cca6eb 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/InclusiveByteRange.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/InclusiveByteRange.java
@@ -44,6 +44,9 @@ import org.eclipse.jetty.util.log.Logger;
  * </PRE>
  * 
  * Based on RFC2616 3.12, 14.16, 14.35.1, 14.35.2
+ * <p>
+ * And yes the spec does strangely say that while 10-20, is bytes 10 to 20 and 10- is bytes 10 until the end that -20 IS NOT bytes 0-20, but the last 20 bytes of the content.
+ * 
  * @version $version$
  * 
  */
diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java
index 136a3c5b6b..74cdf50819 100644
--- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java
+++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java
@@ -941,6 +941,9 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory
                         in.skip(start-pos);
                         pos=start;
                     }
+                    
+                    System.err.println("PART "+ibr);
+                    
                     IO.copy(in,multi,size);
                     pos+=size;
                 }
diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletRangesTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletRangesTest.java
new file mode 100644
index 0000000000..58d9cc29e7
--- /dev/null
+++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletRangesTest.java
@@ -0,0 +1,273 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2012 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.servlet;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.util.EnumSet;
+import javax.servlet.DispatcherType;
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
+
+import junit.framework.AssertionFailedError;
+import org.eclipse.jetty.server.LocalConnector;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.toolchain.test.FS;
+import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
+import org.eclipse.jetty.toolchain.test.OS;
+import org.eclipse.jetty.toolchain.test.TestingDir;
+import org.eclipse.jetty.util.IO;
+import org.eclipse.jetty.util.StringUtil;
+import org.hamcrest.Matchers;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+import static org.junit.Assert.assertTrue;
+
+public class DefaultServletRangesTest
+{
+    public static final String DATA = "01234567890abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWZYZ!@#$%^&*()_+/.,[]";
+    @Rule
+    public TestingDir testdir = new TestingDir();
+
+    private Server server;
+    private LocalConnector connector;
+    private ServletContextHandler context;
+
+    @Before
+    public void init() throws Exception
+    {
+        server = new Server();
+        server.setSendServerVersion(false);
+
+        connector = new LocalConnector(server);
+
+        context = new ServletContextHandler();
+        context.setContextPath("/context");
+        context.setWelcomeFiles(new String[]{"index.html", "index.jsp", "index.htm"});
+
+        server.setHandler(context);
+        server.addConnector(connector);
+
+
+        testdir.ensureEmpty();
+        File resBase = testdir.getFile("docroot");
+        FS.ensureDirExists(resBase);
+        File data = new File(resBase, "data.txt");
+        createFile(data, DATA);
+        String resBasePath = resBase.getAbsolutePath();
+
+        ServletHolder defholder = context.addServlet(DefaultServlet.class, "/");
+        defholder.setInitParameter("acceptRanges", "true");
+        defholder.setInitParameter("resourceBase", resBasePath);
+        
+        server.start();
+    }
+
+    @After
+    public void destroy() throws Exception
+    {
+        server.stop();
+        server.join();
+    }
+
+    @Test
+    public void testNoRangeRequests() throws Exception
+    {
+        String response;
+        
+        response= connector.getResponses(
+                "GET /context/data.txt HTTP/1.1\r\n" +
+                        "Host: localhost\r\n" +
+                        "Connection: close\r\n"+
+                        "\r\n");
+        assertResponseContains("200 OK", response);
+        assertResponseContains("Accept-Ranges: bytes", response);
+        assertResponseContains(DATA,response);
+    }
+    
+    @Test
+    public void testPrefixRangeRequests() throws Exception
+    {
+        String response;
+
+        response = connector.getResponses(
+                "GET /context/data.txt HTTP/1.1\r\n" +
+                        "Host: localhost\r\n" +
+                        "Connection: close\r\n"+
+                        "Range: bytes=0-9\r\n" +
+                        "\r\n");
+        assertResponseContains("206 Partial", response);
+        assertResponseContains("Content-Type: text/plain", response);
+        assertResponseContains("Content-Length: 10", response);
+        assertResponseContains("Content-Range: bytes 0-9/80", response);
+        assertResponseContains(DATA.substring(0,10), response);
+    }
+    
+    @Test
+    public void testSingleRangeRequests() throws Exception
+    {
+        String response;
+
+        response = connector.getResponses(
+                "GET /context/data.txt HTTP/1.1\r\n" +
+                        "Host: localhost\r\n" +
+                        "Connection: close\r\n"+
+                        "Range: bytes=3-9\r\n" +
+                        "\r\n");
+        assertResponseContains("206 Partial", response);
+        assertResponseContains("Content-Type: text/plain", response);
+        assertResponseContains("Content-Length: 7", response);
+        assertResponseContains("Content-Range: bytes 3-9/80", response);
+        assertResponseContains(DATA.substring(3,10), response);
+    }
+    
+    @Test
+    public void testMultipleRangeRequests() throws Exception
+    {
+        String response;
+        response = connector.getResponses(
+                "GET /context/data.txt HTTP/1.1\r\n" +
+                "Host: localhost\r\n" +
+                "Connection: close\r\n"+
+                "Range: bytes=0-9,20-29,40-49\r\n" +
+                "\r\n");
+        int start = response.indexOf("--jetty");
+        String body = response.substring(start);
+        String boundary = body.substring(0, body.indexOf("\r\n"));
+        assertResponseContains("206 Partial", response);
+        assertResponseContains("Content-Type: multipart/byteranges; boundary=", response);
+        assertResponseContains("Content-Range: bytes 0-9/80", response);
+        assertResponseContains("Content-Range: bytes 20-29/80", response);
+        assertResponseContains("Content-Range: bytes 40-49/80", response);
+        assertResponseContains("Content-Length: " + body.length(), response);
+        assertResponseContains(DATA.substring(0,10), response);
+        assertResponseContains(DATA.substring(20,30), response);
+        assertResponseContains(DATA.substring(40,50), response);
+        assertTrue(body.endsWith(boundary + "--\r\n"));
+
+    }
+    
+    @Test
+    public void testOpenEndRange() throws Exception
+    {
+        String response;
+        response = connector.getResponses(
+                "GET /context/data.txt HTTP/1.1\r\n" +
+                "Host: localhost\r\n" +
+                "Connection: close\r\n"+
+                "Range: bytes=20-\r\n" +
+                "\r\n");
+        assertResponseContains("206 Partial", response);
+        assertResponseNotContains("Content-Type: multipart/byteranges; boundary=", response);
+        assertResponseContains("Content-Range: bytes 20-79/80", response);
+        assertResponseContains("Content-Length: 60", response);
+        assertResponseContains(DATA.substring(60), response);
+    }
+    
+    @Test
+    public void testOpenStartRange() throws Exception
+    {
+        String response;
+        response = connector.getResponses(
+                "GET /context/data.txt HTTP/1.1\r\n" +
+                "Host: localhost\r\n" +
+                "Connection: close\r\n"+
+                "Range: bytes=-20\r\n" +
+                "\r\n");
+        assertResponseContains("206 Partial", response);
+        assertResponseNotContains("Content-Type: multipart/byteranges; boundary=", response);
+        assertResponseContains("Content-Range: bytes 60-79/80", response); // yes the spec says it is these bytes
+        assertResponseContains("Content-Length: 20", response);
+        assertResponseContains(DATA.substring(60), response);
+    }
+
+    @Test
+    public void testUnsatisfiableRanges() throws Exception
+    {
+        String response;
+        response = connector.getResponses(
+                "GET /context/data.txt HTTP/1.1\r\n" +
+                "Host: localhost\r\n" +
+                "Connection: close\r\n"+
+                "Range: bytes=100-110\r\n" +
+                "\r\n");
+        assertResponseContains("416 Requested Range Not Satisfiable", response);
+    }
+    
+    
+    
+    
+    private void createFile(File file, String str) throws IOException
+    {
+        FileOutputStream out = null;
+        try
+        {
+            out = new FileOutputStream(file);
+            out.write(str.getBytes(StringUtil.__UTF8));
+            out.flush();
+        }
+        finally
+        {
+            IO.close(out);
+        }
+    }
+
+    private void assertResponseNotContains(String forbidden, String response)
+    {
+        Assert.assertThat(response,Matchers.not(Matchers.containsString(forbidden)));
+    }
+
+    private int assertResponseContains(String expected, String response)
+    {
+        Assert.assertThat(response,Matchers.containsString(expected));
+        return response.indexOf(expected);
+    }
+
+    private void deleteFile(File file) throws IOException
+    {
+        if (OS.IS_WINDOWS)
+        {
+            // Windows doesn't seem to like to delete content that was recently created
+            // Attempt a delete and if it fails, attempt a rename
+            boolean deleted = file.delete();
+            if (!deleted)
+            {
+                File deletedDir = MavenTestingUtils.getTargetFile(".deleted");
+                FS.ensureDirExists(deletedDir);
+                File dest = File.createTempFile(file.getName(), "deleted", deletedDir);
+                boolean renamed = file.renameTo(dest);
+                if (!renamed)
+                    System.err.println("WARNING: unable to move file out of the way: " + file.getName());
+            }
+        }
+        else
+        {
+            Assert.assertTrue("Deleting: " + file.getName(), file.delete());
+        }
+    }
+}
diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java
index 8e5dd05648..3de6d1aca4 100644
--- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java
+++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java
@@ -39,6 +39,7 @@ import org.eclipse.jetty.toolchain.test.OS;
 import org.eclipse.jetty.toolchain.test.TestingDir;
 import org.eclipse.jetty.util.IO;
 import org.eclipse.jetty.util.StringUtil;
+import org.hamcrest.Matchers;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
@@ -102,9 +103,7 @@ public class DefaultServletTest
         defholder.setInitParameter("resourceBase", resBasePath);
 
         StringBuffer req1 = new StringBuffer();
-        req1.append("GET /context/;JSESSIONID=1234567890 HTTP/1.1\n");
-        req1.append("Host: localhost\n");
-        req1.append("\n");
+        req1.append("GET /context/;JSESSIONID=1234567890 HTTP/1.0\n\n");
 
         String response = connector.getResponses(req1.toString());
 
@@ -144,8 +143,7 @@ public class DefaultServletTest
          * Intentionally bad request URI. Sending a non-encoded URI with typically encoded characters '<', '>', and
          * '"'.
          */
-        req1.append("GET /context/;<script>window.alert(\"hi\");</script> HTTP/1.1\n");
-        req1.append("Host: localhost\n");
+        req1.append("GET /context/;<script>window.alert(\"hi\");</script> HTTP/1.0\n");
         req1.append("\n");
 
         String response = connector.getResponses(req1.toString());
@@ -461,86 +459,6 @@ public class DefaultServletTest
     }
 
     @Test
-    public void testRangeRequests() throws Exception
-    {
-        testdir.ensureEmpty();
-        File resBase = testdir.getFile("docroot");
-        FS.ensureDirExists(resBase);
-        File data = new File(resBase, "data.txt");
-        createFile(data, "01234567890123456789012345678901234567890123456789012345678901234567890123456789");
-        String resBasePath = resBase.getAbsolutePath();
-
-        ServletHolder defholder = context.addServlet(DefaultServlet.class, "/");
-        defholder.setInitParameter("acceptRanges", "true");
-        defholder.setInitParameter("resourceBase", resBasePath);
-
-        String response = connector.getResponses(
-                "GET /context/data.txt HTTP/1.1\r\n" +
-                        "Host: localhost\r\n" +
-                        "\r\n");
-        assertResponseContains("200 OK", response);
-        assertResponseContains("Accept-Ranges: bytes", response);
-
-        response = connector.getResponses(
-                "GET /context/data.txt HTTP/1.1\r\n" +
-                        "Host: localhost\r\n" +
-                        "Range: bytes=0-9\r\n" +
-                        "\r\n");
-        assertResponseContains("206 Partial", response);
-        assertResponseContains("Content-Type: text/plain", response);
-        assertResponseContains("Content-Length: 10", response);
-        assertResponseContains("Content-Range: bytes 0-9/80", response);
-
-        response = connector.getResponses(
-                "GET /context/data.txt HTTP/1.1\r\n" +
-                        "Host: localhost\r\n" +
-                        "Range: bytes=0-9,20-29,40-49\r\n" +
-                        "\r\n");
-        int start = response.indexOf("--jetty");
-        String body = response.substring(start);
-        String boundary = body.substring(0, body.indexOf("\r\n"));
-        assertResponseContains("206 Partial", response);
-        assertResponseContains("Content-Type: multipart/byteranges; boundary=", response);
-        assertResponseContains("Content-Range: bytes 0-9/80", response);
-        assertResponseContains("Content-Range: bytes 20-29/80", response);
-        assertResponseContains("Content-Length: " + body.length(), response);
-        assertTrue(body.endsWith(boundary + "--\r\n"));
-
-        response = connector.getResponses(
-                "GET /context/data.txt HTTP/1.1\r\n" +
-                        "Host: localhost\r\n" +
-                        "Range: bytes=0-9,20-29,40-49,70-79\r\n" +
-                        "\r\n");
-        start = response.indexOf("--jetty");
-        body = response.substring(start);
-        boundary = body.substring(0, body.indexOf("\r\n"));
-        assertResponseContains("206 Partial", response);
-        assertResponseContains("Content-Type: multipart/byteranges; boundary=", response);
-        assertResponseContains("Content-Range: bytes 0-9/80", response);
-        assertResponseContains("Content-Range: bytes 20-29/80", response);
-        assertResponseContains("Content-Range: bytes 70-79/80", response);
-        assertResponseContains("Content-Length: " + body.length(), response);
-        assertTrue(body.endsWith(boundary + "--\r\n"));
-
-        response = connector.getResponses(
-                "GET /context/data.txt HTTP/1.1\r\n" +
-                        "Host: localhost\r\n" +
-                        "Range: bytes=0-9,20-29,40-49,60-60,70-79\r\n" +
-                        "\r\n");
-        start = response.indexOf("--jetty");
-        body = response.substring(start);
-        boundary = body.substring(0, body.indexOf("\r\n"));
-        assertResponseContains("206 Partial", response);
-        assertResponseContains("Content-Type: multipart/byteranges; boundary=", response);
-        assertResponseContains("Content-Range: bytes 0-9/80", response);
-        assertResponseContains("Content-Range: bytes 20-29/80", response);
-        assertResponseContains("Content-Range: bytes 60-60/80", response);
-        assertResponseContains("Content-Range: bytes 70-79/80", response);
-        assertResponseContains("Content-Length: " + body.length(), response);
-        assertTrue(body.endsWith(boundary + "--\r\n"));
-    }
-
-    @Test
     public void testFiltered() throws Exception
     {
         testdir.ensureEmpty();
@@ -558,12 +476,12 @@ public class DefaultServletTest
         defholder.setInitParameter("gzip", "false");
         defholder.setInitParameter("resourceBase", resBasePath);
 
-        String response = connector.getResponses("GET /context/data0.txt HTTP/1.1\r\nHost:localhost:8080\r\n\r\n");
+        String response = connector.getResponses("GET /context/data0.txt HTTP/1.0\r\n\r\n");
         assertResponseContains("Content-Length: 12", response);
         assertResponseNotContains("Extra Info", response);
 
         context.addFilter(OutputFilter.class,"/*",EnumSet.of(DispatcherType.REQUEST));
-        response = connector.getResponses("GET /context/data0.txt HTTP/1.1\r\nHost:localhost:8080\r\n\r\n");
+        response = connector.getResponses("GET /context/data0.txt HTTP/1.0\r\n\r\n");
         assertResponseContains("Content-Length: 2", response); // 20 something long
         assertResponseContains("Extra Info", response);
         assertResponseNotContains("Content-Length: 12", response);
@@ -572,7 +490,7 @@ public class DefaultServletTest
         context.getServletHandler().setFilters(new FilterHolder[]{});
 
         context.addFilter(WriterFilter.class,"/*",EnumSet.of(DispatcherType.REQUEST));
-        response = connector.getResponses("GET /context/data0.txt HTTP/1.1\r\nHost:localhost:8080\r\n\r\n");
+        response = connector.getResponses("GET /context/data0.txt HTTP/1.0\r\n\r\n");
         assertResponseContains("Content-Length: 2", response); // 20 something long
         assertResponseContains("Extra Info", response);
         assertResponseNotContains("Content-Length: 12", response);
@@ -629,33 +547,13 @@ public class DefaultServletTest
 
     private void assertResponseNotContains(String forbidden, String response)
     {
-        int idx = response.indexOf(forbidden);
-        if (idx != (-1))
-        {
-            // Found (when should not have)
-            StringBuffer err = new StringBuffer();
-            err.append("Response contain forbidden string \"").append(forbidden).append("\"");
-            err.append("\n").append(response);
-
-            System.err.println(err);
-            throw new AssertionFailedError(err.toString());
-        }
+        Assert.assertThat(response,Matchers.not(Matchers.containsString(forbidden)));
     }
 
     private int assertResponseContains(String expected, String response)
     {
-        int idx = response.indexOf(expected);
-        if (idx == (-1))
-        {
-            // Not found
-            StringBuffer err = new StringBuffer();
-            err.append("Response does not contain expected string \"").append(expected).append("\"");
-            err.append("\n").append(response);
-
-            System.err.println(err);
-            throw new AssertionFailedError(err.toString());
-        }
-        return idx;
+        Assert.assertThat(response,Matchers.containsString(expected));
+        return response.indexOf(expected);
     }
 
     private void deleteFile(File file) throws IOException
diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java
index 677b77913e..09d36d5e78 100644
--- a/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java
+++ b/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java
@@ -344,8 +344,8 @@ public class BufferUtil
     public static void append(ByteBuffer to, byte b)
     {
         int limit=to.limit();
-        to.put(limit,b);
         to.limit(limit+1);
+        to.put(limit,b);
     }
     
     /* ------------------------------------------------------------ */
@@ -359,6 +359,7 @@ public class BufferUtil
             needed=needed-channel.read(buffer);
     }
 
+    /* ------------------------------------------------------------ */
     public static void readFrom(InputStream is, int needed, ByteBuffer buffer) throws IOException
     {
         ByteBuffer tmp = allocate(8192);
diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartOutputStream.java b/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartOutputStream.java
index 8da310d460..a872d28615 100644
--- a/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartOutputStream.java
+++ b/jetty-util/src/main/java/org/eclipse/jetty/util/MultiPartOutputStream.java
@@ -58,8 +58,6 @@ public class MultiPartOutputStream extends FilterOutputStream
         inPart=false;
     }
 
-    
-
     /* ------------------------------------------------------------ */
     /** End the current part.
      * @exception IOException IOException
@@ -124,6 +122,15 @@ public class MultiPartOutputStream extends FilterOutputStream
         }
         out.write(__CRLF);
     }
+
+    /* ------------------------------------------------------------ */
+    @Override
+    public void write(byte[] b, int off, int len) throws IOException
+    {
+        out.write(b,off,len);
+    }
+
+    
     
 }