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

Ensure some $_SERVER variables have the right values #1133

Open
withinboredom opened this issue Nov 3, 2024 · 4 comments · May be fixed by #1136
Open

Ensure some $_SERVER variables have the right values #1133

withinboredom opened this issue Nov 3, 2024 · 4 comments · May be fixed by #1136
Assignees
Labels
enhancement New feature or request

Comments

@withinboredom
Copy link
Collaborator

withinboredom commented Nov 3, 2024

Describe you feature request

The values are somewhat documented here: https://www.php.net/manual/en/reserved.variables.server.php

Specifically, this issue is in regards to:

REQUEST_URI:
The URI which was given in order to access this page; for instance, '/index.html'.

SCRIPT_NAME:
Contains the current script's path. This is useful for pages which need to point to themselves. The FILE constant contains the full path and filename of the current (i.e. included) file.

PHP_SELF:
The filename of the currently executing script, relative to the document root. For instance, $_SERVER['PHP_SELF'] in a script at the address http://example.com/foo/bar.php would be /foo/bar.php. The FILE constant contains the full path and filename of the current (i.e. included) file. If PHP is running as a command-line processor this variable contains the script name.

Which suggests a 'literal', non-processed URI for REQUEST_URI. Digging into the comments on this page, I was able to get a little more information from people studying these behaviors of these variables across time/SAPIs (I love comments on these pages).

REQUEST_URI:

  • should include query parameters
  • before any rewriting (e.g., raw URL after the domain, as the user typed it)
  • includes leading slash

SCRIPT_NAME:

  • filename of the entry point script (e.g., index.php; should be constructable by removing DOCUMENT_ROOT from PHP_SELF)
  • excludes PATH_INFO
  • excludes query parameters

PHP_SELF:

  • URL after all rewrites and processing
  • includes PATH_INFO
  • excludes query parameters

I also noted that we may not have the correct values for PATH_INFO:

PATH_INFO:
Contains any client-provided pathname information trailing the actual script filename but preceding the query string, if available. For instance, if the current script was accessed via the URI http://www.example.com/php/path_info.php/some/stuff?foo=bar, then $_SERVER['PATH_INFO'] would contain /some/stuff.

This should be the values after the php script: /index.php/some/path should be /some/path, different from REQUEST_URI.

Apache and Nginx

For Apache and nginx, I looked into how they handle encoded urls (urls that have %2f instead of /):

Apache has AllowEncodedSlashes to process these urls. It looks like the typical usecase is something like: /example/http:%2F%2Fwww.someurl.com/ for proxies and potentially retrieving screenshots. Apache considers these invalid urls unless this setting is set to On or NoDecode (and processes them or doesn't, depending on the value of this setting).

For nginx, it depends on how you formulate the variable in your configuration. If you use $uri, then it will pass on the unescaped version, but $request_uri will be the original escaped version. Looking at the default fastcgi parameters that came with my nginx installation (ubuntu 24.04), it sets REQUEST_URI to $request_uri.

In both SAPIs, this behavior is configurable for handling url-decoding before passing to PHP.

@withinboredom withinboredom added the enhancement New feature or request label Nov 3, 2024
@withinboredom withinboredom self-assigned this Nov 3, 2024
@AlliBalliBaba
Copy link
Collaborator

Interesting, I imagine correctly handling the encoded string is important for a reverse proxy since it potentially allows bypassing certain paths if the origin server does decode them.
I think we would also need to be careful about how Caddy interprets these paths.
E.g if I have something like this in the Caddyfile:

handle /prefix/* {
    ...
}

And I get a request like this:
/prefix%2fscript.php

Caddy would invoke the handle directive.

@withinboredom
Copy link
Collaborator Author

Yep, it was #1130 that led me to this, specifically in regards to %2f. I was looking at my WordPress logs before doing the switch to FrankenPHP and saw a number of those types of requests, so I got curious what the behavior is supposed to be.

Caddy would invoke the handle directive.

I believe this is reasonably correct. There are some other fun ones:

https://withinboredom.info/2024/08/12///////////////////////////////////optimizing-cgo-handles/

is a valid URI that Apache and Nginx normalize.

@AlliBalliBaba
Copy link
Collaborator

Yeah you are right, /path/////test.php will execute the test.php script on nginx.

And as you mentioned everything but REQUEST_URI is normalized:

[SCRIPT_NAME] => /path/test.php
[SCRIPT_FILENAME] => /var/www/public/path/test.php
[PHP_SELF] => /path/test.php
[DOCUMENT_URI] => /path/test.php
[REQUEST_URI] => /path/////test.php

Probably makes sense to stay consistent with nginx/apache 👍 . I think this is niche enough to not break any application depending on the current logic.

@withinboredom withinboredom linked a pull request Nov 4, 2024 that will close this issue
@ARehmanMahi
Copy link

Thank you @AlliBalliBaba for the dd fix
Thank you @withinboredom for these new issues you keep posting and PR.

I was just going through some of the tickets by both of you. Love the commitment and collaboration both of you doing together.

Reading your replies, it felt like I'm watching 2 friends discussing their next journey. Loved it.
Thank to both of you, as well as dunglas for all the awesome stuff he does.

I've not even started using this but it looks like what the php world should have moved to way sooner.
Cheers.

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

Successfully merging a pull request may close this issue.

3 participants