-
Notifications
You must be signed in to change notification settings - Fork 197
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
HTML resources HTTP Caching is broken on webcompat.com #609
Comments
Yeah, especially since all the dynamic stuff is happening on the client side there's no good reason to not heavily cache the HTML. |
Working on this. |
@karlcow are you adding AppCache or what is your plan? |
@zoepage the issue is about HTTP caching for HTML resources. Currently if we request an issue
The HTTP response has no HTTP caching headers HTTP/1.1 200 OK
Connection: keep-alive
Content-Encoding: gzip
Content-Type: text/html; charset=utf-8
Date: Thu, 19 Jan 2017 12:48:55 GMT
Server: nginx/1.1.19
Strict-Transport-Security: max-age=31536000
Transfer-Encoding: chunked
Vary: Accept-Encoding I'm adding Does it help to understand? |
And in case, someone else reading this issue wants to know how HTTP caching is working, there is a very good resource: Caching Tutorial for Web Authors and Webmasters |
let me restart a branch for this. So I do it against the latest master. |
Fixes also a typo for cache-control.
* Check the body is empty * Check the Cache-Control header has the right value.
* It sets an etag and a cache control for one year. * The etag is based on the content of the response. So if the template changes, the etag will change. If in the future we decide to generate everything on the server side, this should still work.
Issue webcompat#609 - Removes code dust
Let's also make sure we don't break the test in #1371 when we fix it too, I think this changeset caused the permafail (which makes sense, it was a test on an issue page that involved logging in). |
Revert "Issue #609 - Implement Cache-Policy decorator"
I wonder if the reason it broke is because we serve different HTML if the user is authed: Not sure how to fix that. 🤔 |
Interesting issue. Let me check it. 😄 |
On a clean profile. No previous cookies.
I go to the homepage on localhost. Click on login. Then boom
hmm interesting. Let's stop the server. Quit the browser. Restart with a fresh profile.
Let's see the network panel. The logout doesn't seem to have issues. It is recorded. with a the appropriate location. HTTP/1.0 302 FOUND
Content-Type: text/html; charset=utf-8
Content-Length: 271
Location: http://localhost:5000/issues/522
Set-Cookie: session=********; HttpOnly; Path=/
Server: Werkzeug/0.10.4 Python/2.7.10
Date: Mon, 06 Mar 2017 02:05:21 GMT The request headers didn't have anything related to caching GET /logout HTTP/1.1
Host: localhost:5000
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:54.0) Gecko/20100101 Firefox/54.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
Referer: http://localhost:5000/issues/522
Cookie: session=*****
Connection: keep-alive
Upgrade-Insecure-Requests: 1 What is happening? Why is it working on the home page and not on the issue page. Ah gotcha! It DOES login or logout. |
So indeed. This is a templating issue. webcompat.com/webcompat/templates/shared/shared-nav-links.html Lines 7 to 12 in 5d7604b
{% if not session.user_id %}
<a class="wc-Navbar-link js-login-link" href="{{ url_for('login') }}">
<span class="wc-Navbar-link-icon wc-Icon wc-Icon--arrow-circle-right" aria-hidden="true"></span>
<span class="wc-Navbar-link-label">Log in</span>
</a>
{% else %}
<div class="r-ResetButton wc-Navbar-link js-login-link wc-DropdownHeader">
<button class="r-ResetButton" aria-pressed="false">
<img class="wc-Navbar-avatar" src="{{ session.avatar_url }}s=50" alt="User Avatar">
<span class="wc-Navbar-link-icon wc-Icon wc-Icon--chevron-down" aria-hidden="true"></span>
</button>
<div class="wc-DropdownHeader-content">
<a class="wc-Navbar-linkDropdown" href="{{ url_for('me_redirect') }}">My Activity</a>
<a class="wc-Navbar-linkDropdown" href="{{ url_for('logout') }}">Logout</a>
</div>
</div>
{% endif %} |
I need to test a couple of things. Because the content should be changing, hence the etag. |
Let get's to http://localhost:5000/issues/200 with a fresh profile GET /issues/515 HTTP/1.1
Host: localhost:5000
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:54.0) Gecko/20100101 Firefox/54.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
Referer: http://localhost:5000/
Cookie: ***
Connection: keep-alive
Upgrade-Insecure-Requests: 1 and Response is: HTTP/1.0 200 OK
Content-Type: text/html; charset=utf-8
Content-Length: 9366
Cache-Control: private, max-age=86400
Etag: "87f1ed4981ad4a15ae5a2fe84f250ac2"
Date: Mon, 06 Mar 2017 04:41:43 GMT
Server: Werkzeug/0.10.4 Python/2.7.10 This is normal. Let's do a reload GET /issues/515 HTTP/1.1
Host: localhost:5000
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:54.0) Gecko/20100101 Firefox/54.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
Referer: http://localhost:5000/issues?page=1&per_page=50&state=open&stage=needstriage&sort=created&direction=desc
Cookie: session=****
Connection: keep-alive
Upgrade-Insecure-Requests: 1
If-None-Match: "87f1ed4981ad4a15ae5a2fe84f250ac2"
Cache-Control: max-age=0 Then the response is: HTTP/1.0 304 NOT MODIFIED
Cache-Control: private, max-age=86400
Etag: "87f1ed4981ad4a15ae5a2fe84f250ac2"
Date: Mon, 06 Mar 2017 04:45:07 GMT
Connection: close
Server: Werkzeug/0.10.4 Python/2.7.10 Working! Ok let's click the login button. We know that the login is working, but the page doesn't show the new page with the different template. The request headers according to developer tools (I'm dubious says) GET /issues/515 HTTP/1.1
Host: localhost:5000
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:54.0) Gecko/20100101 Firefox/54.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
Referer: https://github.com/
Cookie: session=*****
Connection: keep-alive
Upgrade-Insecure-Requests: 1 and the response headers Content-Type: text/html; charset=utf-8
Content-Length: 9366
Cache-Control: private, max-age=86400
Etag: "87f1ed4981ad4a15ae5a2fe84f250ac2"
Server: Werkzeug/0.10.4 Python/2.7.10
Date: Mon, 06 Mar 2017 04:45:07 GMT but developer tools says also that it comes from the cache and indeed the cookie session is not in there for the response and on the server side log
There is no request to… the issue URI. So the request is going through. if I do another force reload my login state is shown. GET /issues/515 HTTP/1.1
Host: localhost:5000
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:54.0) Gecko/20100101 Firefox/54.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate
Referer: https://github.com/
Cookie: session=****
Connection: keep-alive
Upgrade-Insecure-Requests: 1
If-None-Match: "87f1ed4981ad4a15ae5a2fe84f250ac2"
Cache-Control: max-age=0 There's the correct and the response is HTTP/1.0 200 OK
Content-Type: text/html; charset=utf-8
Content-Length: 11944
Cache-Control: private, max-age=86400
Etag: "a13d8f68a2abc2ae1c0d7c588dd2db82"
Date: Mon, 06 Mar 2017 04:54:18 GMT
Set-Cookie: *****
Server: Werkzeug/0.10.4 Python/2.7.10 with a different etag as it should be because it's based on the HTML content and the template changed. So the caching is working properly, the login dance in between github/webcompat works for the cookie but doesn't generate the correct request with a if-none-match. Basically No conditional request is done because the resource is aged less than 1 day so it is using the cache. |
I have two ideas to fix it. Let's try. |
So basically the right thing to use here would be The way to do something which is interoperable is to do.
|
I will create a new branch with the appropriate information. |
|
This reverts commit bab6f6d.
- also fixes a comment (minor)
This is fixed. |
We need to have a better story at caching HTML resources on webcompat.com.
See #590 and comments on #608
The text was updated successfully, but these errors were encountered: