Skip to content

Conversation

fliespl
Copy link
Contributor

@fliespl fliespl commented Aug 9, 2019

As described above - this location rule provides similar functionality to apache FallbackResource example.

@OskarStark OskarStark added the help wanted Issues and PRs which are looking for volunteers to complete them. label Aug 16, 2019
@OskarStark
Copy link
Contributor

Thank you for your contribution, sadly I never used this before, so we need to find someone who can verify this.

@OskarStark OskarStark added this to the 3.4 milestone Aug 16, 2019
Copy link
Contributor

@Pierstoval Pierstoval left a comment

Choose a reason for hiding this comment

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

Seems like a nice proposal, I see no "no" for it, and I'm already using it in production 👍

# which will allow nginx to return a 404 error when files are
# not found instead of passing the request to Symfony
location /bundles {
try_files $uri $uri/ =404;
Copy link
Member

Choose a reason for hiding this comment

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

Actually, if you are looking for a file, the $uri/ lookup seems useless to me.

try_files $uri /index.php$is_args$args;
}
# optionally disable falling back to php script for the asset directories
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# optionally disable falling back to php script for the asset directories
# optionally disable falling back to PHP script for the asset directories

Copy link
Contributor

Choose a reason for hiding this comment

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

why does it say "optionally" if it's enabled by default? I also don't really see the value of adding it. It only prevents use-cases like having a route starting with /bundles.

@ndench
Copy link
Contributor

ndench commented Sep 18, 2019

I don't see a downside for adding this, though you might want to add more info as to why you would want this and what are the downsides?

ie.

  • this prevents any PHP files in the /bundles directory from being executed
  • you will need to change /bundles to be whatever directory your assets are in
  • this will mean that users are presented with nginx 404 pages instead of Symfony 404 pages which you may have custom styling for

@javiereguiluz javiereguiluz changed the base branch from master to 4.3 September 23, 2019 14:26
javiereguiluz added a commit that referenced this pull request Sep 23, 2019
…ginx (fliespl)

This PR was submitted for the master branch but it was merged into the 4.3 branch instead (closes #12125).

Discussion
----------

add FallbackResource alternative for assets/bundles in nginx

As described above - this location rule provides similar functionality to apache FallbackResource example.

Commits
-------

8958f6c add FallbackResource alternative for assets/bundles in nginx
@javiereguiluz javiereguiluz merged commit 8958f6c into symfony:4.3 Sep 23, 2019
@javiereguiluz
Copy link
Member

Thank you all for the review! I've merged but made some changes to comment this by default (as @Tobion asked), remove the unneeded $uri/ lookup (as @lyrixx said) and mention that you won't see Symfony's 404 error page (as @ndench explained; thanks for listing the disadvantages ... I think that the first two were not that common to mention them).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issues and PRs which are looking for volunteers to complete them. Status: Reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants