net/http: Broken If-Range handling in ServeFile and friends unless caller sets ETag manually

[gopherbot]

What does 'go version' print?

go version go1.3 linux/amd64

What steps reproduce the problem?

1. You will need an MPEG4 version 2 video file that your browser agrees to play. It
needs to be the specific kind of mp4 that has metadata at the end of the file. I'm sorry
but I don't currently have a publicly available example of one. Put it as video.mp4 in
the directory with the attached files.

2. Download the attached *.go and *.html and "go run buggy-logging.go"

3. Open http://localhost:8000/
tested with Chrome 36.0.1985.97 beta on linux

4. Click play

As the bug depends on browser caching behavior, I find I typically need to "Empty
cache and hard reload", and then perform steps 3 & 4 twice to trigger it.

5. You should see just a black screen, no video playing. Log attached as bug.log. I have
done a similar run with "go run buggy.go", which skips the logging, to prove
that my logging kludge does not change behavior.

Log attached as "bug.log"

6. Control-C the buggy-logging version, go run etag-logging.go

7. "Empty cache and hard reload" and perform steps 3 & 4 (twice?) again.

8. You should see the video play.

Log attached as "etag.log"




The difference in the logs is:

GET /video.mp4
Range: bytes=798054665-798503344
If-Range: Wed, 25 Jun 2014 17:12:18 GMT

200 OK  this causes the bug

vs

GET /video.mp4
Range:bytes=798054665-798503344
If-Range: fake

206 OK  this works
Content-Range: bytes 798054665-798503344/798503345


The above seems to be caused by net/http not sending an Etag, only Last-Modified, but
only accepting ETags for If-Range. I would recommend either

1. Make net/http serveFile provide an ETag (if not already set in ResponseWriter) from
hashing FileInfo.Size(), .ModTime().Unix(), .ModTime().Nanosecond(), and on unix,
syscall.Stat_t.Ctim.Sec, .Ctim.Nsec, .Dev, .Ino
OR
2. checkETag() should fall back to w.Header().get("Last-Modified") if the
response has no ETag

The first one seems preferable, and the stat call is done already, so there's no real
cost to it.

Chrome should also probably know to handle the failure right. It seems like this is
reported as https://code.google.com/p/chromium/issues/detail?id=74852 -- but net/http
should not trigger that logic when no actual file change happened.

Attachments:

1. index.html (209 bytes)
2. buggy-logging.go (842 bytes)
3. buggy.go (484 bytes)
4. etag-logging.go (875 bytes)
5. bug.log (1082 bytes)
6. etag.log (2295 bytes)

[ianlancetaylor]

Comment 1:

Labels changed: added repo-main, release-go1.4.

[taruti]

Comment 2:

This issue also was a problem for us, did workaround by modifying net/http/fs.go
locally. The issue is that Chrome/ium sends a date in If-Range and that causes
net/http/fs.go to zero the Range. Around line 300 in fs.go there is a TODO about this.
Occurs in e.g. seeking large mp4 files with html5 videos.

[bradfitz]

Comment 3:

Let me try to re-summarize this bug more succinctly, without attachments. Please correct
me if this is wrong (or confirm if this is correct).
Browsers may send an If-Range header containing either an ETag or a date:
http://tools.ietf.org/html/rfc7233#section-3.2
    ....
    If-Range = entity-tag / HTTP-date
    ....
The Go code only deals with the entity-tag half, and in fact has a comment about nobody
seeming to care about sending a date, and the spec being unclear. From fs.go:
    if ir := r.Header.get("If-Range"); ir != "" && ir != etag {
        // TODO(bradfitz): handle If-Range requests with Last-Modified                                                  
        // times instead of ETags? I'd rather not, at least for                                                         
        // now. That seems like a bug/compromise in the RFC 2616, and                                                   
        // I've never heard of anybody caring about that (yet).                                                         
        rangeReq = ""
    }
I like the "yet" part. That time apparently is now and Chrome does send a date in
If-Range if the handler never sent an ETag.
Note that the comment is discussing RFC 2616, and not the clarification RFC 7233.  For
reference, 2616 said: http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.27
I'd rather not get into the business of guessing ETags for users. Maybe for on-disk
files that's acceptable, but it should be a separate bug.
For this bug, I think simply accepting an If-Range date matching the Last-Modified time
should be an acceptable fix.

Status changed to Accepted.

[gopherbot]

Comment 4:

Yes, the point of the attachments was purely to demonstrate that this is not a
theoretical concern: net/http cannot serve html5 video without workarounds, currently. 
ServeFile is already talking about on-disk files, so I don't see why an ETag *there*
would actively hurt. I don't think net/http should provide ETags for just any resource.

[bradfitz]

Comment 5:

ServeFile is a thin wrapper around ServeContent. The missing If-Range date handling is
in ServeContent.
Adding ETag in the way proposed could actively hurt: imagine a load-balanced environment
with Go servers on many backends with different filesystems. The inode number could
differ and cause problems if mixed in to the ETag. Again, please open a separate bug for
that if you feel strongly and it can be discussed or designed separately. This bug
should be about the date handling only.

[gopherbot]

Comment 6:

CL https://golang.org/cl/116300044 mentions this issue.

[bradfitz]

Comment 7:

This issue was closed by revision f5037ee.

Status changed to Fixed.