Bug #36312 for CGI: Needs Confirmation: PATCH: Also consider REDIRECT_QUERY_STRING in mod

Fri May 30 18:40:44 2008 bitcard [...] purplefoot.com - Ticket created

Subject:

Potential bug with init function

So in init there is this code: # If method is GET or HEAD, fetch the query from # the environment. if ($meth=~/^(GET|HEAD)$/) { if ($MOD_PERL) { $query_string = $self->r->args; } else { $query_string = $ENV{'QUERY_STRING'} if defined $ENV{'QUERY_STRING'}; $query_string ||= $ENV{'REDIRECT_QUERY_STRING'} if defined $ENV{'REDIRECT_QUERY_STRING'}; } last METHOD; } Due to internal redirects I was losing the form in mod_perl. I created an override library for myself this has this change which fixed the issue: # If method is GET or HEAD, fetch the query from # the environment. if ($meth=~/^(GET|HEAD)$/) { if ($MOD_PERL) { $query_string = $self->r->args; $query_string ||= $ENV{'REDIRECT_QUERY_STRING'} if defined $ENV{'REDIRECT_QUERY_STRING'} && $query_string eq ''; } else { $query_string = $ENV{'QUERY_STRING'} if defined $ENV{'QUERY_STRING'}; $query_string ||= $ENV{'REDIRECT_QUERY_STRING'} if defined $ENV{'REDIRECT_QUERY_STRING'}; } last METHOD; }

Sat Jul 25 16:55:47 2009 MARKSTOS [...] cpan.org - Subject changed from 'Potential bug with init function' to 'PATCH: Also consider REDIRECT_QUERY_STRING in mod_perl'

Sat Jul 25 16:58:30 2009 MARKSTOS [...] cpan.org - Correspondence added

On Fri May 30 18:40:44 2008, wetnun wrote: Show quoted text

> So in init there is this code: > > # If method is GET or HEAD, fetch the query from > # the environment. > if ($meth=~/^(GET|HEAD)$/) { > if ($MOD_PERL) { > $query_string = $self->r->args; > } else { > $query_string = $ENV{'QUERY_STRING'} if defined > $ENV{'QUERY_STRING'}; > $query_string ||= $ENV{'REDIRECT_QUERY_STRING'} if defined > $ENV{'REDIRECT_QUERY_STRING'}; > } > last METHOD; > } > > Due to internal redirects I was losing the form in mod_perl. I created > an override library for myself this has this change which fixed the issue: > > # If method is GET or HEAD, fetch the query from > # the environment. > if ($meth=~/^(GET|HEAD)$/) { > if ($MOD_PERL) { > $query_string = $self->r->args; > $query_string ||= $ENV{'REDIRECT_QUERY_STRING'} if defined > $ENV{'REDIRECT_QUERY_STRING'} && $query_string eq ''; > } else { > $query_string = $ENV{'QUERY_STRING'} if defined > $ENV{'QUERY_STRING'}; > $query_string ||= $ENV{'REDIRECT_QUERY_STRING'} if defined > $ENV{'REDIRECT_QUERY_STRING'}; > } > last METHOD; > }

I have peer-reviewed this patch and recommend that it be accepted. However, it adds the same line both the "if" and "else" clause. To reduce code duplication, I would implement the fix by moving this line from the "else" to below the if/else clause: $query_string ||= $ENV{'REDIRECT_QUERY_STRING'} if defined $ENV{'REDIRECT_QUERY_STRING'};

Sat Jul 25 16:58:31 2009 The RT System itself - Status changed from 'new' to 'open'

Mon Jul 27 00:45:26 2009 jon [...] purplefoot.com - Correspondence added

Subject:	Re: [rt.cpan.org #36312] PATCH: Also consider REDIRECT_QUERY_STRING in mod_perl
Date:	Sun, 26 Jul 2009 21:43:59 -0700
To:	bug-CGI.pm [...] rt.cpan.org
From:	Jon Stephens <jon [...] purplefoot.com>

Sounds good to me. :) MARKSTOS via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=36312 > > > On Fri May 30 18:40:44 2008, wetnun wrote: >

>> So in init there is this code: >> >> # If method is GET or HEAD, fetch the query from >> # the environment. >> if ($meth=~/^(GET|HEAD)$/) { >> if ($MOD_PERL) { >> $query_string = $self->r->args; >> } else { >> $query_string = $ENV{'QUERY_STRING'} if defined >> $ENV{'QUERY_STRING'}; >> $query_string ||= $ENV{'REDIRECT_QUERY_STRING'} if defined >> $ENV{'REDIRECT_QUERY_STRING'}; >> } >> last METHOD; >> } >> >> Due to internal redirects I was losing the form in mod_perl. I created >> an override library for myself this has this change which fixed the issue: >> >> # If method is GET or HEAD, fetch the query from >> # the environment. >> if ($meth=~/^(GET|HEAD)$/) { >> if ($MOD_PERL) { >> $query_string = $self->r->args; >> $query_string ||= $ENV{'REDIRECT_QUERY_STRING'} if defined >> $ENV{'REDIRECT_QUERY_STRING'} && $query_string eq ''; >> } else { >> $query_string = $ENV{'QUERY_STRING'} if defined >> $ENV{'QUERY_STRING'}; >> $query_string ||= $ENV{'REDIRECT_QUERY_STRING'} if defined >> $ENV{'REDIRECT_QUERY_STRING'}; >> } >> last METHOD; >> } >>

> > > I have peer-reviewed this patch and recommend that it be accepted. > However, it adds the same line both the "if" and "else" clause. To > reduce code duplication, I would implement the fix by moving this line > from the "else" to below the if/else clause: > > $query_string ||= $ENV{'REDIRECT_QUERY_STRING'} if defined > $ENV{'REDIRECT_QUERY_STRING'}; > ------------------------------------------------------------------------ > > > No virus found in this incoming message. > Checked by AVG - www.avg.com > Version: 8.5.375 / Virus Database: 270.13.29/2261 - Release Date: 07/25/09 05:58:00 > >

Fri Jul 31 05:23:25 2009 burak [...] cpan.org - Correspondence added

25 Tem. 2009 Cmt., 16:58:30 tarihinde, MARKSTOS yazdı: Show quoted text

> I have peer-reviewed this patch and recommend that it be accepted. > However, it adds the same line both the "if" and "else" clause. To > reduce code duplication, I would implement the fix by moving this line > from the "else" to below the if/else clause: > > $query_string ||= $ENV{'REDIRECT_QUERY_STRING'} if defined > $ENV{'REDIRECT_QUERY_STRING'};

I'm not so sure about this. I've experienced a similar issue with redirects (and it took a while to realise that multiple redirects can happen) and can say that this is an apache specific issue and the parameter gets "REDIRECT_" prefix everytime a redirect occurs. So, if it's redirected again, it'll be "REDIRECT_REDIRECT_QUERY_STRING". The code should check for $ENV{ qr/REDIRECT_ .* QUERY_STRING/ } (pseudo code) or should not check it at all.

Fri Jul 31 09:21:28 2009 MARKSTOS [...] cpan.org - Correspondence added

On Fri Jul 31 05:23:25 2009, BURAK wrote: Show quoted text

> 25 Tem. 2009 Cmt., 16:58:30 tarihinde, MARKSTOS yazdı: >

> > I have peer-reviewed this patch and recommend that it be accepted. > > However, it adds the same line both the "if" and "else" clause. To > > reduce code duplication, I would implement the fix by moving this line > > from the "else" to below the if/else clause: > > > > $query_string ||= $ENV{'REDIRECT_QUERY_STRING'} if defined > > $ENV{'REDIRECT_QUERY_STRING'};

> > I'm not so sure about this. I've experienced a similar issue with > redirects (and it took a while to realise that multiple redirects can > happen) and can say that this is an apache specific issue and the > parameter gets "REDIRECT_" prefix everytime a redirect occurs. So, if > it's redirected again, it'll be "REDIRECT_REDIRECT_QUERY_STRING". The > code should check for $ENV{ qr/REDIRECT_ .* QUERY_STRING/ } (pseudo > code) or should not check it at all.

