Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

return "Accept-Ranges" header when making file requests #666

Open
wants to merge 1 commit into from

4 participants

@nuex

No description provided.

@mikegee

Could you provide some context for this?

@nuex

Clients downloading part of the file using the Range header will typically make a HEAD request and check the Accept-Ranges header to see if ranges are supported.

Satan is using a local rack file server for testing a client app, but rack isn't returning Accept-Ranges so the client can test if Range is supported.

@mikegee

Thanks. That's helpful.

I just read the spec on Accept-Ranges, and it indicates the header should be conditional based on the server supporting byte-range requests. Does rack always support byte ranges, or does it depend on the server that runs it?

@nuex

This only affects Rack::File which supports range requests.

@mikegee

Thanks again @nuex

@nuex

You're welcome!

@raggi
Owner

If I remember correctly, we do not accept multiple ranges. Until we do, this seems unadvisable, what do you think?

@raggi raggi added this to the Rack 1.6 milestone
@raggi raggi modified the milestone: Rack 1.6, Rack 2.0
@cjlarose

According to RFC 2616 §14.5 Accept-Ranges, and revalidated by RFC 7233 §2.3:

An origin server that supports byte-range requests for a given target
resource MAY send

Accept-Ranges: bytes

to indicate what range units are supported. A client MAY generate
range requests without having received this header field for the
resource involved. Range units are defined in Section 2.

A server that does not support any kind of range request for the
target resource MAY send

Accept-Ranges: none

So there doesn't seem to be a violation of the spec in the current implementation. However, I can't find any specific references to what a server should do in the event that, like Rack, the server doesn't support some content ranges.

In these cases, Rack today ignores the header and responds to such requests with the entire document. I think it might be more appropriate to serve instead 501 Unimplemented, which is defined by the RFC as "The server does not support the functionality required to fulfill the request."

So I think the best way to handle this is to send out the Accept-Ranges header like the PR does, but also return a 501 Unimplemented response in case Rack receives multiple byte ranges.

@raggi
Owner

I strongly disagree. What we do today makes requests work for clients, and is only problematic for people serving very large files. Those users can handle their special case in a special way. General users should always receive behavior that is most "working" as far as their customers are concerned, and serving up a single range or whole file is suitable for that.

If you want to change this for the better, then please improve our range implementation instead. We do have one, it's here: https://github.com/rack/rack/blob/master/lib/rack/file.rb#L81-L98

@cjlarose

I think I worded my recommendation in way that you might have misinterpreted. I meant to suggest that Rack serve 501 Unimplemented only in the case that Rack receives a Range requests that it cannot fulfill (one with multiple ranges). Otherwise, Rack should most definitely serve single-range and whole file requests without a hitch. Most clients will never notice.

In either case, you're right that a more complete range implementation would make the point moot. I'd be happy to submit a PR.

@raggi
Owner

What use case would this enable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 7 additions and 1 deletion.
  1. +1 −1  lib/rack/file.rb
  2. +6 −0 test/spec_file.rb
View
2  lib/rack/file.rb
@@ -63,7 +63,7 @@ def serving(env)
last_modified = F.mtime(@path).httpdate
return [304, {}, []] if env['HTTP_IF_MODIFIED_SINCE'] == last_modified
- headers = { "Last-Modified" => last_modified }
+ headers = {'Last-Modified' => last_modified, 'Accept-Ranges' => 'bytes'}
mime = Mime.mime_type(F.extname(@path), @default_mime)
headers["Content-Type"] = mime if mime
View
6 test/spec_file.rb
@@ -25,6 +25,12 @@ def file(*args)
res["Last-Modified"].should.equal File.mtime(path).httpdate
end
+ should "set Accept-Ranges header" do
+ res = Rack::MockRequest.new(file(DOCROOT)).get("/cgi/test")
+ res.should.be.ok
+ res["Accept-Ranges"].should.equal "bytes"
+ end
+
should "return 304 if file isn't modified since last serve" do
path = File.join(DOCROOT, "/cgi/test")
res = Rack::MockRequest.new(file(DOCROOT)).
Something went wrong with that request. Please try again.