Skip to content

Conversation

@MichaIng
Copy link
Member

@MichaIng MichaIng commented Jan 26, 2024

☑️ Resolves

Nginx resets all parent add_header directives in a location block, if itself contains any add_header directive. When setting the Cache-Control header for static assets, this was originally worked around by using the expires directive instead. #8083 however added the immutable flag for assets with v= query parameter via add_header directive, and broke the parent block add_header directives for common security response headers for assets that way.

This commit fixes this by re-adding all response headers explicitly for those assets. Originally this doubled code was removed thanks to using expires, but I see no way to have both: the immutable flag as well as avoiding doubled add_header directives via expires directive.

Additionally, this commit avoids the trailing comma and space in the Cache-Control header for assets without v= query parameter, sets the wasm MIME type in a cleaner/more consistent way together with js/mjs, and rephrases the comment sentence.

🖼️ Screenshots

@MichaIng MichaIng added this to the Nextcloud 29 milestone Jan 26, 2024
@MichaIng MichaIng requested a review from icewind1991 January 26, 2024 16:04
@MichaIng MichaIng force-pushed the fix/nginx-config branch 2 times, most recently from ce213fb to f09be76 Compare January 26, 2024 16:09
Nginx resets all response headers in a location block, if it contains any "add_header" directive. When setting the "Cache-Control" header for static assets, this was originally worked around by using the "expires" directive instead. #8083 however added the "immutable" flag for assets with "v=" query parameter and broke all other response headers for assets that way.

This commit fixes this by re-adding all reponse headers explicitly for those assets. Originally those doubled code was removed thanks to using "expires", but I see no way to have both: the "immutable" flag as well as avoiding doubled headers via "expires" directive.

Additionally, this commit avoids the trailing comma and space in the Cache-Control header for assets without "v=" query parameter, and adds sets the wasm MIME type in a cleaner/more consistent way together with js/mjs, and rephrases the broken comment sentence.

Signed-off-by: MichaIng <micha@dietpi.com>
@icewind1991 icewind1991 merged commit ef0d415 into master Feb 9, 2024
@icewind1991 icewind1991 deleted the fix/nginx-config branch February 9, 2024 14:40
@MichaIng
Copy link
Member Author

MichaIng commented Feb 9, 2024

/backport to stable28

@MichaIng
Copy link
Member Author

MichaIng commented Feb 9, 2024

/backport to stable27

@MichaIng
Copy link
Member Author

MichaIng commented Feb 9, 2024

/backport to stable26

@github-actions
Copy link
Contributor

github-actions bot commented May 2, 2024

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants