Skip to content
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

Block direct access to ESDs #8

Merged
merged 1 commit into from
Dec 18, 2014

Conversation

samuelvogel
Copy link
Contributor

The PHP setting is the default anyways, so restricting direct access should not be a problem.

@bcremer
Copy link
Owner

bcremer commented Dec 16, 2014

Looks fine to me, will check this later today.

# * 'PHP' (slow)
# * 'X-Accel' (optimized)
# Also see http://wiki.shopware.com/ESD_detail_1116.html#Ab_Shopware_4.2.2
location ^~ /files/esds/ {
Copy link
Owner

Choose a reason for hiding this comment

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

I would opt for using the default location for esd files:
location ^~ /files/552211cce724117c3178e3d22bec532ec/ {

This may not be the best choice shopware made for the default esd location but it's the default now. And this config should work out of the box for most users so setting the path to the default seems like a sane choice.

Maybe we should switch the default esd location and deprecate/remove the link download entirely but that's another issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah ok, just found that. This snippet is from a config of a project which we started with Showare 4.1.3 where the default was sill /files/esds/ (and was never changed, since we blocked access from the beginning).

So yes, it should be merged with the new default path!

Copy link
Owner

Choose a reason for hiding this comment

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

May you please update the PR accordingly so I can merge it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@bcremer
Copy link
Owner

bcremer commented Dec 18, 2014

Thanks @samuelvogel 👍

bcremer added a commit that referenced this pull request Dec 18, 2014
@bcremer bcremer merged commit ec66772 into bcremer:master Dec 18, 2014
@bcremer
Copy link
Owner

bcremer commented Dec 18, 2014

After merging I was thinking about creating a variable for the esd key to get rid of the hardcoded value in global/shopware.con.

# sites-available/example.com.conf
set $shopware_esd_key '552211cce724117c3178e3d22bec532ec';`
# global/shopware.con
location ^~ /files/$shopware_esd_key/ {
    internal;
}

Unfortunately that seems not possible with plain Nginx config.

Edit:
Created an issue for that: #9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants