[cgiapp] CGI.pm: discussing a patch to update url() behavior under mod_rewrite

[Mark Stosberg]

I've posted a patch with the complete proposed fix/docs/tests for this
issue here if you want to take a look:

https://github.com/markstos/CGI.pm/commit/0023f178943c687333285713971ea82e54b3dcc6

It may change the behavior of your CGI::App's if you come to depend on
or workaround the behavior of this old bug.

   Mark

On 05/13/2011 03:41 PM, Mark Stosberg wrote:
> 
> Some of us here have been bitten by CGI.pm's behavior under mod_rewrite,
> which is common to use with CGI::Application::Dispatch.
> 
> This test illustrates the issue:
> 
> ###
> use Test::More;
> use CGI;
> print "CGI: $CGI::VERSION\n";
> 
> %ENV =  (
>   # These two are always set
>   'SCRIPT_NAME'     => '/real/cgi-bin/dispatch.cgi',
>   'SCRIPT_FILENAME' => '/home/mark/real/path/cgi-bin/dispatch.cgi',
> 
>   # These two are added by mod_rewrite Ref:
> http://httpd.apache.org/docs/2.2/mod/mod_rewrite.html
> 
>   'SCRIPT_URL'      => '/real/path/info',
>   'SCRIPT_URI'      => 'http://example.com/real/path/info',
> 
>   'PATH_INFO'       => '/path/info',
>   'REQUEST_URI'     => '/real/path/info',
>   'HTTP_HOST'       => 'example.com'
> );
> 
> my $q = CGI->new;
> 
> is(
>     $q->url( -absolute => 1, -query => 1, -path_info => 1 ),
>     '/real/path/info',
>     '$q->url( -absolute => 1, -query => 1, -path_info => 1 ) should
> return complete path, even when mod_rewrite is detected.'
> );
> 
> done_testing();
> 
> #####
> 
> Here's the result with 3.54:
> 
> #          got: '/real'
> #     expected: '/real/path/info'
> 
> This behavior was not always the case. Older versions of CGI.pm ( I
> happened to test with 3.05 ), gave the expected result.
> 
> There's a bug report about the current behavior here:
> https://rt.cpan.org/Ticket/Display.html?id=45019
> 
> I'm evaluating accepting the proposal to address it by removing this
> line in url():
> 
>    undef $path if $rewrite_in_use && $rewrite;  # path not valid when
> rewriting active
> 
> First, I'm looking for feedback on this change. It *is* a behavior
> change, so it will likely break someone's code somewhere, but on the
> other hand, it restores a previous functionality, so it will "fix" the
> upgrade experience for those with an old enough CGI.pm vintage.
> 
> Unlike adding another option to support both behaviors, I would rather
> simply remove this code to simply things, if there's some agreement that
> this is the better behavior.
> 
> Second, I'm looking for a volunteer to beef up the test coverage of
> url() so that we have more regression tests for it. A starting point
> might be to borrow the url() tests from CGI::Simple, and convert them to
> OO style:
> 
> http://cpansearch.perl.org/src/ANDYA/CGI-Simple-1.113/t/070.standard.t
> 
> A simple "diff" is fine, but you are always welcome to fork on github
> and send a pull request:
> 
> https://github.com/markstos/CGI.pm
> 
> Thanks for your feedback on this issue.