fix $_SERVER['SCRIPT_NAME'], 'PHP_SELF', and 'PATH_INFO' #2317
fix $_SERVER['SCRIPT_NAME'], 'PHP_SELF', and 'PATH_INFO' #2317
Conversation
…rkers apache passes PHP_SELF as SCRIPT_NAME + REQUEST_URL, but the docs say it's the same as SCRIPT_NAME and that's how Caddy+fpm behave too
cgi.go
Outdated
| // If a worker is already assigned explicitly, derive SCRIPT_NAME from its filename | ||
| if fc.worker != nil { | ||
| fc.scriptFilename = fc.worker.fileName | ||
| fc.scriptName = strings.TrimPrefix(fc.worker.fileName, fc.documentRoot) |
There was a problem hiding this comment.
Hmm probably would be better to leave scriptName empty if the worker is in the public path:
root /some/path
worker /other/path {
match *
}There was a problem hiding this comment.
I'm not sure about that. Shouldn't it show /other/path/index.php then?
There was a problem hiding this comment.
Not sure either TBH. In CGI spec it's the path relative to the root, so /other/path/index.php might be misleading.
There was a problem hiding this comment.
Technically that's a path relative to the root, haha.
I don't see a better solution. Emptying it out would be even more of a violation.
… would activate for the non-worker case too
cgi.go
Outdated
| // If a worker is already assigned explicitly, derive SCRIPT_NAME from its filename | ||
| if fc.worker != nil { | ||
| fc.scriptFilename = fc.worker.fileName | ||
| fc.scriptName = filepath.ToSlash(strings.TrimPrefix(fc.worker.fileName, fc.documentRoot)) |
There was a problem hiding this comment.
Can you also add a test for when the worker is outside of the public path (match)? Thinking about it again, scriptname should be empty since forwarding an absoule path when expecting a relative one is probably worse than forwarding nothing.
There was a problem hiding this comment.
Happy to add the test, but I'm not sure if I agree with passing nothing being the correct choice. We should probably agree on what the correct behaviour is first.
'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.
If anything, "This is useful for pages which need to point to themselves." would mean that we should perhaps just give the current request uri. But on the other hand, "Contains the current script's path." would mean it should be the absolute path, rather than a uri.
There was a problem hiding this comment.
It's still expected to be a relative path, so I'd rather prevent potential redirects like this (and just leave it empty):
/path-the-user-visits -> /app/vendor/some-framework/worker.php
Realistically, most worker mode implementations probably just ignore SCRIPT_NAME.
There was a problem hiding this comment.
I don't know, that doesn't feel right to me. I've changed it to what you suggested either way. Nobody should be using these server vars anyway.
fixes #2274 (comment)
apache/nginx/caddy pass PHP_SELF as SCRIPT_NAME + PATH_INFO, but our PATH_INFO wasn't working because our matcher stripped the rest of the path.
request url: localhost/index.php/en