Fixing "Range" vs "Content-Range" support in DefaultServlet per RFC2616.

git-svn-id: svn+ssh://dev.eclipse.org/svnroot/rt/org.eclipse.jetty/jetty/trunk@513 7e9141cc-0065-0410-87d8-b60c137991c4

jetty-server/src/main/java/org/eclipse/jetty/server/InclusiveByteRange.java

@@ -70,7 +70,7 @@ public class InclusiveByteRange
      * @param size Size of the resource.
      * @return LazyList of satisfiable ranges
      */
-    public static List satisfiableRanges(Enumeration headers,long size)
+    public static List satisfiableRanges(Enumeration headers, boolean allowRelativeRange, long size)
     {
         Object satRanges=null;
         
@@ -86,54 +86,66 @@ public class InclusiveByteRange
                 // read all byte ranges for this header 
                 while (tok.hasMoreTokens())
                 {
-                    t=tok.nextToken().trim();
+                    try
-                    
-                    long first = -1;
-                    long last  = -1;
-                    int d=t.indexOf('-');
-                    if (d<0 || t.indexOf("-",d+1)>=0)
-                    {           
-                        if ("bytes".equals(t))
-                            continue;
-                        Log.warn("Bad range format: {}",t);
-                        continue headers;
-                    }
-                    else if (d==0)
                     {
-                        if (d+1<t.length())
+                        t = tok.nextToken().trim();
-                            last = Long.parseLong(t.substring(d+1).trim());
+
-                        else
+                        long first = -1;
+                        long last = -1;
+                        int d = t.indexOf('-');
+                        if (d < 0 || t.indexOf("-",d + 1) >= 0)
                         {
+                            if ("bytes".equals(t))
+                                continue;
                             Log.warn("Bad range format: {}",t);
                             continue headers;
                         }
-                    }
+                        else if (d == 0)
-                    else if (d+1<t.length())
+                        {
-                    {
+                            if (d + 1 < t.length())
-                        first = Long.parseLong(t.substring(0,d).trim());
+                                last = Long.parseLong(t.substring(d + 1).trim());
-                        last = Long.parseLong(t.substring(d+1).trim());
+                            else
-                    }
+                            {
-                    else
+                                Log.warn("Bad range format: {}",t);
-                        first = Long.parseLong(t.substring(0,d).trim());
+                                continue;
+                            }
+                        }
+                        else if (d + 1 < t.length())
+                        {
+                            first = Long.parseLong(t.substring(0,d).trim());
+                            last = Long.parseLong(t.substring(d + 1).trim());
+                        }
+                        else
+                            first = Long.parseLong(t.substring(0,d).trim());
 
-                    
+                        if (first == -1 && last == -1)
-                    if (first == -1 && last == -1)
+                            continue headers;
-                        continue headers;
-                    
-                    if (first != -1 && last != -1 && (first > last))
-                        continue headers;
 
-                    if (first<size)
+                        if (first != -1 && last != -1 && (first > last))
+                            continue headers;
+
+                        if (first < size)
+                        {
+                            // Relative range end points not allowed (in some cases)
+                            if ((!allowRelativeRange) && ((first < 0) || (last < 0)))
+                            {
+                                continue headers;
+                            }
+                            InclusiveByteRange range = new InclusiveByteRange(first,last);
+                            satRanges = LazyList.add(satRanges,range);
+                        }
+                    }
+                    catch (NumberFormatException e)
                     {
-                        InclusiveByteRange range = new
+                        Log.warn("Bad range format: {}",t);
-                            InclusiveByteRange(first, last);
+                        Log.ignore(e);
-                        satRanges=LazyList.add(satRanges,range);
+                        continue;
                     }
                 }
             }
             catch(Exception e)
             {
-                Log.warn("Bad range format: "+t);
+                Log.warn("Bad range format: {}",t);
                 Log.ignore(e);
             }    
         }
@@ -195,6 +207,7 @@ public class InclusiveByteRange
 
 
     /* ------------------------------------------------------------ */
+    @Override
     public String toString()
     {
         StringBuilder sb = new StringBuilder(60);

jetty-server/src/test/java/org/eclipse/jetty/server/InclusiveByteRangeTest.java

@@ -0,0 +1,168 @@
+// ========================================================================
+// Copyright (c) Webtide LLC
+// ------------------------------------------------------------------------
+// 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.apache.org/licenses/LICENSE-2.0.txt
+//
+// You may elect to redistribute this code under either of these licenses. 
+// ========================================================================
+
+package org.eclipse.jetty.server;
+
+import java.util.List;
+import java.util.Vector;
+
+import junit.framework.TestCase;
+
+public class InclusiveByteRangeTest extends TestCase
+{
+    @SuppressWarnings("unchecked")
+    private void assertInvalidRange(String rangeString, boolean allowRelativeRange)
+    {
+        Vector strings = new Vector();
+        strings.add(rangeString);
+
+        List ranges = InclusiveByteRange.satisfiableRanges(strings.elements(),allowRelativeRange,200);
+        assertNull("Invalid Range [" + rangeString + "] should result in no satisfiable ranges",ranges);
+    }
+    
+    private void assertRange(String msg, int expectedFirst, int expectedLast, int size, InclusiveByteRange actualRange)
+    {
+        assertEquals(msg + " - first",expectedFirst,actualRange.getFirst(size));
+        assertEquals(msg + " - last",expectedLast,actualRange.getLast(size));
+        String expectedHeader = String.format("bytes %d-%d/%d",expectedFirst,expectedLast,size);
+        assertEquals(msg + " - header range string",expectedHeader,actualRange.toHeaderRangeString(size));
+    }
+
+    private void assertSimpleRange(int expectedFirst, int expectedLast, String rangeId, int size, boolean allowRelativeRanges)
+    {
+        InclusiveByteRange range = parseRange(rangeId,size,allowRelativeRanges);
+
+        assertEquals("Range [" + rangeId + "] - first",expectedFirst,range.getFirst(size));
+        assertEquals("Range [" + rangeId + "] - last",expectedLast,range.getLast(size));
+        String expectedHeader = String.format("bytes %d-%d/%d",expectedFirst,expectedLast,size);
+        assertEquals("Range [" + rangeId + "] - header range string",expectedHeader,range.toHeaderRangeString(size));
+    }
+
+    @SuppressWarnings("unchecked")
+    private InclusiveByteRange parseRange(String rangeString, int size, boolean allowRelativeRange)
+    {
+        Vector strings = new Vector();
+        strings.add(rangeString);
+
+        List ranges = InclusiveByteRange.satisfiableRanges(strings.elements(),allowRelativeRange,size);
+        assertNotNull("Satisfiable Ranges should not be null",ranges);
+        assertEquals("Satisfiable Ranges of [" + rangeString + "] count",1,ranges.size());
+        return (InclusiveByteRange)ranges.get(0);
+    }
+
+    @SuppressWarnings("unchecked")
+    private List<InclusiveByteRange> parseRanges(String rangeString, int size, boolean allowRelativeRange)
+    {
+        Vector strings = new Vector();
+        strings.add(rangeString);
+
+        List<InclusiveByteRange> ranges;
+        ranges = InclusiveByteRange.satisfiableRanges(strings.elements(),allowRelativeRange,size);
+        assertNotNull("Satisfiable Ranges should not be null",ranges);
+        return ranges;
+    }
+    
+    public void testHeader416RangeString()
+    {
+        assertEquals("416 Header on size 100","bytes */100",InclusiveByteRange.to416HeaderRangeString(100));
+        assertEquals("416 Header on size 123456789","bytes */123456789",InclusiveByteRange.to416HeaderRangeString(123456789));
+    }
+    
+    public void testInvalidRanges()
+    {
+        boolean relativeRange = true;
+
+        // Invalid if parsing "Range" header
+        assertInvalidRange("bytes=a-b",relativeRange); // letters invalid
+        assertInvalidRange("byte=10-3",relativeRange); // key is bad
+        assertInvalidRange("onceuponatime=5-10",relativeRange); // key is bad
+        assertInvalidRange("bytes=300-310",relativeRange); // outside of size (200) 
+        
+        // Invalid if parsing "Content-Range" header 
+        relativeRange = false;
+        assertInvalidRange("bytes=-5",relativeRange); // relative from end
+        assertInvalidRange("bytes=10-",relativeRange); // relative from start
+        assertInvalidRange("bytes=250-300",relativeRange); // outside of size (200)
+    }
+
+    /**
+     * Ranges have a multiple ranges, all absolutely defined.
+     */
+    public void testMultipleAbsoluteRanges()
+    {
+        boolean relativeRange = false;
+        int size = 50;
+        String rangeString;
+
+        rangeString = "bytes=5-20,35-65";
+
+        List<InclusiveByteRange> ranges = parseRanges(rangeString,size,relativeRange);
+        assertEquals("Satisfiable Ranges of [" + rangeString + "] count",2,ranges.size());
+        assertRange("Range [" + rangeString + "]",5,20,size,ranges.get(0));
+        assertRange("Range [" + rangeString + "]",35,49,size,ranges.get(1));
+    }
+
+    /**
+     * Range definition has a range that is clipped due to the size.
+     */
+    public void testMultipleRangesClipped()
+    {
+        String rangeString;
+
+        rangeString = "bytes=5-20,35-65,-5";
+
+        List<InclusiveByteRange> ranges = parseRanges(rangeString,50,true);
+        assertEquals("Satisfiable Ranges of [" + rangeString + "] count",3,ranges.size());
+        assertRange("Range [" + rangeString + "]",5,20,50,ranges.get(0));
+        assertRange("Range [" + rangeString + "]",35,49,50,ranges.get(1));
+        assertRange("Range [" + rangeString + "]",45,49,50,ranges.get(2));
+    }
+
+    public void testMultipleRangesOverlapping()
+    {
+        String rangeString;
+
+        rangeString = "bytes=5-20,15-25";
+
+        List<InclusiveByteRange> ranges = parseRanges(rangeString,200,true);
+        assertEquals("Satisfiable Ranges of [" + rangeString + "] count",2,ranges.size());
+        assertRange("Range [" + rangeString + "]",5,20,200,ranges.get(0));
+        assertRange("Range [" + rangeString + "]",15,25,200,ranges.get(1));
+    }
+
+    public void testMultipleRangesSplit()
+    {
+        String rangeString;
+
+        rangeString = "bytes=5-10,15-20";
+
+        List<InclusiveByteRange> ranges = parseRanges(rangeString,200,true);
+        assertEquals("Satisfiable Ranges of [" + rangeString + "] count",2,ranges.size());
+        assertRange("Range [" + rangeString + "]",5,10,200,ranges.get(0));
+        assertRange("Range [" + rangeString + "]",15,20,200,ranges.get(1));
+    }
+
+    public void testSimpleRange()
+    {
+        boolean relativeRange = true;
+
+        assertSimpleRange(5,10,"bytes=5-10",200,relativeRange);
+        assertSimpleRange(195,199,"bytes=-5",200,relativeRange);
+        assertSimpleRange(50,119,"bytes=50-150",120,relativeRange);
+        assertSimpleRange(50,119,"bytes=50-",120,relativeRange);
+    }
+
+}

jetty-servlet/src/main/java/org/eclipse/jetty/servlet/DefaultServlet.java

@@ -141,6 +141,7 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory
     
     
     /* ------------------------------------------------------------ */
+    @Override
     public void init()
         throws UnavailableException
     {
@@ -243,6 +244,7 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory
     }
 
     /* ------------------------------------------------------------ */
+    @Override
     public String getInitParameter(String name)
     {
         String value=getServletContext().getInitParameter("org.eclipse.jetty.servlet.Default."+name);
@@ -309,12 +311,14 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory
     }
     
     /* ------------------------------------------------------------ */
+    @Override
     protected void doGet(HttpServletRequest request, HttpServletResponse response)
     	throws ServletException, IOException
     {
         String servletPath=null;
         String pathInfo=null;
         Enumeration reqRanges = null;
+        boolean byteRangeRules = false;
         Boolean included =request.getAttribute(Dispatcher.INCLUDE_REQUEST_URI)!=null;
         if (included!=null && included.booleanValue())
         {
@@ -328,14 +332,25 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory
         }
         else
         {
-            included=Boolean.FALSE;
+            included = Boolean.FALSE;
-            servletPath=request.getServletPath();
+            servletPath = request.getServletPath();
-            pathInfo=request.getPathInfo();
+            pathInfo = request.getPathInfo();
-            
+
-            // Is this a range request?
+            // Is this a Content-Range request?
-            reqRanges = request.getHeaders(HttpHeaders.RANGE);
+            reqRanges = request.getHeaders(HttpHeaders.CONTENT_RANGE);
-            if (reqRanges!=null && !reqRanges.hasMoreElements())
+            if (!hasDefinedRange(reqRanges))
-                reqRanges=null;
+            {
+                // Is this a Range request?
+                reqRanges = request.getHeaders(HttpHeaders.RANGE);
+                if (hasDefinedRange(reqRanges))
+                {
+                    byteRangeRules = true;
+                }
+                else
+                {
+                    reqRanges = null;
+                }
+            }
         }
         
         String pathInContext=URIUtil.addPaths(servletPath,pathInfo);
@@ -438,7 +453,7 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory
 			   if (mt!=null)
 			       response.setContentType(mt);
 			}
-			sendData(request,response,included.booleanValue(),resource,content,reqRanges);  
+			sendData(request,response,included.booleanValue(),resource,content,reqRanges,byteRangeRules);  
 		    }
 		}
             }
@@ -520,7 +535,13 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory
         
     }
     
+    private boolean hasDefinedRange(Enumeration reqRanges)
+    {
+        return (reqRanges!=null && reqRanges.hasMoreElements());
+    }
+
     /* ------------------------------------------------------------ */
+    @Override
     protected void doPost(HttpServletRequest request, HttpServletResponse response)
         throws ServletException, IOException
     {
@@ -531,6 +552,7 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory
     /* (non-Javadoc)
      * @see javax.servlet.http.HttpServlet#doTrace(javax.servlet.http.HttpServletRequest, javax.servlet.http.HttpServletResponse)
      */
+    @Override
     protected void doTrace(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException
     {
         resp.sendError(HttpServletResponse.SC_METHOD_NOT_ALLOWED);
@@ -674,7 +696,8 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory
                             boolean include,
                             Resource resource,
                             HttpContent content,
-                            Enumeration reqRanges)
+                            Enumeration reqRanges,
+                            boolean byteRangeRules)
     throws IOException
     {
         long content_length=resource.length();
@@ -723,7 +746,7 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory
         else
         {
             // Parse the satisfiable ranges
-            List ranges =InclusiveByteRange.satisfiableRanges(reqRanges,content_length);
+            List ranges =InclusiveByteRange.satisfiableRanges(reqRanges,byteRangeRules,content_length);
             
             //  if there are no satisfiable ranges, send 416 response
             if (ranges==null || ranges.size()==0)
@@ -736,7 +759,6 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory
                 return;
             }
             
-            
             //  if there is only a single valid range (must be satisfiable 
             //  since were here now), send that range with a 216 response
             if ( ranges.size()== 1)
@@ -752,6 +774,17 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory
                 return;
             }
             
+            // If not following byte range rules (such as when using the "Content-Range" request)
+            // There is no possibility of sending a multi-part range.
+            // See http://tools.ietf.org/html/rfc2616#section-14.16
+            if (!byteRangeRules)
+            {
+                writeHeaders(response,content,content_length);
+                response.setStatus(HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE);
+                response.setHeader(HttpHeaders.CONTENT_RANGE,InclusiveByteRange.to416HeaderRangeString(content_length));
+                resource.writeTo(out,0,content_length);
+                return;
+            }
             
             //  multiple non-overlapping valid ranges cause a multipart
             //  216 response which does not require an overall 
@@ -881,6 +914,7 @@ public class DefaultServlet extends HttpServlet implements ResourceFactory
     /* 
      * @see javax.servlet.Servlet#destroy()
      */
+    @Override
     public void destroy()
     {
         try