From 9a01e3d4920c675219cf45d01532497c4af5dfbf Mon Sep 17 00:00:00 2001 From: Jens Alfke Date: Sun, 17 Oct 2010 12:31:07 -0700 Subject: [PATCH] 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 --- CHANGES | 3 ++ lib/sinatra/base.rb | 77 +++++++++++++++++++++++++++++++++++++++++---- test/static_test.rb | 64 +++++++++++++++++++++++++++++++++++++ 3 files changed, 138 insertions(+), 6 deletions(-) diff --git a/CHANGES b/CHANGES index b4251750..b14d92f9 100644 --- a/CHANGES +++ b/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. diff --git a/lib/sinatra/base.rb b/lib/sinatra/base.rb index e280d852..91a3f63d 100644 --- a/lib/sinatra/base.rb +++ b/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_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 diff --git a/test/static_test.rb b/test/static_test.rb index ba6869b5..bd3b5da7 100644 --- a/test/static_test.rb +++ b/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