Skip to content
This repository has been archived by the owner on Mar 20, 2021. It is now read-only.

Inconsistent IfModifiedSince behavior #1881

Closed
glassfishrobot opened this issue Dec 23, 2016 · 5 comments
Closed

Inconsistent IfModifiedSince behavior #1881

glassfishrobot opened this issue Dec 23, 2016 · 5 comments
Assignees

Comments

@glassfishrobot
Copy link

In our OpenTripPlanner deployment, we have recently noticed strange caching behavior which required us to force-refresh after redeploying the app. After some investigation, it appears that the Grizzly FileCache is using an incorrect check to determine whether to send back a 304 status or a 200 status. (CUTR-at-USF/usf-mobullity#390 for a little more context)

By playing with the HTTP headers I found that setting an If-Modified-Since in the past nearly always returned a 304 status (which seems wrong to me), and specifying one in the future returned a 200 status (also wrong). Other HTTP servers seem to have the opposite behavior – specifying the If-Modified-Since header in the past == a 200 OK (the file changed), and specifying the If-Modified-Since >= the current file mtime returns a 304 status as expected.

I tracked this change down to https://github.com/GrizzlyNIO/grizzly-mirror/blob/2.3.x/modules/http-server/src/main/java/org/glassfish/grizzly/http/server/filecache/FileCache.java#L822

Following the Github mirror's "blame" yields https://github.com/GrizzlyNIO/grizzly-mirror/commit/f4cbe3468623067bbf74e38bc0172c10845388ed from 2012, and if you notice, the condition changed from:

(lastModified < headerValue + 1000) (true == 304 not modified)
to
(headerValue - lastModified <= 1000)

which is also the same condition used by IfUnmodifiedSince (clue that something was amiss). Also, the inequalities are fundamentally different. In order to maintain the same logic as the first, the order of headerValue and lastModified should be reversed, or the "<=" should be ">=" instead.

Oddly enough, the test case was written in such a way that my patch fails, but I believe the test is also wrong, so I changed it and added a third case to specifically check the IfModifiedSince behavior.

May be related to https://java.net/jira/browse/GRIZZLY-1222

I've submitted the simpler fix ">=" and test case to the following gist:
https://gist.github.com/jmfield2/a51b4970b1a8e97db4427533019c117a

Hopefully this is now correct and helpful

Thank you

Environment

Linux

Affected Versions

[2.3.28]

@glassfishrobot
Copy link
Author

Reported by jmfield2

@glassfishrobot
Copy link
Author

@jashap said:
See the following commits:
2.3.x: 643a55f
master: ccd3cc0

@glassfishrobot
Copy link
Author

Marked as fixed on Friday, January 6th 2017, 11:06:59 am

@glassfishrobot
Copy link
Author

jmfield2 said:
Thanks!

@glassfishrobot
Copy link
Author

This issue was imported from java.net JIRA GRIZZLY-1881

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants