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

Update piwigo-nginx-site #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

philtuckey
Copy link

Use search-terminating prefix location matching, i.e. "^~", for the "internal" directories, to make these blocks more robust against additional regular expression location blocks which may be added.
Add 3 work-arounds for the extra security blocks.

Use search-terminating prefix location matching, i.e. "^~", for the "internal" directories, to make these blocks more robust against additional regular expression location blocks which may be added.
Add 3 work-arounds for the extra security blocks.
Copy link
Owner

@yonjah yonjah left a comment

Choose a reason for hiding this comment

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

Hi @philtuckey
Thank you for this changes.

just a few minor changes

# Enable the letsencrypt "webroot installation" verification step
# as used for example by the openmediavault letsencrypt plugin
location ^~ /.well-known/acme-challenge {
Copy link
Owner

Choose a reason for hiding this comment

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

I think including letsencrypt block is out of scope for this file.
We want to keep it relevant to the piwigo plugin.

# Enable the "Display reference file" link of the LocalFiles Editor plugin
location = /plugins/LocalFilesEditor/show_default.php {
try_files $uri =404;
Copy link
Owner

Choose a reason for hiding this comment

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

I think this should probably come after the location = /plugins/piwigo_privacy/get.php block
Also better to give generic explanation that if you use the security block this is how you can open other php routes but leave the actual block commented out.
This is also apply for the metadata block. I don't think we need a block for each file
But just give the tools folder as another location with files that are worth considering

@Catfriend1
Copy link

What is blocking the merge/finishing here?

@yonjah
Copy link
Owner

yonjah commented Apr 1, 2021

What is blocking the merge/finishing here?

See comments above

@philtuckey
Copy link
Author

Hi. 2 years ago I figured out how to do a change request, I think because I was asked to at the time. I haven't gone any further on guthub since and have probably forgotten whatever I knew at the time. If I understand correctly I am somehow blocking a process here. Please tell me the simplest thing I can do to unblock it, maybe cancelling the request somehow? Best regards

@yonjah
Copy link
Owner

yonjah commented Apr 4, 2021

Hi. 2 years ago I figured out how to do a change request, I think because I was asked to at the time. I haven't gone any further on guthub since and have probably forgotten whatever I knew at the time. If I understand correctly I am somehow blocking a process here. Please tell me the simplest thing I can do to unblock it, maybe cancelling the request somehow? Best regards

Hi @philtuckey. It's really up to you if you want it merged or not.
You can close this issue if you don't plan to work on it any more so it'll be merged or do the changes when you can, or someone else doing them, it's all good.

Not sure why @Catfriend1 need this getting merged as this PR is mostly documentation change that doesn't really affect the plugin behaviour.

@Catfriend1 if you need this changes for any specific reason you can work on this PR so it'll be ok to merge

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.

3 participants