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

Fix GH-17645: FPM with httpd ProxyPass does not decode script path #17896

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

Conversation

bukka
Copy link
Member

@bukka bukka commented Feb 22, 2025

This always decodes SCRIPT_FILENAME when Apache ProxyPass or ProxyPassMatch is used. It also introduces a new INI option fastcgi.script_path_encoded that allows using previous behavior of not decoding as there is a chance that some users could use encoded file paths in FS.

@bukka bukka linked an issue Feb 22, 2025 that may be closed by this pull request
@bukka bukka force-pushed the fpm_gh17645_apache_pp_sft_decoding branch from 7dc17df to f990379 Compare March 2, 2025 08:31
bukka added a commit to bukka/php-src that referenced this pull request Mar 2, 2025
This changes make FPM always decode SCRIPT_FILENAME when Apache
ProxyPass or ProxyPassMatch is used. It also introduces a new INI
option fastcgi.script_path_encoded that allows using the previous
behavior of not decoding the path. The INI is introduced because
there is a chance that some users could use encoded file paths in
their file system as a workaround for the previous behavior.

Close phpGH-17896
Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

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

Still LGTM

@nielsdos
Copy link
Member

nielsdos commented Mar 2, 2025

I do see on ARM64 macOS this fails: sapi/fpm/tests/log-suppress-output-request-body.phpt
Might not be a flaky one but could be real

This changes make FPM always decode SCRIPT_FILENAME when Apache
ProxyPass or ProxyPassMatch is used. It also introduces a new INI
option fastcgi.script_path_encoded that allows using the previous
behavior of not decoding the path. The INI is introduced because
there is a chance that some users could use encoded file paths in
their file system as a workaround for the previous behavior.

Close phpGH-17896
@bukka bukka force-pushed the fpm_gh17645_apache_pp_sft_decoding branch from f990379 to 8fae6fc Compare March 15, 2025 16:11
@bukka
Copy link
Member Author

bukka commented Mar 15, 2025

Hmm very strange that it fails. I just tested on my mac arm64 and can't recreate it.

It shows:

========DIFF========
001+ ERROR: The NOTICE does not match expected message
002+ 
003+ LOGS:
004+ --------------------------------------------------------------------
005+ [02-Mar-2025 08:44:17] NOTICE: fpm is running, pid 96302
006+ [02-Mar-2025 08:44:17] NOTICE: ready to handle connections
007+ [02-Mar-2025 08:44:17] NOTICE: Terminating ...
008+ --------------------------------------------------------------------
009+ 
010+ Most likely invalid match for entry:
011+ - PATTERN: /^(?:\[\d\d-\w\w\w-\d{4} \d\d:\d\d:\d\d(?:\.\d+)?\] )?NOTICE: (?:pid \d+, (?:\w+|\(null\))\(\), line \d+: )?(exiting, bye-bye!)$/
012+ - LINE: 
013+ - EXPECTED: exiting, bye-bye!

Which looks more like missing fetch for last log output so it didn't match last line. Can't really think how it could be related to this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FPM with httpd ProxyPass does not decode script path
2 participants