fix regular expressions in nginx config

[jknockaert]

A number of fixes to regular expressions in the nginx config example:

• escaping forward slash by prefixing a backslash (not essential, but seems good practise anyway)
• fix the second expression in fastcgi_split_path_info to allow for uris ending in .php (which do satisfy the location definition but would result in a 404 with the current specification of fastcgi_split_path_info
• adding /nextcloud to all regex location definitions in the subdirectory example

[skjnldsv]

Yes!

[nickvergessen]

🙈

[MorrisJobke]

/backport to stable15

[MorrisJobke]

/backport to stable14

[MorrisJobke]

/backport to stable13

[backportbot-nextcloud]

backport to stable15 in #1158

[backportbot-nextcloud]

The backport to stable13 failed. Please do this backport manually.

[backportbot-nextcloud]

backport to stable14 in #1159

[josh4trunks]

@jknockaert I was not notified about this PR and had I had some questions/comments on the changes.

1. escaping forward slash by prefixing a backslash (not essential, but seems good practise anyway)

Why is your change good practice? Forward slash "/" is not a special character in Nginx regex. In my opinion this change just makes the config harder to read.

2. fix the second expression in fastcgi_split_path_info to allow for uris ending in .php (which do satisfy the location definition but would result in a 404 with the current specification of fastcgi_split_path_info

How could a 404 can ever be encountered with the original config? If a client requests for example index.php, the original configuration of fastcgi_split_path_info ^(.+?\.php)(\/.*)$ will just not match and thus not unnecessarily alter fastcgi_script_name or set fastcgi_path_info

3. adding /nextcloud to all regex location definitions in the subdirectory example

Why is this necessary? The two locations you added this to are already nested under location ^~ /nextcloud so ^/nextcloud/.* is implied before any regex location.

[jknockaert]

@josh4trunks Sorry for not notifiying you.
As for the the forward slash escaping: it may or may not make it more readable. When using a regex tester like regex101.com it will give you an error when you don't escape. So sticking to the spec may be preferable here? If I remember well these regex are specified to be PCRE.
As for the fastcgi_split_path_info I don't exactly remember the specific scenario I had in mind that would result in a 404. But I would think that matching the regex for fastcgi_split_path_info to that of the location would be preferable in any case.
Adding /nextcloud to the two regex location prevents a mismatch to hidden files in the nextcloud root (e.g. .html).

[josh4trunks]

@jknockaert Not your fault I wasn't notified =]. I usually am mentioned for all NGINX changes but this one must have slipped by.

The only reason regex101.com wants / to be commented is because that particular website uses / as a delimiter by default (which is configurable). This is all irrelevant to NGINX which never uses delimiters. What "spec" you are referring to?

Why is it preferable to match fastcgi_split_path_info if a path (like index.php) doesn't need to be split? Why add complication to the regex just to have NGINX define fastcgi_path_info as ""?

Hidden files (like ".html") would never be processed by the two regex locations being discussed. They would have already matched the below location since it resides earlier in the config

location ~ ^/nextcloud/(?:\.|autotest|occ|issue|indie|db_|console) {
    deny all;
}

[jknockaert]

If I remember well, for the fastcgi_split_path_info I was thinking along the lines of where a php file was stored in a folder with a name like folder.php. An unlikely scenario, but eventually bound to happen in an environment with third party plugins and user interaction. And by defining the regexes consistently any exotic case should be accounted for.
As for the escapes and the hidden files, you are right that there may be no issue in the current setting. But when someone decides to change the other location definition for whatever reason (or even just the relative order of the locations), things may go wrong. That can be avoided by the modification I made.
So this PR was to mostly an attempt to make the nginx code more robust.

[josh4trunks]

I am confused by your folder .php scenario? Can you give me an example that I can try to reproduce where the simpler logic of fastcgi_split_path_info ^(.+?\.php)(/.*)$; does not work?

Changing or reordering regex locations does break things. This has always been the case, and I do not see how adding ^\/nextcloud\/.+[^\/] really helps in that case. As I said earlier, ^/nextcloud/.* is already implied.

[josh4trunks]

@jknockaert I never received a response to my previous question.

By my perspective (see #1152 (comment)) the 3 changes here make no difference in any of NGINX's interpretation of the example configurations, and only add complexity for admins reading them. I am open to further discussion, but otherwise plan on submitting a PR reverting these changes.