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: 32119
Status: resolved
Priority: 0/
Queue: CGI

People
Owner: MARKSTOS [...] cpan.org
Requestors: agentzh [...] yahoo.cn
Cc:
AdminCc:

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



Subject: $CGI::POST_MAX gets reset in CGI::Fast::new
Download (untitled) / with headers
text/plain 490b
Hi~ I've noticed that setting $CGI::POST_MAX has no effect at all when using CGI::Fast. By reading the code, I've found the reason in CGI::Fast::New: CGI->_reset_globals; return $CGI::Q = $self->SUPER::new($initializer, @param); Here CGI's _reset_globals method calls initialize_globals which in turn resets POST_MAX to -1 which overrides my own setting. So the same is true for other variables like DISABLE_UPLOADS. Hopefully this bug will get fixed soon ;) Thanks! -agentzh
It should be an easy fix. I'll send you a patch to try.
Subject: Re: [rt.cpan.org #32119] $CGI::POST_MAX gets reset in CGI::Fast::new
Date: Tue, 8 Jan 2008 10:44:47 +0800
To: bug-CGI.pm [...] rt.cpan.org
From: "Agent Zhang" <agentzh [...] gmail.com>
Download (untitled) / with headers
text/plain 200b
On Jan 7, 2008 11:25 PM, Lincoln_D_Stein via RT <bug-CGI.pm@rt.cpan.org> wrote: Show quoted text
> > It should be an easy fix. I'll send you a patch to try. >
Cool! Looking forward to your patch :) Thanks! -agentzh
From: dst
Download (untitled) / with headers
text/plain 566b
Hi, I'm not the original reporter, but this bug really bothers me and I have a fix that I'd like to discuss. The solution is simple: Resetting the state variables has to be separated from resetting the settings variables in CGI.pm, so that CGI/Fast.pm can reset only the first on a new request. I'm still a little unhappy with the name for the new function I introduced, but the patch itself is working here. Although the function "_reset_globals" is neither public nor mentioned in the documentation I did not dare to change its behaviour. Regards, dst
Download cgi-fix.diff
text/x-diff 1.5k
diff -ur CGI.pm-3.33.orig/CGI/Fast.pm CGI.pm-3.33/CGI/Fast.pm --- CGI.pm-3.33.orig/CGI/Fast.pm 2006-02-23 16:39:44.000000000 +0100 +++ CGI.pm-3.33/CGI/Fast.pm 2008-03-11 17:17:35.000000000 +0100 @@ -54,7 +54,7 @@ return undef unless FCGI::accept() >= 0; } } - CGI->_reset_globals; + CGI->_reset_globals_keep_settings; return $CGI::Q = $self->SUPER::new($initializer, @param); } --- CGI.pm-3.33.orig/CGI.pm 2008-01-03 16:01:27.000000000 +0100 +++ CGI.pm-3.33/CGI.pm 2008-03-11 17:17:23.000000000 +0100 @@ -42,7 +42,7 @@ # >>>>> Here are some globals that you might want to adjust <<<<<< -sub initialize_globals { +sub initialize_settings { # Set this to 1 to enable copious autoloader debugging messages $AUTOLOAD_DEBUG = 0; @@ -113,6 +113,9 @@ # return everything as utf-8 $PARAM_UTF8 = 0; +} + +sub initialize_globals { # Other globals that you shouldn't worry about. undef $Q; @@ -133,4 +136,5 @@ *end_form = \&endform; # make mod_perlhappy +initialize_settings(); initialize_globals(); # FIGURE OUT THE OS WE'RE RUNNING UNDER @@ -890,7 +894,16 @@ return $XHTML ? qq(checked="checked" ) : qq(checked ); } -sub _reset_globals { initialize_globals(); } +sub _reset_globals { + initialize_settings(); + initialize_globals(); +} + +sub _reset_globals_keep_settings { + # CGI::Fast uses this function to reset the "globals" between requests + # and still keep the user settings. + initialize_globals(); +} sub _setup_symbols { my $self = shift;
Proposed patch enclosed. Please test and if it works I'll release it in 3.37.
Index: CGI/Fast.pm =================================================================== RCS file: /usr/local/cvs_repository/CGI.pm/CGI/Fast.pm,v retrieving revision 1.9 diff -u -r1.9 Fast.pm --- CGI/Fast.pm 24 Feb 2006 19:02:24 -0000 1.9 +++ CGI/Fast.pm 14 Apr 2008 17:54:24 -0000 @@ -55,6 +55,7 @@ } } CGI->_reset_globals; + $self->_setup_symbols(@SAVED_SYMBOLS) if @CGI::SAVED_SYMBOLS; return $CGI::Q = $self->SUPER::new($initializer, @param); }
Didn't send the whole thing. Here it is.
Index: CGI.pm =================================================================== RCS file: /usr/local/cvs_repository/CGI.pm/CGI.pm,v retrieving revision 1.250 diff -u -r1.250 CGI.pm --- CGI.pm 27 Mar 2008 14:24:25 -0000 1.250 +++ CGI.pm 14 Apr 2008 17:59:56 -0000 @@ -19,7 +19,7 @@ # http://stein.cshl.org/WWW/software/CGI/ $CGI::revision = '$Id: CGI.pm,v 1.250 2008/03/27 14:24:25 lstein Exp $'; -$CGI::VERSION='3.35'; +$CGI::VERSION='3.36'; # HARD-CODED LOCATION FOR FILE UPLOAD TEMPORARY FILES. # UNCOMMENT THIS ONLY IF YOU KNOW WHAT YOU'RE DOING. @@ -37,7 +37,12 @@ $TAINTED = substr("$0$^X",0,0); } -$MOD_PERL = 0; # no mod_perl by default +$MOD_PERL = 0; # no mod_perl by default + +#global settings +$POST_MAX = -1; # no limit to uploaded files +$DISABLE_UPLOADS = 0; + @SAVED_SYMBOLS = (); @@ -91,13 +96,6 @@ # it can just be renamed, instead of read and written. $CLOSE_UPLOAD_FILES = 0; - # Set this to a positive value to limit the size of a POSTing - # to a certain number of bytes: - $POST_MAX = -1; - - # Change this to 1 to disable uploads entirely: - $DISABLE_UPLOADS = 0; - # Automatically determined -- don't change $EBCDIC = 0; @@ -355,6 +353,7 @@ $self->r(Apache->request) unless $self->r; my $r = $self->r; $r->register_cleanup(\&CGI::_reset_globals); + $self->_setup_symbols(@SAVED_SYMBOLS) if @SAVED_SYMBOLS; } else { # XXX: once we have the new API @@ -363,6 +362,7 @@ my $r = $self->r; $r->subprocess_env unless exists $ENV{REQUEST_METHOD}; $r->pool->cleanup_register(\&CGI::_reset_globals); + $self->_setup_symbols(@SAVED_SYMBOLS) if @SAVED_SYMBOLS; } undef $NPH; } Index: Changes =================================================================== RCS file: /usr/local/cvs_repository/CGI.pm/Changes,v retrieving revision 1.69 diff -u -r1.69 Changes --- Changes 25 Mar 2008 15:15:45 -0000 1.69 +++ Changes 14 Apr 2008 17:59:56 -0000 @@ -1,3 +1,9 @@ + Version 3.37 + 1. Fix pragmas so that they persist over modperl invocations (e.g. RT 34761) + + Version 3.36 + 1. Fix CGI::Cookie to support cookies that are separated by "," instead of ";". + Version 3.35 1. Resync with bleadperl, primarily fixing a bug in parsing semicolons in uploaded filenames. Index: CGI/Cookie.pm =================================================================== RCS file: /usr/local/cvs_repository/CGI.pm/CGI/Cookie.pm,v retrieving revision 1.38 diff -u -r1.38 Cookie.pm --- CGI/Cookie.pm 19 Mar 2007 13:24:54 -0000 1.38 +++ CGI/Cookie.pm 14 Apr 2008 17:59:56 -0000 @@ -13,7 +13,7 @@ # wish, but if you redistribute a modified version, please attach a note # listing the modifications you have made. -$CGI::Cookie::VERSION='1.28'; +$CGI::Cookie::VERSION='1.29'; use CGI::Util qw(rearrange unescape escape); use CGI; @@ -51,7 +51,7 @@ my %results; my($key,$value); - my(@pairs) = split("[;,] ?",$raw_cookie); + my @pairs = split("[;,] ?",$raw_cookie); foreach (@pairs) { s/\s*(.*?)\s*/$1/; if (/^([^=]+)=(.*)/) { @@ -88,7 +88,7 @@ my ($self,$raw_cookie) = @_; my %results; - my(@pairs) = split("; ?",$raw_cookie); + my @pairs = split("[;,] ?",$raw_cookie); foreach (@pairs) { s/\s*(.*?)\s*/$1/; my($key,$value) = split("=",$_,2); Index: CGI/Fast.pm =================================================================== RCS file: /usr/local/cvs_repository/CGI.pm/CGI/Fast.pm,v retrieving revision 1.9 diff -u -r1.9 Fast.pm --- CGI/Fast.pm 24 Feb 2006 19:02:24 -0000 1.9 +++ CGI/Fast.pm 14 Apr 2008 17:59:56 -0000 @@ -55,6 +55,7 @@ } } CGI->_reset_globals; + $self->_setup_symbols(@SAVED_SYMBOLS) if @CGI::SAVED_SYMBOLS; return $CGI::Q = $self->SUPER::new($initializer, @param); } Index: t/upload.t =================================================================== RCS file: /usr/local/cvs_repository/CGI.pm/t/upload.t,v retrieving revision 1.4 diff -u -r1.4 upload.t --- t/upload.t 16 Apr 2007 16:54:42 -0000 1.4 +++ t/upload.t 14 Apr 2008 17:59:56 -0000 @@ -71,7 +71,8 @@ { my $test = "multiple file names are handled right with same-named upload fields"; my @hello_names = $q->param('hello_world'); - is_deeply(\@hello_names, [ 'goodbye_world.txt','hello_world.txt' ], $test); + is ($hello_names[0],'goodbye_world.txt',$test. "...first file"); + is ($hello_names[1],'hello_world.txt',$test. "...second file"); } #-----------------------------------------------------------------------------
Download (untitled) / with headers
text/plain 894b
There's already a mechanism for maintaining pragma settings; I just added that to CGI::Fast so that it works. Unfortunately the two upload globals do not use the pragma API and so they have to be moved out of the reset_globals call chain entirely. On Tue Mar 11 12:34:17 2008, dst wrote: Show quoted text
> > Hi, > > I'm not the original reporter, but this bug really bothers me and I > have a fix that I'd like to discuss. > > The solution is simple: Resetting the state variables has to be > separated from resetting the settings variables in CGI.pm, so that > CGI/Fast.pm can reset only the first on a new request. > > I'm still a little unhappy with the name for the new function I > introduced, but the patch itself is working here. Although the > function "_reset_globals" is neither public nor mentioned in the > documentation I did not dare to change its behaviour. > > Regards, > dst
Could someone comment on whether this bug still exists in CGI.pm 3.43? Mark
Download (untitled) / with headers
text/plain 378b
Hi Mark I'm running version 3.45 and it still exists. I just discovered that when I was trying to use the -private_tempfiles pragma. On the first call it works, on the other call it set the values to the default in initialize_globals(); Em Sáb. Jul. 25 17:36:38 2009, MARKSTOS escreveu: Show quoted text
> Could someone comment on whether this bug still exists in CGI.pm 3.43? > > Mark
Download (untitled) / with headers
text/plain 445b
Show quoted text
> I just discovered that when I was trying to use the -private_tempfiles > pragma. On the first call it works, on the other call it set the values > to the default in initialize_globals();
Aha! This sounds like the same bug as RT#30774: "modperl resets $XHTML to 1 after first request" http://rt.cpan.org/Public/Bug/Display.html?id=30774 It seems like there's a general problem with the global variables in persistent environments. Mark
Download (untitled) / with headers
text/plain 879b
On Wed Sep 02 09:43:38 2009, MARKSTOS wrote: Show quoted text
>
> > I just discovered that when I was trying to use the -private_tempfiles > > pragma. On the first call it works, on the other call it set the values > > to the default in initialize_globals();
> > Aha! This sounds like the same bug as RT#30774: > > "modperl resets $XHTML to 1 after first request" > http://rt.cpan.org/Public/Bug/Display.html?id=30774
In that other ticket, I suggested this fix, although I don't have handy access to a persistent environment to test it in. Replace "sub _reset_globals" like this: sub _reset_globals { initialize_globals(); CGI->_setup_symbols(@SAVED_SYMBOLS); } If that doesn't work, I think it's close to the solution. Perhaps you could review the related code, test more and formulate a patch. We'll be happy to give you credit for this fix in the next release. Thanks! Mark
Download (untitled) / with headers
text/plain 1.5k
On Wed Sep 02 06:00:55 2009, heinst wrote: Show quoted text
> Hi Mark > > I'm running version 3.45 and it still exists. > > I just discovered that when I was trying to use the -private_tempfiles > pragma. On the first call it works, on the other call it set the values > to the default in initialize_globals();
First, I want to say that I think this is rather separate issue than the POST_MAX issue, and I think from a careful code reading that the POST_MAX issue is resolved. Also, please ignore my first attempt at patch for this. Here's my new suggestion. In CGI::Fast, update this line: $self->_setup_symbols(@SAVED_SYMBOLS) if @CGI::SAVED_SYMBOLS; I think it should read like this: $self->_setup_symbols(@CGI::SAVED_SYMBOLS) if @CGI::SAVED_SYMBOLS; Notice how the first line prefixed one copy of @SAVED_SYMBOLS with $CGI::, but the other? To confirm how package variables would be inherited, I wrote this little test script: ### package Parent; $SAVED_SYMBOLS = 'in parent!'; package Child; sub new { print "no: $SAVED_SYMBOLS\n"; print "parent: $Parent::SAVED_SYMBOLS\n"; print "child: $Child::SAVED_SYMBOLS\n"; } Child->new; ### It shows that the only way this variable works is if it fully qualified with the parent's name space. I think that means that in $CGI::Fast, @SAVED_SYMBOLS will always be undefined, and that $CGI::SAVED_SYMBOLS is what you want. Not that these "symbols" always just reply to the pragmas given to CGI.pm on the "use" line. A few global variables like $POST_MAX are handled differently. So could you test the new patch for us? Mark
Download (untitled) / with headers
text/plain 224b
I found I was able to write an automated test to confirm the bug and the fix directly. The patch will be pushed to my github repo shortly. The fix was as I suspected: qualifying @SAVED_SYMBOLS to be in the CGI:: name space.
Subject: Thanks, released
Download (untitled) / with headers
text/plain 160b
The patch for this ticket has now been released in CGI.pm 3.47, and this ticket is considered resolved. Thanks again for you help to improve CGI.pm! Mark


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.