Skip to content

Conversation

@CarlSchwan
Copy link
Member

@CarlSchwan CarlSchwan commented Feb 14, 2022

See nextcloud/server#31141

I didn't test it, can an nginx expert try to see if the syntax is correct?

Signed-off-by: Carl Schwan carl@carlschwan.eu

@CarlSchwan CarlSchwan force-pushed the nginx-immuatable-cache branch from 318adbd to b30299c Compare February 14, 2022 15:02
Copy link
Member

@icewind1991 icewind1991 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look good, doesn't break

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@CarlSchwan CarlSchwan merged commit 7873bd1 into master Feb 15, 2022
@CarlSchwan CarlSchwan deleted the nginx-immuatable-cache branch February 15, 2022 08:58
@te-online
Copy link

This didn't work for me. Adding the directive resulted in 404s for urls like https://example.org/css/core/88a4-d75f-server.css?v=36d57b3bd263334af5e8ac7e29611776-a53c3031-7 and thus no CSS being loaded.

This is a pretty vanilla setup, using docker and nginxproxy/nginx-proxy in front of the app and I'm basically using the documentation's nginx-config 1:1. Highly possible that I'm doing something wrong here, but I still wanted to let you know.

@CarlSchwan
Copy link
Member Author

This didn't work for me. Adding the directive resulted in 404s for urls like https://example.org/css/core/88a4-d75f-server.css?v=36d57b3bd263334af5e8ac7e29611776-a53c3031-7 and thus no CSS being loaded.

This is a pretty vanilla setup, using docker and nginxproxy/nginx-proxy in front of the app and I'm basically using the documentation's nginx-config 1:1. Highly possible that I'm doing something wrong here, but I still wanted to let you know.

Thanks for the feedback, I will look into ut tomorrow

@icewind1991
Copy link
Member

Did some more testing and I can reproduce, I think I placed the block in a different place when testing initially 🙈

Adding the try_files $uri /index.php$request_uri; line to the new location block fixes things for me

@MichaIng
Copy link
Member

MichaIng commented Feb 24, 2022

This is a general "issue" with Nginx, not all directives present in the parent location block are inherited. AFAIK this affects location ~ \.wasm$ { as well regarding e.g. the cache control (expires) header which should currently not apply to these.

Are there actually assets without versioned query string? Otherwise we could just merge it by appending (\?v=.*) to the parent. Directly defining the Cache-Control header seems more reasonable anyway compared to using expires 6M; as another layer of abstraction which sets/implies a few other things we may not want, depending on Nginx version as well.

Also, what is actually the reason for using just 7 days caching for fonts? Couldn't they be merged with the other assets?

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.

6 participants