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

People
Owner: MARKSTOS [...] cpan.org
Requestors: philip.g.potter [...] gmail.com
Cc:
AdminCc:

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



Subject: PUT method with empty body seems to freeze CGI->new()
Download (untitled) / with headers
text/plain 1.6k
A PUT HTTP request with no body and no Content-Length or Content-Type headers seems to freeze up CGI->new(). I found this while trying to install Test::WWW::Mechanize, whose t/put_ok.t script fails under CGI versions 3.44, 3.45, and 3.48. 3.43 works fine, as does 3.29; I haven't tested any others. Steps to reproduce: 1. install CGI 3.44 or later 2. get Test::WWW::Mechanize (I'm using 1.24 but I don't think it matters) 3. unpack T::WWW::M 4. in the T::WWW::M directory, call "perl Makefile.PL" and "make" to set up the blib/ directory 5. in the T::WWW::M directory, call "prove -bv t/put_ok.t". The HTTP PUT requests will time out. I have narrowed the path of execution down to this: * T::WWW::M's test t/put_ok.t calls the testserver in t/TestServer.pm * TestServer subclasses HTTP::Server::Simple::CGI * The method HTTP::Server::Simple::CGI::handler calls CGI->new() * CGI->new() calls CGI->init() * CGI->init() calls CGI->read_from_stdin() around line 651 * CGI->read_from_stdin() never returns It's difficult to say that this is invalid behaviour, since the utility of an empty PUT request is questionable. However there's nothing in the RFC I can see which explicitly forbids an empty PUT request, and if there's no Content-Length header then it's certainly possible that there's no body to the HTTP request. In any case, even if you think an empty HTTP PUT request is invalid, CGI should probably fail gracefully rather than getting stuck. I had previously raised this issue with T::WWW::M before finding out that CGI is probably responsible. The previous discussion can be viewed here: http://code.google.com/p/www-mechanize/issues/detail?id=127
Subject: Re: [rt.cpan.org #52469] PUT method with empty body seems to freeze CGI->new()
Date: Sun, 06 Dec 2009 12:04:54 -0500
To: bug-CGI.pm [...] rt.cpan.org
From: Yanick Champoux <yanick [...] babyl.dyndns.org>
Download (untitled) / with headers
text/plain 314b
Philip Potter via RT wrote: Show quoted text
> A PUT HTTP request with no body and no Content-Length or Content-Type > headers seems to freeze up CGI->new().
Here's a minimal test that reproduces the problem: use CGI; $ENV{REQUEST_METHOD} = 'PUT'; my $cgi = CGI->new; Yup, that's it. Told ya it was minimal. :-) `/anick
Subject: Re: [rt.cpan.org #52469] PUT method with empty body seems to freeze CGI->new()
Date: Sun, 06 Dec 2009 12:50:27 -0500
To: bug-CGI.pm [...] rt.cpan.org
From: Yanick Champoux <yanick [...] babyl.dyndns.org>
Download (untitled) / with headers
text/plain 207b
ah AH! Got the culprit! Commit 11a14db3b849388a in the Git repo. Patch made the 14th of May 2009, with the comment 'Patch from Kurt Jaeger to allow HTTP PUT even if the content length is unknown.'
Subject: Re: [rt.cpan.org #52469] PUT method with empty body seems to freeze CGI->new()
Date: Sun, 06 Dec 2009 14:35:36 -0500
To: bug-CGI.pm [...] rt.cpan.org
From: Yanick Champoux <yanick [...] babyl.dyndns.org>
Download (untitled) / with headers
text/plain 370b
Potential fix at http://github.com/yanick/CGI.pm/commits/rt-52469 I inserted a timeout of 5 seconds for reading a line from STDIN. After that time, if we got nothing we bail out of the read_from_stdin function. I'm trying to remember if there's a more elegant way to verify that something is coming down the STDIN pipe, but I can't think of anything right now...
On Sun Dec 06 14:32:47 2009, yanick@babyl.dyndns.org wrote: Show quoted text
> I'm trying to remember if there's a more elegant way to verify that > something is coming down the STDIN pipe, but I can't think of anything > right now...
It's certainly not ideal; it's trying to second-guess the user's behaviour when they haven't given us enough information to work out what they're doing. It's like trying to work out the user's charset when they haven't told you; really, you just want to tell them to go away until they present their data properly. Idealistically, I would say if the user doesn't put a Content-Length: header then they can't expect any content to be used. Are there user agents in the wild which make PUT requests without setting Content-Length? Clearly if there are a significant number then they should be supported; but if not (and given how many people make PUT requests in the first place, I suspect not) then is this something that needs to be supported? Was the original change to support this made in response to user demand? Does it have an attached ticket?
Download (untitled) / with headers
text/plain 872b
Thanks for the report. To consider this, I read some of the HTTP RFC 2616 myself, as well as the CGI RFC 3875: http://www.ietf.org/rfc/rfc3875 In particular, there's this: "The server MUST set this meta-variable if and only if the request is accompanied by a message-body entity." I also commented on the issue via github: http://github.com/yanick/CGI.pm/commit/b132e1a6676b1698b15b1ece0944ac620 4e17fc7 I think the CGI::Simple approach closely models the spec, and we consider that approach: If a content_length is set, it reads that many bytes. If no content_length is set, it reads no bytes. I read the HTTP spec first, and got on detour reading about "Tranfer- Encoding" and chunked messages, but the CGI spec seems rather clear on the matter. Yanick, would you mind trying a second patch that more closely follows the CGI::Simple approach? Mark
Download (untitled) / with headers
text/plain 177b
Hi Mark, Thanks for your message. I largely agree with your approach here. The CGI::Simple approach seems correct to me. Second guessing the user agent just seems like trouble.
Subject: Re: [rt.cpan.org #52469] PUT method with empty body seems to freeze CGI->new()
Date: Thu, 10 Dec 2009 12:18:00 -0500
To: bug-CGI.pm [...] rt.cpan.org
From: Yanick Champoux <yanick.champoux [...] gmail.com>
Download (untitled) / with headers
text/plain 199b
On December 9, 2009 09:46:52 pm MARKSTOS via RT wrote: Show quoted text
> Yanick, would you mind trying a second patch that more closely follows > the CGI::Simple approach?
No problem at all, will do. :-) `/anick
RT-Send-CC: yanick.champoux [...] gmail.com
Download (untitled) / with headers
text/plain 112b
Yanick or Philip, of you could submit a refined patch here based on the CGI::Simple code, that would be great.
Subject: Re: [rt.cpan.org #52469] PUT method with empty body seems to freeze CGI->new()
Date: Sat, 18 Dec 2010 12:50:32 -0500
To: bug-CGI.pm [...] rt.cpan.org
From: Yanick Champoux <yanick [...] babyl.dyndns.org>
Download (untitled) / with headers
text/plain 172b
Show quoted text
> Yanick or Philip, of you could submit a refined patch here based on the > CGI::Simple code, that would be great.
No problem. I'll try to do that tonight. Joy, `/anick
Subject: Re: [rt.cpan.org #52469] PUT method with empty body seems to freeze CGI->new()
Date: Tue, 21 Dec 2010 12:13:24 -0500
To: bug-CGI.pm [...] rt.cpan.org
From: Yanick Champoux <yanick [...] babyl.dyndns.org>
Download (untitled) / with headers
text/plain 752b
Looking at the code, and if we go the way of not reading anything if CONTENT_LENGTH is not defined, I think we'll interfere with the stanza of line 666 of CGI.pm: elsif (not defined $ENV{CONTENT_LENGTH}) { $self->read_from_stdin(\$query_string); # should this be PUTDATA in case of PUT ? my($param) = $meth . 'DATA' ; $self->add_parameter($param) ; push (@{$self->{param}{$param}},$query_string); undef $query_string ; } If I'm not mistaken, this is probably there to take care of the case where the script is run from the command-line. On the plus side, though, this seems to be the only place where read_from_stdin is used, so if we go ahead with the change, at least the effects of it will be contained.
Download (untitled) / with headers
text/plain 1.1k
On Tue Dec 21 12:12:35 2010, yanick@babyl.dyndns.org wrote: Show quoted text
> > Looking at the code, and if we go the way of not reading anything if > CONTENT_LENGTH is not defined, I think we'll interfere with the stanza > of line 666 of CGI.pm: > > elsif (not defined $ENV{CONTENT_LENGTH}) { > $self->read_from_stdin(\$query_string); > # should this be PUTDATA in case of PUT ? > my($param) = $meth . 'DATA' ; > $self->add_parameter($param) ; > push (@{$self->{param}{$param}},$query_string); > undef $query_string ; > } > > > If I'm not mistaken, this is probably there to take care of the case > where the script is run from the command-line. On the plus side,
though, Show quoted text
> this seems to be the only place where read_from_stdin is used, so if
we Show quoted text
> go ahead with the change, at least the effects of it will be
contained. The read_from_stdin() method was only introduced in 3.44... in the patch you tracked down entitled "Patch from Kurt Jaeger to allow HTTP PUT even if the content length is unknown." I think the impact will be contained to changing the affect of that patch, which seems like it was essentially the source of this bug.
Giving to Yanick, who I think has a patch-in-progress for this. Mark
RT-Send-CC: yanick.champoux [...] gmail.com
Download (untitled) / with headers
text/plain 220b
Since It's been about a year, I think it's safe to assume that Yanick won't be getting to this. A patch from someone else is welcome. Ideally, it will be in the form of a github pull request that I can easily merge.
Download (untitled) / with headers
text/plain 388b
On Fri Nov 11 22:39:16 2011, MARKSTOS wrote: Show quoted text
> Since It's been about a year, I think it's safe to assume that Yanick > won't be getting to this.
Apologies for that. As usual, I've been pulled in many directions, and I'm afraid my CGI.pm patches fell on the wayside. I've dusted off that particular patch, and submitted a github pull off it: https://github.com/markstos/CGI.pm/pull/7
Subject: Re: [rt.cpan.org #52469] PUT method with empty body seems to freeze CGI->new()
Date: Tue, 15 Nov 2011 10:47:55 -0500
To: bug-cgi.pm [...] rt.cpan.org
From: Mark Stosberg <mark [...] summersault.com>
Download (untitled) / with headers
text/plain 153b
Show quoted text
> I've dusted off that particular patch, and submitted a github pull off it: > > https://github.com/markstos/CGI.pm/pull/7
Awesome. Thanks, Yanick.
Download (untitled) / with headers
text/plain 290b
I provided the patch to allow CGI.pm read from stdin even if no content-length-header was sent. There's a scanner available from AVISION which allows HTTP PUT to submit scanner input. The system does not provide a content-lenght-header. I'm trying to get them fix this, for a while now...
Subject: Re: [rt.cpan.org #52469] PUT method with empty body seems to freeze CGI->new()
Date: Tue, 27 Mar 2012 08:41:45 -0400
To: bug-cgi.pm [...] rt.cpan.org
From: Mark Stosberg <mark [...] summersault.com>
Thanks for the nudge on this ticket.
Download (untitled) / with headers
text/plain 240b
This issue has been copied to: https://github.com/leejo/CGI.pm/issues/63 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.
Download (untitled) / with headers
text/plain 308b
It seems this issue has been resolved by the original pull request merge above, d8277a5. The original patch to accept a PUT request with no content length didn't make sense and was to support a client not doing the right thing, the original changes have effectively been reverted fixing this issue. Closing.


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.