Thanks for the input. Is there a link to Apache documentation which confirms this, or is there another way to confirm the REDIRECT_* behavior? It would be useful to know for example how many redirects it will do before it gives up. Mark

Fri Jul 31 22:16:59 2009 MARKSTOS [...] cpan.org - Subject changed from 'PATCH: Also consider REDIRECT_QUERY_STRING in mod_perl' to 'Needs Confirmation: PATCH: Also consider REDIRECT_QUERY_STRING in mod_perl'

Sat Aug 01 23:29:51 2009 burak [...] cpan.org - Correspondence added

31 Tem. 2009 Cum., 09:21:28 tarihinde, MARKSTOS yazdı: Show quoted text

> Thanks for the input. Is there a link to Apache documentation which > confirms this, or is there another way to confirm the REDIRECT_* > behavior? It would be useful to know for example how many redirects it > will do before it gives up. > > Mark

There is this documentation, but it does not discuss multiple redirects: http://httpd.apache.org/docs/2.0/en/custom-error.html However, it says: "A new batch of environment variables will be initialized for use by a script which has been redirected to. Each new variable will have the prefix REDIRECT_. REDIRECT_ environment variables are created from the CGI environment variables which existed prior to the redirect, they are renamed with a REDIRECT_ prefix, ...". So, if a redirect occurs %ENV will be renamed to have that prefix. If a second redirect occurs, it'll be REDIRECT_REDIRECT_ and if it's more then we can say 'REDIRECT_' x $n :) I can confirm that two level redirects can happen tho. How deep this goes? this I don't have a clue. Maybe asking in the apache mailing list or something will help.

Wed Jan 23 21:24:19 2013 MARKSTOS [...] cpan.org - Correspondence added

I would like to make the CGI and mod_perl symmetric here, but I'm not interested in supporting N levels of redirect. From reading more of the mod_perl docs, I think the more mod_perl'ish thing to be might be to use the "prev" method to get a possible previous request object in the redirect cycle. So the proposed code would be something like this for mod_perl: $query_string = $self->r->args || $self->r->prev->args; Anyone care to test out variations of that under mod_perl, with and without redirects in play?

Thu May 22 08:03:56 2014 LEEJO [...] cpan.org - Correspondence added

This issue has been copied to: https://github.com/leejo/CGI.pm/issues/55 please take all future correspondence there. This ticket will remain open but please do not reply here. This ticket will be closed when the github issue is dealt with.

Fri May 23 14:28:02 2014 The RT System itself - Queue changed from CGI.pm to CGI

Sat Jun 14 14:55:48 2014 LEEJO [...] cpan.org - Correspondence added

I have gone with both solutions here. If under mod_perl2 then we will try to use the ->prev method. Since this is a mod_perl2-ism only, i.e. not mod_perl1.* then i have also added support for getting the query string from the REDIRECT_ env variables, which will support up to 5 redirects. Will go out in the next release. Thanks! commit 1079337db6b05beb3c564c980f4262700b571eb9 Author: Lee Johnson <lee@givengain.ch> Date: Sat Jun 14 20:48:44 2014 +0200 resolve #55 [rt.cpan.org #36312] - redirect query string handling, specifically with mod_perl2 - use the ->prev method of the Apache::RequestUtil object to get at the *previous* request object and thus its env variables for getting the query string in theory this would support an unlimited number of redirects as the next redirect will always pass on the details from the previous redirect this is mod_perl2 only, i cannot see this in Apache->request so it does not appear to be supported in mod_perl1.*. To get around this use the REDIRECT_ env variable for up to 5 redirects, this also handles up to 5 redirects when not running under mod perl. the logic to get the query string from the ENV has been split into its own method to remove duplication, and a test has been added. currently there are no tests for mod_perl, Apache, Apache2 use here, but i don't see any tests at all for any parts of the code using these modules given i don't have them installed and all the tests currently pass (the Apache/Apache2 modules are runtime requires so not hard dependencies)

Sat Jun 14 14:55:50 2014 LEEJO [...] cpan.org - Status changed from 'open' to 'resolved'

Bug #36312 for CGI: Needs Confirmation: PATCH: Also consider REDIRECT_QUERY_STRING in mod_perl

Preferred bug tracker