Skip to content

Apache fcgi sethandler fixes #765

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

Merged
merged 3 commits into from
Aug 12, 2014

Conversation

dzuelke
Copy link
Contributor

@dzuelke dzuelke commented Aug 9, 2014

The fix for #694 / http://bugs.php.net/67541 broke Apache with mod_fastcgi, this restores compatibility.

This change puts the old PATH_TRANSLATED workarounds back in place so everything works like before, and adds a check for the new mod_proxy_fastcgi stuff (which breaks in certain cases with these workarounds) to skip the workarounds (by checking apache_was_here, which is set in case it's mod_proxy_fcgi).

Fixes bug http://bugs.php.net/67606.

@Tyrael
Copy link
Contributor

Tyrael commented Aug 12, 2014

I will merge this before tagging the RC4 later today if nobody objects. /cc @jpauli

@jpauli
Copy link
Member

jpauli commented Aug 12, 2014

If this fixes something, I have no objection :-)

@php-pulls php-pulls merged commit 37c08f4 into php:PHP-5.6 Aug 12, 2014
@Tyrael
Copy link
Contributor

Tyrael commented Aug 12, 2014

I've merged it.

@dzuelke
Copy link
Contributor Author

dzuelke commented Aug 12, 2014

Whoop 👍

Now we should get it backported to 5.5 and 5.4 soon, eh, @jpauli ? :)

@jpauli
Copy link
Member

jpauli commented Aug 12, 2014

Was the original patch (introducing the bug) merged against 5.5 ? I'm not sure about this.

@Tyrael
Copy link
Contributor

Tyrael commented Aug 13, 2014

@jpauli nope, the decision was to only merge it to 5.6 and backport it later. and @dzuelke seems to think that the revisited patch(eg. this pull request) is ready to be backported.
personally I would wait for the patch to be first shipped with 5.6.0 stable and see the feedback before backporting it.

@jpauli
Copy link
Member

jpauli commented Aug 13, 2014

Ok. But isn't the current discussion about a fix for a commit that has not been applied to 5.5 ?
So we won't need to merge this against 5.5 right ?

@Tyrael
Copy link
Contributor

Tyrael commented Aug 13, 2014

@jpauli
the original fix (#694) was only merged to 5.6(we decided that together, including you) so that it can be tested a bit more before backporting.
an issue was reported with the fix after it was merged to 5.6(http://bugs.php.net/67606), this PR fixes that bug via reverting the original fix and introducing a less intrusive fix not affecting anybody else (hopefully).
@dzuelke is asking if we are ready to backport the improved fix, and as I mentioned before I would prefer backporting it after the 5.6.0 stable release, but it is your call for 5.5 ofc.

@jpauli
Copy link
Member

jpauli commented Aug 13, 2014

Yep OK I see. Thx for refreshing my mind which often needs refreshing.
So, sure : let's wait to see how this works into 5.6 codebase before backporting to lower branches, which are in stable state.

@dzuelke
Copy link
Contributor Author

dzuelke commented Aug 13, 2014

Yup, exactly; the new fix (this whole thing addresses mod_proxy_fcgi issues) is a lot less intrusive as it keeps the stuff I originally removed in place, but skips them if mod_proxy_fcgi is detected (and for that it relies on routines that were already there for mod_proxy_fcgi handling).

This patch is much safer, but I agree it's still a good idea to have it in 5.6.0RC4 before thinking about merges to 5.5/5.4 for final validation it fixes the breakage mod_fastcgi saw since RC2 (I know it does, I tested that part this time, but better safe than sorry :))

@dzuelke
Copy link
Contributor Author

dzuelke commented Aug 14, 2014

@Tyrael or @jpauli, can you close https://bugs.php.net/bug.php?id=67606 please?

@jpauli
Copy link
Member

jpauli commented Aug 14, 2014

I closed it

@Freeaqingme
Copy link

@dzuelke @jpauli Are you guys aware of a workaround for this issue in 5.4.30?

@dzuelke
Copy link
Contributor Author

dzuelke commented Aug 15, 2014

No, @Freeaqingme, but I think the change will/should get merged to 5.5 and 5.4 soon; Apache with mod_proxy_fcgi is broken otherwise and this new change is a lot less intrusive and risky.

/cc @jpauli @Tyrael @smalyshev

@Freeaqingme
Copy link

For what it's worth, applying the patch didn't yield any changes for me. Instead I've now used a PHP script that's included via the auto_prepend_file directive with the following contents, which seems to work:
$_SERVER['PHP_SELF'] = $_SERVER['SCRIPT_URL'];
$_SERVER['SCRIPT_NAME'] = substr($_SERVER['SCRIPT_FILENAME'], strlen($_SERVER['DOCUMENT_ROOT']));

@dzuelke
Copy link
Contributor Author

dzuelke commented Aug 29, 2014

Ported to 5.4 in ee275e3 / b206b0e and merged to 5.5 in e55c641

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

Successfully merging this pull request may close these issues.

5 participants