search

knutin / elli

Simple, robust and performant Erlang web server
MIT License
663 stars 79 forks source link

{ok, [...], {file, FileName, Start, End}} wanted #36

Closed dvv closed 11 years ago

dvv commented 11 years ago

Hi!

I'd like to implement serving ranges of file based on Range: request header. Wonder if you could add signature from the subj to make partial sendfile.

TIA

zambal commented 11 years ago

By total coincidence, I just implemented this and added a pull request for the code, before I saw your comment.

Please check https://github.com/knutin/elli/pull/38 and let me know what you think.

dvv commented 11 years ago

magic :) well, i believe passing request headers is not good -- there is fileopts already for that. or, if we do want to pass range header, let us pass namely it. will be supplying some code on parsing ranges in an hour.

dvv commented 11 years ago

https://github.com/dvv/cowboy-citats/blob/master/src/cowboy_citats_range.erl -- here i drafted a generic Range: parser. It honors negative offsets and lengths as well. Ranges in form of list of tuples can be passed as {ok, [...], {file, FileName, [{bytes,[{start1,end1},{start2,end2},...]}]}} to send_file.

zambal commented 11 years ago

Hi, I was yesterday too busy improving my own implementation and missed your replies. I guess it's magic indeed that we both made an implementation at virtually the same time :)

Why would passing the request headers be a bad idea? Since the behavior of send_file depends on a possible requested byte range, it seems sensible to check if such a range is present inside send_file self.

Regarding negative offsets and lengths, if one would follow the rfc for HTTP ranges, negative offsets or lengths should be regarded as not satisfiable, so I choose for this behavior.

EDIT: and using suffix-lengths give the same benefits as wrapping negative numbers.

I guess that in the end the differences between our implementations are mostly a matter of taste.

zambal commented 11 years ago

Hi, your remark about passing the request headers to send_file being a bad idea eventually triggered some ideas about how to refactor send_file for better readability and maintainability (at least, that's the intention :). I just committed the changes to my branch, so if you like, take a look and let me know what you think.

I'm done for now with working on this patch and wait for Knut's response about what/whose code to use.

cheers, vincent