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

net/http: ServeFile should recognise unix epoch as "zero time" ? #9842

Closed
adob opened this issue Feb 11, 2015 · 8 comments

Comments

Projects
None yet
5 participants
@adob
Copy link

commented Feb 11, 2015

The check for timestamp validity in http.ServeFile() is not sufficient to detect invalid timestamps on unix systems. On App Engine, all files have the modified date set to 1 Jan 1970 (the unix epoch). In this environment, http.ServeFile() serves files with the header

  Last-Modified: Thu, 01 Jan 1970 00:00:00 GMT

regardless of the actual last modified date of the file. This is a problem because it means files may be cached on client browsers forever. This happens because the client may request the file with

 GET /file.html HTTP/1.1
 If-Modified-Since: Thu, 01 Jan 1970 00:00:00 GMT

Then App Engine will replies with

  304 Not Modified.

In the file src/net/http/fs.go, there is an attempt to check if the modified timestamp is valid by calling Time.IsZero() but this checks if the date is Jan 1, 1 AD, which is always false for all unix timestamps. It probably makes sense to add an explicit check for unix timestamp == 0 to serve ServeFile and convert this to Time.Zero.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Feb 11, 2015

/cc @dsymonds for thoughts. There might be multiple fixes here.

@bradfitz bradfitz added this to the Go1.5 milestone Feb 11, 2015

@bradfitz bradfitz self-assigned this Feb 11, 2015

@adg

This comment has been minimized.

Copy link
Contributor

commented Feb 12, 2015

On 11 February 2015 at 20:35, Aleksandr Dobkin notifications@github.com
wrote:

On App Engine, all files have the modified date set to 1 Jan 1970 (the
unix epoch)

Seems to me that this is the problem.
The modified date should be the date of deployment.

@dsymonds

This comment has been minimized.

Copy link
Member

commented Feb 12, 2015

If http.ServeFile is looking at the local disk state (which isn't a real filesystem), there's going to be failure. App files don't have mtime (or any other *time) recorded. I'm not surprised to see them appear with mtime at the Unix epoch.

@dsymonds

This comment has been minimized.

Copy link
Member

commented Feb 12, 2015

It seems like fs.go should be checking Unix epoch (and maybe the Mac epoch too?), not just Time.IsZero.

@adg

This comment has been minimized.

Copy link
Contributor

commented Feb 12, 2015

Isn't it a failure of the FS layer to provide shoddy mtimes?

What if a file really was created at unix epoch? What does fs.go do then?

On 12 February 2015 at 02:13, David Symonds notifications@github.com
wrote:

It seems like fs.go should be checking Unix epoch (and maybe the Mac
epoch too?), not just Time.IsZero.


Reply to this email directly or view it on GitHub
#9842 (comment).

@dsymonds

This comment has been minimized.

Copy link
Member

commented Feb 12, 2015

On 12 February 2015 at 13:41, Andrew Gerrand notifications@github.com wrote:

Isn't it a failure of the FS layer to provide shoddy mtimes?

Maybe. App Engine doesn't give you a real filesystem, and you don't
have other things like symlinks or executable bits, nor does it
preserve other metadata like user/group.

What if a file really was created at unix epoch? What does fs.go do then?

Dunno, but the safe option would be to pretend that the mtime is
effectively unknown, which is what it appears to be trying to do by
using Time.IsZero, right? It seems like a mostly academic concern
for actual files with a real mtime set to Unix epoch.

@adg

This comment has been minimized.

Copy link
Contributor

commented Feb 12, 2015

I guess the test at the top of checkLastModified could be modified to check if modtime.UnixNano == 0 too.

@adg adg changed the title http.ServeFile caching headers on App Engine net/http: ServeFile should recognise unix epoch as "zero time" ? Feb 12, 2015

@bradfitz

This comment has been minimized.

Copy link
Member

commented Mar 23, 2015

@bradfitz bradfitz closed this in a0fb8f8 Mar 23, 2015

@golang golang locked and limited conversation to collaborators Jun 25, 2016

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.