Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: net/http: ServeContentStream #14896

Closed
colinmarc opened this issue Mar 21, 2016 · 5 comments

Comments

Projects
None yet
5 participants
@colinmarc
Copy link

commented Mar 21, 2016

net/http.ServeContent is an extremely useful utility for serving byte blobs and files. However, it requires an io.ReadSeeker with a valid Seek, so that it can a) determine the total size and b) seek around in the file to serve content ranges.

I recently found myself in a situation where I had an io.Reader, and really wanted to be able to handle Accept-Range, If-Not-Modified, etc. I had the total size and modtime ahead of time, so that was taken care of. But because ServeContent takes an io.ReadSeeker, I was left with two choices: either wrap the stream in an object that can do a fake "seek", or reimplement stuff like Accept-Range parsing, which is super fiddly.

I think my situation probably isn't that uncommon - any time you're serving data from an underlying store or database which supports streaming data off disk, but doesn't let you seek around in it, and you want to serve that over HTTP with Accept-Range support, you end up with this problem. To that end, I'd like to propose a sister method to ServeContent, maybe something like:

func ServeContentStream(w ResponseWriter, req *Request, name string, modtime time.Time, size int64, content io.Reader)

The implementation should be pretty simple. serveContent, which ServeContent backends to, already accepts a size argument (or more accurately, a sizeFunc). It could be changed to take an io.Reader as well, and attempt to type-assert the reader to an io.Seeker when and if it needs to. If that fails, it could discard bytes off the stream in order to serve successive ranges.

Note that the bits that require Seek are only for Accept-Range, where the ranges are known in advance and can be ordered; a backwards seek is never required. Discarding bytes is obviously not ideal; but it's way better than buffering the whole blob into memory to wrap it in a bytes.Reader.

Obviously, I'd be happy to put in the work myself to make this change! Just filing an issue first since that's what the docs say to do. =)

@ianlancetaylor ianlancetaylor changed the title Proposal: net/http.ServeContentStream proposal: net/http: ServeContentStream Mar 21, 2016

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2016

@bradfitz

This comment has been minimized.

Copy link
Member

commented Mar 22, 2016

I think ServeContent already had too many parameters and I wouldn't have done the ReadSeeker thing again in retrospect. I don't want to repeat those mistakes with even more parameters.

I'd prefer something like:

// ConditionalHandler wraps returns an http.Handler wrapping h which
// adds support for HTTP conditional requests as defined by RFC 7232.
// The returned handler answers If-Modified-Since and If-None-Match
// queries using the headers set on the response to HEAD requests against h.
func ConditionalHandler(h http.Handler) http.Handler
@colinmarc

This comment has been minimized.

Copy link
Author

commented Mar 22, 2016

That seems useful, although it doesn't handle range requests. Perhaps that could be solved by exporting parseRange?

@extemporalgenome

This comment has been minimized.

Copy link

commented Apr 4, 2016

Trying to fit that problem into the existing API, I'd suggest creating a function that turns any io.Reader with a Size() int64 method into a restricted io.ReadSeeker that accepts "start" and "end" whences with value 0, but returns an error for any other seeks.

@colinmarc are you saying these DB streaming APIs and such allow you to start streaming from an arbitrary offset, but that same stream is then unseekable? If so, you can provide a ReadSeeker that doesn't actually "open" the stream until the first Read occurs (in which case you just open it from wherever the offset happens to be). AFAIK, ServeContent does not seek into the middle of the ReadSeeker more than once.

If the answer is "no, the database may only stream from the beginning", then Accept-Ranges: none is the only meaningful response (or omit the header entirely). Aside from live data streams (with no history), large response streaming is essentially non-robust if it doesn't allow for resumable or partial transfers.

@bradfitz bradfitz modified the milestone: Unplanned Apr 7, 2016

@bradfitz

This comment has been minimized.

Copy link
Member

commented Aug 15, 2016

I never saw a reply from @colinmarc from @extemporalgenome's question, so I'm going to close this for now.

I'm also worried about this part:

If that fails, it could discard bytes off the stream in order to serve successive ranges.

I don't think that's something we want to be doing automatically for people. If you don't have a ReadSeeker, we shouldn't be transparently reading potentially gigabytes of data to get to your Range seek position.

@bradfitz bradfitz closed this Aug 15, 2016

@golang golang locked and limited conversation to collaborators Aug 15, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.