yonjah / piwigo_privacy

A small script and nginx configuration to increase the privacy of piwigo gallery
GNU General Public License v2.0
20 stars 8 forks source link

Using search-terminating prefix matching #16

Open philtuckey opened 5 years ago

philtuckey commented 5 years ago

Hi yonjah I have a suggestion and not knowing github etiquette I hope this method is ok. The suggestion is to use search-terminating matching in piwigo-nginx-site for the /upload /galleries and /_data locations, i.e. location ^~ /upload ... The reason is that it's more robust in case inexperienced people try to "improve" your config by adding location regexp matches which may otherwise override these prefix matches. I speak from experience. Thanks very much for this plugin and server config. Best

yonjah commented 5 years ago

@philtuckey Thank you for this suggestion. I guess it makes sense. If you have the time to submit a pull request with the suggested change to the nginx config that would be great.

In general the nginx conf is just an example and it will probably be very hard to get a conf that it will work with every setup or will be robust against users who will add changes to it without fully understand what they are changing. So it might make sense to add a bit more info about this type of mistakes in the README

philtuckey commented 5 years ago

@yonjah Thanks for your reply. I've never had to learn anything about git or github, so it will take a little while before I can "submit a pull request". A couple of other comments (at least not to forget):

Best

yonjah commented 5 years ago

your nginx server config blocks (successfully!) the "Display reference file: "config_default.inc.php""

Yea I guess it does.

I never used the localFileEditor so didn't noticed it was broken. Your fix is ok and I don't think you need to worry about security issues with the fix. In essence you need to take the nginx.conf with a grain of salt, it is just there as an example and you need to consider what work or doesn't work for you.

I wonder if it is possible to define a "subdir" variable in the nginx server file which could be pre-pended to all the location paths, to make it easier for people who install piwigo in a subdirectory.

There is already a similar pull request for this issue https://github.com/yonjah/piwigo_privacy/pull/15 I have a few changes I want to make before adding this feature (especially putting it behind a feature flag and not requiring any special changes to nginx.conf (Other than pointing to the correct subdir) I don't consider it as high priority but if there is more demand for it I might try to add it sooner