Re-implement byte-range support for static files.

Correct handling of "Range:" request header. Replaces buggy implementation (480b1e8, 44ab090) that was recently backed out.
Fixes #93.

NOTE: Does not yet support multiple ranges (e.g. "bytes=1-10,20-30") because that requires sending a multipart response, which is more complex than I want to get into now.

Signed-off-by: Konstantin Haase <konstantin.mailinglists@googlemail.com>

CHANGES

@@ -1,5 +1,8 @@
 = 1.1 / Not Yet Released
 
+ * Now supports byte-range requests (the HTTP_RANGE header) for static files.
+   Multi-range requests are not supported, however. (Jens Alfke)
+
  * Before and after filters now support pattern matching, including the
    ability to use captures: "before('/user/:name') { |name| ... }". This
    avoids manual path checking. No performance loss if patterns are avoided.

lib/sinatra/base.rb

@@ -160,15 +160,25 @@ module Sinatra
         response['Content-Type'] ||
         'application/octet-stream'
 
-      response['Content-Length'] ||= (opts[:length] || stat.size).to_s
-
       if opts[:disposition] == 'attachment' || opts[:filename]
         attachment opts[:filename] || path
       elsif opts[:disposition] == 'inline'
         response['Content-Disposition'] = 'inline'
       end
 
-      halt StaticFile.open(path, 'rb')
+      file_length = opts[:length] || stat.size
+      sf = StaticFile.open(path, 'rb')
+      if ! sf.parse_ranges(env, file_length)
+        response['Content-Range'] = "bytes */#{file_length}"
+        halt 416
+      elsif r=sf.range
+        response['Content-Range'] = "bytes #{r.begin}-#{r.end}/#{file_length}"
+        response['Content-Length'] = (r.end - r.begin + 1).to_s
+        halt 206, sf
+      else
+        response['Content-Length'] ||= file_length.to_s
+        halt sf
+      end
     rescue Errno::ENOENT
       not_found
     end
@@ -177,10 +187,65 @@ module Sinatra
     # generated iteratively in 8K chunks.
     class StaticFile < ::File #:nodoc:
       alias_method :to_path, :path
+
+      attr_accessor :range  # a Range or nil
+
+      # Checks for byte-ranges in the request and sets self.range appropriately.
+      # Returns false if the ranges are unsatisfiable and the request should return 416.
+      def parse_ranges(env, size)
+        #r = Rack::Utils::byte_ranges(env, size)  # TODO: not available yet in released Rack
+        r = byte_ranges(env, size)
+        return false if r == []  # Unsatisfiable; report error
+        @range = r[0] if r && r.length == 1  # Ignore multiple-range requests for now
+        return true
+      end
+
+      # TODO: Copied from the new method Rack::Utils::byte_ranges; this method can be removed once
+      # a version of Rack with that method is released and Sinatra can depend on it.
+      def byte_ranges(env, size)
+        # See <http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.35>
+        http_range = env['HTTP_RANGE']
+        return nil unless http_range
+        ranges = []
+        http_range.split(/,\s*/).each do |range_spec|
+          matches = range_spec.match(/bytes=(\d*)-(\d*)/)
+          return nil  unless matches
+          r0,r1 = matches[1], matches[2]
+          if r0.empty?
+            return nil  if r1.empty?
+            # suffix-byte-range-spec, represents trailing suffix of file
+            r0 = [size - r1.to_i, 0].max
+            r1 = size - 1
+          else
+            r0 = r0.to_i
+            if r1.empty?
+              r1 = size - 1
+            else
+              r1 = r1.to_i
+              return nil  if r1 < r0  # backwards range is syntactically invalid
+              r1 = size-1  if r1 >= size
+            end
+          end
+          ranges << (r0..r1)  if r0 <= r1
+        end
+        ranges
+      end
+
+      CHUNK_SIZE = 8192
+
       def each
