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.
| } | ||
| rewriteHandler := rewrite.Rewrite{ | ||
| URI: "{http.matchers.file.relative}", | ||
| URI: "{http.matchers.file.relative}{http.matchers.file.remainder}", |
There was a problem hiding this comment.
Hmm without this change the path for PHP_SELF will not be forwarded? Not sure what other consequences changing the matcher here might have.
There was a problem hiding this comment.
yeah it'll be missing like PATH_INFO.
There was a problem hiding this comment.
TBH PHP_SELF and PATH_INFO have always confused me, we probably need some tests verifying that their behaviors conform to CGI spec exactly. Modern frameworks mostly handle routing themselves, but more traditional setups might rely on these very specific routing rules to not be vulnerable to some kind of bypassing.
There was a problem hiding this comment.
Time to put my AI credits to good use.
… would activate for the non-worker case too
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