dukecon / dukecon_server

MIT License
12 stars 6 forks source link

Server doesn't reply with "304 not modified" if content hasn't changed #68

Open ahus1 opened 7 years ago

ahus1 commented 7 years ago

When retrieving a rest resource the server never answers "304 not modified" when the client sends the etag.

Example: Open https://latest.dukecon.org/javaland/2017/rest/conferences/javaland2017/ in your browser. Call the same URL again (but not press F5). You'll see "If-None-Match" in the request, the server answers with exactly the same Etag and 200, but not 304.

This leads to users retrieving more data than needed and slower responses.

As this has broken without notice and is hard to detect, this issue needs an automatic test as well.

https://github.com/dukecon/dukecon_pwa/issues/1

sippsack commented 7 years ago

You are right, didn't recognize this is broken. Maybe it has something to do with spring boot update.

sippsack commented 7 years ago

It works on localhost: image

It does not work on prod and latest: image

@ascheman: maybe the Apache is causing this behaviour?

ahus1 commented 7 years ago

There is a question on SO describing that the mod_deflate Apache module adds the suffix -gzip and therefore breaks caching.

https://stackoverflow.com/questions/29127144/cant-cache-resource-when-having-both-gzip-and-etag

The solution described at SO is to not modify the etag. I suggest that it would be even better to keep the etag suffix when sending it out to the browser but to remove any gzip suffix from any etag received in a request before handing it over to spring boot.

ascheman commented 7 years ago

The SO article suggests to perform a header edit on a certain location:

<Location /js>
  RequestHeader  edit "If-None-Match" "^(.*)-gzip$" "$1"
  Header  edit "ETag" "^(.*[^g][^z][^i][^p])$" "$1-gzip"
</Location>

Should we do that to all requests to '*/rest/*'? The right way to do that is probably the LocationMatch Apache directive (https://httpd.apache.org/docs/current/mod/core.html#locationmatch)? I am not sure if Puppet supports that?

ahus1 commented 7 years ago

@ascheman - i suggest that we do it on all requests, independent of the location. I suppose it also breaks caching of all javascript resources at the moment.

Good luck with puppet :-(

ahus1 commented 6 years ago

@ascheman / CC: @sippsack - there is now a workaround in dukecon server for this. See commit referenced in this ticket. If you consider it not only a fix but a solution please close this issue.

sippsack commented 6 years ago

works on latest, so we can close this ticket in my opinion