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
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>
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
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
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"); } #-----------------------------------------------------------------------------
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
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
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
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
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
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
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