Skip Menu |
 

Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the CGI CPAN distribution.

Report information
The Basics
Id: 45019
Status: resolved
Priority: 0/
Queue: CGI

People
Owner: MARKSTOS [...] cpan.org
Requestors: MARKSTOS [...] cpan.org
sec [...] 42.org
Cc: GTERMARS [...] cpan.org
AdminCc:

Bug Information
Severity: Normal
Broken in: (no value)
Fixed in: (no value)



Subject: url() rewrite/path_info handling not compatible with CGI::Application::Dispatch rewriting
Download (untitled) / with headers
text/plain 2.1k
The current logic for url() forces the path_info to be undefined if rewriting is in use. However, there is somewhat common use-case where combining rewriting and the path_info is desired, and this is currently not possible with URL. The rewriting recipe is simple conceptually: If a file or directory physically exists, serve it directly. If the path does not physically exist, give the path to a dispatcher as path_info, which will take it from there. This is how Drupal implements their "Clean URIs" solution, and CGI::Application::Dispatch uses the same recipe. Here's how it looks in mod_rewrite: # If an actual file or directory is requested, serve directly RewriteCond %{REQUEST_FILENAME} !-f RewriteCond %{REQUEST_FILENAME} !-d # Otherwise, pass everything through to the dispatcher RewriteRule ^(.*)$ /cgi-bin/dispatch.cgi/$1 [L,QSA] Below is one proposed fix. It says to only clear out the path_info if you specifically asked for it to be thrown away ( path_info => 0 ). This allows "rewrite" to be used with or without path_info(). Although it seems technically correct, it's not backwards compatible, since the 3.12 release around 2005. The situation is complicated by self_url() always "passes path_info => 1" To stay backwards compatible, we have to keep the notion that rewrite=>1 always implies throwing out path_info. I think a compatible solution would have to involve adding yet-another flag, like "keep_rewrite_path_info"... or something. Mark --- old-alphasite/perllib/CGI.pm 2009-04-13 21:29:16.000000000 -0400 +++ new-alphasite/perllib/CGI.pm 2009-04-13 21:29:16.000000000 -0400 @@ -2717,7 +2717,11 @@ my $query_str = $self->query_string; my $rewrite_in_use = $request_uri && $request_uri !~ /^\Q$script_name/; - undef $path if $rewrite_in_use && $rewrite; # path not valid when rewriting active + + # path not valid when rewriting active, unless you specifically ask for it. + if ($rewrite_in_use && $rewrite && not $path_info) { + undef $path; + } my $uri = $rewrite && $request_uri ? $request_uri : $script_name; $uri =~ s/\?.*$//s; # remove query string
RT-Send-CC: lincoln.stein [...] gmail.com
Download (untitled) / with headers
text/plain 2.3k
Lincoln, Yanick, Could either of you comment on this proposed behavior change? It just came up again as a bug on the CGI::Application mailing list. See the latest context below: ### From: Graham TerMarsch <cgi-application@howlingfrog.com> On June 16, 2010, n1zero@aol.com wrote: Show quoted text
> I am using CA::Dispatch & CAP::Authentication in pretty much their > out-the-box state. I have found that I get this to work only if I hack
a Show quoted text
> line in CAP::Authentication, but I'd like to know why things don't
work Show quoted text
> without the hack and what I should do to avoid it. > > For apache I have > DocumentRoot /srv/trial/www/ > PerlOptions +Parent > PerlSwitches -I/srv/trial/mod > PerlSwitches -I/srv/trial/lib > <Location /trial> > SetHandler perl-script > PerlHandler CGI::Application::Dispatch > </Location> > so that the user hits /trial/mod3/page2 and this is dispatched to
Mod3.pm Show quoted text
> in runmode page2. (This all works perfectly.) > > In Mod3.pm I have > __PACKAGE__->authen->protected_runmodes('page2'); > so that upon the above hit, the user is given the built-in login form. > (This all works perfectly.) > > But this is where it goes wrong. When the user hits <Sign In> the
post Show quoted text
> goes to /trial, which doesn't exist (in this example app). Looking at
the Show quoted text
> page of the form reveals it has action="/trial". That comes from > CAP::Authentication, "my $action = $query->url( -absolute => 1 );",
which Show quoted text
> gives in my case '/trial'. Now, it turns out $destination is set > correctly, so if I follow this with "$action = $destination" then it
all Show quoted text
> appears to work perfectly. I'm guessing this is a case of user error,
but Show quoted text
> I can't see what I've done wrong to make $query->url give a > less-than-useful value.
Hmmm... this sounds an awful lot like: https://rt.cpan.org/Ticket/Display.html?id=45019 where the "path_info" isn't properly set up if you're using URL rewriting via mod_rewrite, and as a result generated links thereafter aren't generated the way that you think they should be. Although you don't say you're using mod_rewrite in your config, this sounds a lot like this problem (which I've experienced ever since CGI.pm changed its behaviour w.r.t. this). Which reminds me... markstos... did you ever get any further discussion on this (and is it something we can ever hope to see fixed in CGI.pm)?
Subject: Re: [rt.cpan.org #45019] Needs Discussion: PATCH: url() rewrite/path_info handling not compatible with CGI::Application::Dispatch rewriting
Date: Thu, 17 Jun 2010 12:31:01 -0400
To: bug-CGI.pm [...] rt.cpan.org
From: Lincoln Stein <lincoln.stein [...] gmail.com>
Download (untitled) / with headers
text/plain 3.2k
Some versions of Apache have bugs in how they set up the PATH_INFO and REQUEST_URL environment variables in the context of mod_rewrite. This has created the need for considerable workaround code in CGI.pm. The workarounds catch most, but not all cases. I would be very happy to pull out all this code and replace it with the simple code and simply tell people to file the bug reports against Apache. Lincoln On Thu, Jun 17, 2010 at 10:15 AM, MARKSTOS via RT <bug-CGI.pm@rt.cpan.org>wrote: Show quoted text
> Queue: CGI.pm > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=45019 > > > Lincoln, Yanick, > > Could either of you comment on this proposed behavior change? > > It just came up again as a bug on the CGI::Application mailing list. See > the latest context below: > > ### > > > From: Graham TerMarsch <cgi-application@howlingfrog.com> > > On June 16, 2010, n1zero@aol.com wrote:
> > I am using CA::Dispatch & CAP::Authentication in pretty much their > > out-the-box state. I have found that I get this to work only if I hack
> a
> > line in CAP::Authentication, but I'd like to know why things don't
> work
> > without the hack and what I should do to avoid it. > > > > For apache I have > > DocumentRoot /srv/trial/www/ > > PerlOptions +Parent > > PerlSwitches -I/srv/trial/mod > > PerlSwitches -I/srv/trial/lib > > <Location /trial> > > SetHandler perl-script > > PerlHandler CGI::Application::Dispatch > > </Location> > > so that the user hits /trial/mod3/page2 and this is dispatched to
> Mod3.pm
> > in runmode page2. (This all works perfectly.) > > > > In Mod3.pm I have > > __PACKAGE__->authen->protected_runmodes('page2'); > > so that upon the above hit, the user is given the built-in login form. > > (This all works perfectly.) > > > > But this is where it goes wrong. When the user hits <Sign In> the
> post
> > goes to /trial, which doesn't exist (in this example app). Looking at
> the
> > page of the form reveals it has action="/trial". That comes from > > CAP::Authentication, "my $action = $query->url( -absolute => 1 );",
> which
> > gives in my case '/trial'. Now, it turns out $destination is set > > correctly, so if I follow this with "$action = $destination" then it
> all
> > appears to work perfectly. I'm guessing this is a case of user error,
> but
> > I can't see what I've done wrong to make $query->url give a > > less-than-useful value.
> > Hmmm... this sounds an awful lot like: > > https://rt.cpan.org/Ticket/Display.html?id=45019 > > where the "path_info" isn't properly set up if you're using URL > rewriting via > mod_rewrite, and as a result generated links thereafter aren't generated > the > way that you think they should be. > > Although you don't say you're using mod_rewrite in your config, this > sounds a > lot like this problem (which I've experienced ever since CGI.pm changed > its > behaviour w.r.t. this). > > Which reminds me... markstos... did you ever get any further discussion > on > this (and is it something we can ever hope to see fixed in CGI.pm)? > >
-- Lincoln D. Stein Director, Informatics and Biocomputing Platform Ontario Institute for Cancer Research 101 College St., Suite 800 Toronto, ON, Canada M5G0A3 416 673-8514 Assistant: Renata Musa <Renata.Musa@oicr.on.ca>
Subject: CGI.pm url() function bug
Date: Mon, 28 Jun 2010 12:30:19 +0200
To: bug-CGI.pm [...] rt.cpan.org
From: Stefan `Sec` Zehl <sec [...] 42.org>
Download (untitled) / with headers
text/plain 1005b
Hi, I have encountered a problem with CGI.pm's url() function. When mod_rewrite is active. In these cases it then unconditionally clears "path_info" so that url() and url(-path => 1) always return the same info. Removing line 2720: Show quoted text
> undef $path if $rewrite_in_use && $rewrite; # path not valid when rewriting active
restores this functionality and makes it work like it should. setting "-rewrite => 0" unfortunately isn't an option because it then uses "script_name" instead of "request_uri" to construct the URL which is not what I want. I can see no harm in removing that line, as $path is only used if manually requested with url(-path =>1) - and then it should just do what I requested, not try to second-guess. I encountered that bug with CGI.pm 3.43 , but a quick check via CPAN shows that the offending code is still there in 3.49 CU, Sec -- Good spelling, punctuation, and formatting are essentially the on-line equivalent of bathing. -- Elf Sternberg http://4c2877f5.w.b.cx/stamp
Download (untitled) / with headers
text/plain 193b
This bug report is the same as RT#45019: https://rt.cpan.org/Public/Bug/Display.html?id=45019 Please comment on fix proposed there. I will merge this ticket into that one shortly. Mark
Download (untitled) / with headers
text/plain 1.2k
On Mon Jun 28 06:30:31 2010, sec@42.org wrote: Show quoted text
> Hi, > > I have encountered a problem with CGI.pm's url() function. When > mod_rewrite is active. > > In these cases it then unconditionally clears "path_info" so that > url() and url(-path => 1) always return the same info. > > Removing line 2720:
> > undef $path if $rewrite_in_use && $rewrite; # path not valid when
> rewriting active > > restores this functionality and makes it work like it should. > > setting "-rewrite => 0" unfortunately isn't an option because it then > uses "script_name" instead of "request_uri" to construct the URL which > is not what I want. > > I can see no harm in removing that line, as $path is only used if > manually requested with url(-path =>1) - and then it should just do > what I requested, not try to second-guess. > > I encountered that bug with CGI.pm 3.43 , but a quick check via CPAN > shows that the offending code is still there in 3.49
Thanks for the report. I'm considering accepting this patch, and am currently getting the proposal peer reviewed on the CGI::Application mailing list. I found that I'm bitten by it as well-- it affects some CGI.pm upgrades, because older versions used to work as you proposed, some the current version has the bad behavior you recommend reverting. Mark
Subject: Re: [rt.cpan.org #45019] Needs Discussion: PATCH: url() rewrite/path_info handling not compatible with CGI::Application::Dispatch rewriting
Date: Fri, 13 May 2011 16:47:03 -0400
To: bug-CGI.pm [...] rt.cpan.org
From: Lincoln Stein <lincoln.stein [...] gmail.com>
Download (untitled) / with headers
text/plain 1.8k
I think it is fair to remove the line. Caveat emptor when using mod_rewrite. CGI.pm shouldn't try to second guess the user. Lincoln On Fri, May 13, 2011 at 3:43 PM, MARKSTOS via RT <bug-CGI.pm@rt.cpan.org>wrote: Show quoted text
> Queue: CGI.pm > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=45019 > > > On Mon Jun 28 06:30:31 2010, sec@42.org wrote:
> > Hi, > > > > I have encountered a problem with CGI.pm's url() function. When > > mod_rewrite is active. > > > > In these cases it then unconditionally clears "path_info" so that > > url() and url(-path => 1) always return the same info. > > > > Removing line 2720:
> > > undef $path if $rewrite_in_use && $rewrite; # path not valid when
> > rewriting active > > > > restores this functionality and makes it work like it should. > > > > setting "-rewrite => 0" unfortunately isn't an option because it then > > uses "script_name" instead of "request_uri" to construct the URL which > > is not what I want. > > > > I can see no harm in removing that line, as $path is only used if > > manually requested with url(-path =>1) - and then it should just do > > what I requested, not try to second-guess. > > > > I encountered that bug with CGI.pm 3.43 , but a quick check via CPAN > > shows that the offending code is still there in 3.49
> > Thanks for the report. I'm considering accepting this patch, and am > currently getting the proposal peer reviewed on the CGI::Application > mailing list. > > I found that I'm bitten by it as well-- it affects some CGI.pm upgrades, > because older versions used to work as you proposed, some the current > version has the bad behavior you recommend reverting. > > Mark >
-- Lincoln D. Stein Director, Informatics and Biocomputing Platform Ontario Institute for Cancer Research 101 College St., Suite 800 Toronto, ON, Canada M5G0A3 416 673-8514 Assistant: Renata Musa <Renata.Musa@oicr.on.ca>
Download (untitled) / with headers
text/plain 857b
Thanks for the comment, Lincoln. As I wrote tests for this and reviewed it more detail, it clearer to me that this is not just a "behavior change", it's a bug fix for a bug that happens to be very old. I was considering doing a "developer release" for a comment period on the change, but now that's more clearly a bug fix, I think we should go ahead with a next release, with a clear warning in the "Changes" file that people may have come to depend on the old-but-buggy behavior, and should check their apps. The full proposed diff is here. I've also posted into the CGI::Application mailing list. Excepting blocking feedback, I would plan to release the new version later this week ( and excepting that my wife doesn't go into labor this week ). https://github.com/markstos/CGI.pm/commit/0023f178943c687333285713971ea8 2e54b3dcc6 Mark
Download (untitled) / with headers
text/plain 164b
My wife did go into labor, but I'm back at work now after paternity leave and have just now pushed 3.55 to CPAN to address solely this issue. Resolving. Mark
Subject: Re: [rt.cpan.org #45019] Needs Discussion: PATCH: url() rewrite/path_info handling not compatible with CGI::Application::Dispatch rewriting
Date: Sun, 05 Jun 2011 21:33:49 -0400
To: bug-CGI.pm [...] rt.cpan.org
From: Yanick Champoux <yanick [...] babyl.dyndns.org>
Download (untitled) / with headers
text/plain 353b
On 11-06-03 11:40 AM, MARKSTOS via RT wrote: Show quoted text
> My wife did go into labor, but I'm back at work now after paternity leave > and have just now pushed 3.55 to CPAN to address solely this issue.
Absolutely unrelated to the ticket, but I just read this, and I have to send my congrats to the happy dad and mom, and my welcomes to the new wee hacker. :-)


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.