-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Add Cache-Control header to html and api responses, add no-transform #20432
Conversation
`no-transform` allegedly disables CloudFlare auto-minify and we did not set caching headers on html or api requests, which seems good to have regardless.
This PR will close #19309 |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Golang's nil
can be used as empty slice directly, so no need to allocate memory for an empty slice when calling fn(..., []string{})
.
Although I think argument additionalDirectives ...string
could be more simple:
fn(..., time)
instead offn(..., time, nil)
fn(..., time, "no-transform")
instead offn(..., time, []string{"no-transform"})
Current approach is also fine to me.
Thanks, didn't know that. I'm still a golang noob 😉 |
I think @wxiaoguang is right and we should use the varargs notation for this so I've pushed up a change to do that. I guess one thing I'm wondering is if it's really correct that we don't want any caching of our generated html. I guess that's right but it seems bad. |
There's not much point in allowing HTML cache that I see. Content is too dynamic to allow caching. Most pages change on every page load.
|
Signed-off-by: Andrew Thornton <art27@cantab.net>
IMO one more thing that why Gitea need 'no-store' or 'no-cache': there are a lot of JS code. If the page is cached, then when users make the browser navigate back to a history page, if the page is cached, then the JS won't be executed, the UI may be stuck in a strange state. So Gitea should tell browsers to reload every page during navigation. |
We are in good company regarding anti-cache headers on HTML: gitlab: |
…o-gitea#20432) `no-transform` allegedly disables CloudFlare auto-minify and we did not set caching headers on html or api requests, which seems good to have regardless. Transformation is still allowed for asset requests. Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: Andrew Thornton <art27@cantab.net>
…20432) (#20459) `no-transform` allegedly disables CloudFlare auto-minify and we did not set caching headers on html or api requests, which seems good to have regardless. Transformation is still allowed for asset requests. Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: Andrew Thornton <art27@cantab.net>
sorry, late to the party: I checked and I think the only thing changing here (for consecutive requests) is the embedded csrf token, in fact most of the page stays the same (until something changes of course, a month from now the dates will show +1 in "X months ago") for quite some time. do we know how these changes affect deployments with Gitea behind proxies? won't adding should |
since Gitea is not a hosted service, I'd really prefer if this stayed user-tunable, e.g. in my set-up, |
Yes, it's a hint to not transform responses. Cloudfare does destructive HTML minification per default and that problem came up a few times in the past. I don't think There's no real spec for what the header allows/forbids, but I think it's mostly the destructive stuff. If you control the proxy, you can of course always do whatever you like, this header is just a hint to automated systems. |
thanks for the reply. that probably means with nginx I'll have to start clearing headers before setting them...will test, report back in case of trouble. |
* giteaofficial/main: Fix Ruby package parsing by removed unused email field (go-gitea#20470) [skip ci] Updated translations via Crowdin Add repository condition for issue count (go-gitea#20454) Prepend commit message to template content (go-gitea#20429) Improve pprof doc (go-gitea#20463) Improve code diff highlight, fix incorrect rendered diff result (go-gitea#19958) Add Cache-Control header to html and api responses, add no-transform (go-gitea#20432) [skip ci] Updated translations via Crowdin Allow non-semver packages in the Conan package registry (go-gitea#20412) Use body text color in repository files table links (go-gitea#20386) Correct code block in installation docs for Snap (go-gitea#20440) Downgrade golangci-lint to 1.47.0 (go-gitea#20445) Add eslint-plugin-sonarjs (go-gitea#20431) Fix: Actor is required to get user repositories (go-gitea#20443) Add "X-Gitea-Object-Type" header for GET `/raw/` & `/media/` API (go-gitea#20438) Simplify visibility checks (go-gitea#20406)
…o-gitea#20432) `no-transform` allegedly disables CloudFlare auto-minify and we did not set caching headers on html or api requests, which seems good to have regardless. Transformation is still allowed for asset requests. Signed-off-by: Andrew Thornton <art27@cantab.net> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: Andrew Thornton <art27@cantab.net>
The no-store cache control added in go-gitea#20432 is causing form input to be cleared unnecessarily on page reload. Instead use max-age=0,private,must-revalidate which avoid this. This was particularly a problem when typing a long comment for an issue and then for example changing the label. The page would be reload and lose the unsubmitted comment. Fixes go-gitea#22603
…2604) The `no-store` cache control added in #20432 is causing form input to be cleared unnecessarily on page reload. Instead use `max-age=0,private,must-revalidate` which avoids this. This was particularly a problem when typing a long comment for an issue and then for example changing the label. The page would be reloaded and lose the unsubmitted comment. Fixes #22603
no-transform
allegedly disables CloudFlare auto-minify and we did not set caching headers on html requests, which seems good to have regardless.Transformation is still allowed for asset requests.
Fixes: #19309