-        rewind
-        while buf = read(8192)
-          yield buf
+        if @range
+          self.pos = @range.begin
+          length = @range.end - @range.begin + 1
+          while length > 0 && (buf = read([CHUNK_SIZE,length].min))
+            yield buf
+            length -= buf.length
+          end
+        else
+          rewind
+          while buf = read(CHUNK_SIZE)
+            yield buf
+          end
         end
       end
     end

test/static_test.rb

@@ -90,4 +90,68 @@ class StaticTest < Test::Unit::TestCase
     get "/../#{File.basename(__FILE__)}"
     assert not_found?
   end
+
+  def test_valid_range(http_range, range, path, file)
+    request = Rack::MockRequest.new(@app)
+    response = request.get("/#{File.basename(path)}", 'HTTP_RANGE' => http_range)
+
+    should_be = file[range]
+    expected_range = "bytes #{range.begin}-#{range.end}/#{file.length}"
+
+    assert_equal 206,response.status, "Should be HTTP/1.1 206 Partial content"
+    assert_equal should_be.length, response.body.length, "Unexpected response length for #{http_range}"
+    assert_equal should_be, response.body, "Unexpected response data for #{http_range}"
+    assert_equal should_be.length.to_s, response['Content-Length'], "Incorrect Content-Length for #{http_range}"
+    assert_equal expected_range, response['Content-Range'], "Incorrect Content-Range for #{http_range}"
+  end
+
+  it 'handles valid byte ranges correctly' do
+    # Use the biggest file in this dir so we can test ranges > 8k bytes. (StaticFile sends in 8k chunks.)
+    path = File.dirname(__FILE__) + '/helpers_test.rb'  # currently 16k bytes
+    file = File.read(path)
+    length = file.length
+    assert length > 9000, "The test file #{path} is too short (#{length} bytes) to run these tests"
+
+    [0..0, 42..88, 1234..1234, 100..9000, 0..(length-1), (length-1)..(length-1)].each do |range|
+      test_valid_range("bytes=#{range.begin}-#{range.end}", range, path, file)
+    end
+
+    [0, 100, length-100, length-1].each do |start|
+      test_valid_range("bytes=#{start}-", (start..length-1), path, file)
+    end
+
+    [1, 100, length-100, length-1, length].each do |range_length|
+      test_valid_range("bytes=-#{range_length}", (length-range_length..length-1), path, file)
+    end
+
+    # Some valid ranges that exceed the length of the file:
+    test_valid_range("bytes=100-999999", (100..length-1), path, file)
+    test_valid_range("bytes=100-#{length}", (100..length-1), path, file)
+    test_valid_range("bytes=-#{length}", (0..length-1), path, file)
+    test_valid_range("bytes=-#{length+1}", (0..length-1), path, file)
+    test_valid_range("bytes=-999999", (0..length-1), path, file)
+  end
+
+  it 'correctly ignores syntactically invalid range requests' do
+    # ...and also ignores multi-range requests, which aren't supported yet
+    ["bytes=45-40", "bytes=IV-LXVI", "octets=10-20", "bytes=-", "bytes=1-2,3-4"].each do |http_range|
+      request = Rack::MockRequest.new(@app)
+      response = request.get("/#{File.basename(__FILE__)}", 'HTTP_RANGE' => http_range)
+
+      assert_equal 200,response.status, "Invalid range '#{http_range}' should be ignored"
+      assert_equal nil,response['Content-Range'], "Invalid range '#{http_range}' should be ignored"
+    end
+  end
+
+  it 'returns error 416 for unsatisfiable range requests' do
+    # An unsatisfiable request is one that specifies a start that's at or past the end of the file.
+    length = File.read(__FILE__).length
+    ["bytes=888888-", "bytes=888888-999999", "bytes=#{length}-#{length}"].each do |http_range|
+      request = Rack::MockRequest.new(@app)
+      response = request.get("/#{File.basename(__FILE__)}", 'HTTP_RANGE' => http_range)
+
+      assert_equal 416,response.status, "Unsatisfiable range '#{http_range}' should return 416"
+      assert_equal "bytes */#{length}",response['Content-Range'], "416 response should include actual length"
+    end
+  end
 end