Re: [PHP-DEV] Re: [PHP-CVS] cvs: php4(PHP_4_3) /sapi/cgi cgi_main.c

[Shane Caraveo]

Zeev Suraski wrote:
> How does this patch possibly fix something in this magnitude? It
> doesn't appear to do anything crucial at all, or do these variables have
> some far-reaching indirect effect?
>
> Zeev

Well, the one patch refered to in this thread was only a minor fix over the previous patches I applied yesterday.

I did send out emails regarding and describing this problem (http://marc.theaimsgroup.com/?l=php-dev&m=103834043816376&w=2) with no response so aparently no one cares until after I fix the problem ;)

The basics in case you don't want to read my long email...

When was this broken? A very long time ago, possibly/probably the entire 4 series.

Are there bug reports? Yes, some inapropriately marked bogus.

Does the patch present potential problems? Sure, it's a significant patch that changes the way the cgi module works, but at least, minus any bugs, it works, which is much better than not having the patch at all. I have tested this on win32 and osx, as cgi and fastcgi, under apache 1.3/2.0 and IIS, which is probably the most testing CGI has gotten in a very long time.

Should it go in 4.3? I strongly think so. It would be sad and ironic to have yet another release with cgi that doesn't work at all (given that PHP is *the* Web language). I think it's worth an extra release cycle and extra QA to have this in.

Could the patch be better written? Sure, I don't claim to be a supreme C developer. There are memory allocations in there I'd like to get rid of at a later time, and I think the 'backwards broken compatibility' should be removed.

Now the long part.
So, why did this have to happen?

First, PATH_INFO was being used for PHP_SELF, which prior to the fix, apache set to the php binary. The cgi module used SCRIPT_FILENAME if DISCARD_PATH was set, and again, this would be set to the php binary by apache, so php would parse itself and not the script desired. Third, none of the CGI variables (as defined in CGI spec) were ever correct. This broke using PATH_INFO in applications (one example being the pres2 system). These variables are the primary targets of the patch since they are required for proper execution of php as CGI. However, since other variables were also wrong (SCRIPT_NAME and PATH_TRANSLATED) rather than just fixing what PHP absolutely needed to operate right, I fixed them all. Then, additionaly, using the mod_cgi cgi_script directive (using bang lines in php) was broken, thus the small patch to fopen_wrappers.c.

This stuff has been broken for a very long time, since before 4.2 if I remember right. In the last month, I've seen at least two non-php people attempt to set up php cgi and give up due to these bugs. I didn't realy clue in to how broken it was until I saw that, then tried it myself on my shiny new mac. The self-parsing of the php binary did not occure on IIS because IIS sends the script name as an argument when executing php, and that would override the environment variables, but PATH_INFO was still broken.

There are bugs in the bug database that this fixes also, one example: http://bugs.php.net/bug.php?id=19378

There are also bugs marked as bogus that probably were not, one example: http://bugs.php.net/bug.php?id=10075. (CGI and PATH_INFO aparently is not understood in this instance)

Shane



--
PHP Development Mailing List <http://www.php.net/>
To unsubscribe, visit: http://www.php.net/unsub.php