Add condition for pdf file type in safari issue #20455
Conversation
routers/common/repo.go
Outdated
| ctx.Resp.Header().Set("Content-Security-Policy", "default-src 'none'; style-src 'unsafe-inline'; sandbox") | ||
| if !st.IsPDF() { | ||
| ctx.Resp.Header().Set("Content-Security-Policy", "default-src 'none'; style-src 'unsafe-inline'; sandbox") | ||
| } |
There was a problem hiding this comment.
You may as well remove the || st.IsPDF() above for the same effect.
There was a problem hiding this comment.
I think this was added here for security reasons otherwise JavaScript in pdf would be executed
There was a problem hiding this comment.
You may as well remove the
|| st.IsPDF()above for the same effect.
yes, modified
There was a problem hiding this comment.
I think this was added here for security reasons otherwise JavaScript in pdf would be executed
With or without policy javascript will be executed
GiteaBot
added
the
lgtm/need 2
|
What is the exact problem? Assuming it works in other browsers, what does Safari not like? |
|
I'm rather hesitant about this PR, as it removes some protections that were put in place to restrict XSS from PDFs |
There was a problem hiding this comment.
@techknowlogick agreed and to that end I'm going to mark this PR as request changes until these concerns about security are addressed.
With policy header only safari browser can't display pdf file in browser, pdf only direct to download |
|
test GitHub pdf attachment |
|
The root problem here is the I'm not sure what the worst-case scenario of a pdf attack will be, but if we can come up with a best-effort CSP for PDFs that does not rely on the Note that the issue is generally only relevant when such user content is served on the same origin, which I think is true for most gitea instances. |
|
#20460 will fix the |
|
#20464 fixes the |
|
Even with Content Security Policy javascript will still be executed in chrome browser |
|
I've added the |
- Always respect the user's configured mime type map - Allow more types like image/pdf/video/audio to serve with correct content-type - Shorten cache duration of raw files to 5 minutes, matching GitHub - Don't set `content-disposition: attachment`, let the browser decide whether it wants to download or display a file directly - Implement rfc5987 for filenames, remove previous hack. Confirmed it working in Safari. - Make PDF attachment work in Safari by removing `sandbox` attribute. This change will make a lot more file types open directly in browser now. Logic should generally be more readable than before with less `if` nesting and such. Replaces: #20460 Replaces: #20455 Fixes: #20404
- Always respect the user's configured mime type map - Allow more types like image/pdf/video/audio to serve with correct content-type - Shorten cache duration of raw files to 5 minutes, matching GitHub - Don't set `content-disposition: attachment`, let the browser decide whether it wants to download or display a file directly - Implement rfc5987 for filenames, remove previous hack. Confirmed it working in Safari. - Make PDF attachment work in Safari by removing `sandbox` attribute. This change will make a lot more file types open directly in browser now. Logic should generally be more readable than before with less `if` nesting and such. Replaces: go-gitea#20460 Replaces: go-gitea#20455 Fixes: go-gitea#20404
- Always respect the user's configured mime type map - Allow more types like image/pdf/video/audio to serve with correct content-type - Shorten cache duration of raw files to 5 minutes, matching GitHub - Don't set `content-disposition: attachment`, let the browser decide whether it wants to download or display a file directly - Implement rfc5987 for filenames, remove previous hack. Confirmed it working in Safari. - Make PDF attachment work in Safari by removing `sandbox` attribute. This change will make a lot more file types open directly in browser now. Logic should generally be more readable than before with less `if` nesting and such. Replaces: #20460 Replaces: #20455 Fixes: #20404
- Always respect the user's configured mime type map - Allow more types like image/pdf/video/audio to serve with correct content-type - Shorten cache duration of raw files to 5 minutes, matching GitHub - Don't set `content-disposition: attachment`, let the browser decide whether it wants to download or display a file directly - Implement rfc5987 for filenames, remove previous hack. Confirmed it working in Safari. - Make PDF attachment work in Safari by removing `sandbox` attribute. This change will make a lot more file types open directly in browser now. Logic should generally be more readable than before with less `if` nesting and such. Replaces: go-gitea#20460 Replaces: go-gitea#20455 Fixes: go-gitea#20404
Closes #20404