Skip Menu |
 

This queue is for tickets about the Net-HTTP CPAN distribution.

Report information
The Basics
Id: 81237
Status: resolved
Worked: 3 hours (180 min)
Priority: 0/
Queue: Net-HTTP

People
Owner: Nobody in particular
Requestors: ishitoya [...] engraphia.com
Cc: ARODLAND [...] cpan.org
chisel [...] chizography.net
ether [...] cpan.org
rsrchboy [...] cpan.org
AdminCc:

Bug Information
Severity: Important
Broken in: 6.04
Fixed in: (no value)



Subject: Long header(longer than 1024bytes) may cause timeout.
Download (untitled) / with headers
text/plain 2.3k
Hi, I asked question on stackoverflow, http://stackoverflow.com/questions/13421307/oauthliteconsumer-timeouts-when-i-access-to-twitter Environment is Net::HTTP 6.05, perl 5.16.2, OAuth::Lite 1.31. Server is CentOS release 6.3 (Final), 2.6.32-279.14.1.el6.x86_64. Minimum code to reproduce the bug is below, use OAuth::Lite::Consumer; my $consumer = OAuth::Lite::Consumer->new( request_token_path => 'https://api.twitter.com/oauth/request_token', consumer_key => '[CONSUMER KEY]', consumer_secret => '[CONSUMER SECRET]', callback_url => '[CALLBACK]', ); my $rtoken = $consumer->get_request_token(); will return after 30sec. I read the code, and I figure out that method my_readline in Net::HTTP::Methods line 259-260 in Net::HTTP 6.05 die "read timeout" unless $self->can_read; my $n = $self->sysread($_, 1024, length); blocks request. And the header that my_readline trying to read is HTTP/1.1 200 OK Date: Fri, 16 Nov 2012 16:16:39 GMT Status: 200 OK X-MID: 266cca0268c8eb68e76eedda256c77829ea9b22e X-Transaction: 784ff126dfead993 Tag: "382e5356bb65e976f6b117bacdfa05e4" Cache-Control: no-cache, no-store, must-revalidate, pre-check=0, post-check=0 X-Runtime: 0.02892 Pragma: no-cache Expires: Tue, 31 Mar 1981 05:00:00 GMT X-Frame-Options: SAMEORIGIN Content-Type: text/html; charset=utf-8 Last-Modified: Fri, 16 Nov 2012 16:16:39 GMT Set-Cookie: k=10.36.55.109.1353082599368934; path=/; expires=Fri, 23-Nov-12 16:16:39 GMT; domain=.twitter.com Set-Cookie: guest_id=v1%3A135308259937793094; domain=.twitter.com; path=/; expires=Mon, 17-Nov-2014 04:16:39 GMT Set-Cookie: _twitter_sess=BAh7CCIKZmxhc2hJQzonQWN0aW9uQ29udHJvbGxlcjo6Rmxhc2g6OkZsYXNo%250ASGFzaHsABjoKQHVzZWR7ADoHaWQiJTJlZGJiN2MyOTMyNmM0OGY0MjJmZmY0%250AMTU3YzllYTI0Og9jcmVhdGVkX2F0bCsI0vcBCjsB--21ab24a6dd6596b1dcc78651c0b6a4836b320621; domain=.twitter.com; path=/; HttpOnly Vary: Accept-Encoding Content-Encoding: gzip Content-Length: 150 Server: tfe This header exceeded 1024 characters. When I downgrade Net-HTTP to 6.03, it works fine. Thanks.
Download (untitled) / with headers
text/plain 1019b
So the problem here (I think) is that IO::Socket::SSL doesn't actually support blocking sockets - everything is really non-blocking and buffered underneath. So the first time something is read, the select call (in can_read) checks the socket, sees that there is data available, and returns true, and so the read completes. That read doesn't just read 1024 bytes though, it actually fills up the IO::Socket::SSL internal buffers with however much data is available. Then, the next time that my_read or my_readline is called, calling select() will fail (and thus hang), because the socket itself doesn't have any more data available - the data is all in the buffers by this point. Unfortunately, I'm not really sure what the solution here is. There's no real way to override select to do the right thing here. IO::Socket::SSL does provide a 'pending' method to see if there is data ready to be read, so something like "return 1 if $self->isa('IO::Socket::SSL') && $self->pending" might be the best option, ugly as it is.
Download (untitled) / with headers
text/plain 113b
And, possible fix here https://github.com/gisle/net-http/pull/2 although I don't know if I re-broke other things.
From: jim.driscoll [...] heartinternet.co.uk
Download (untitled) / with headers
text/plain 899b
On Wed Jan 02 22:39:42 2013, DOY wrote: Show quoted text
although Show quoted text
> I don't know if I re-broke other things.
Given the "loop until ready to read or timeout" existing behaviour, you want something like (perl >= 5.004): --- /usr/local/share/perl5/Net/HTTP/Methods.pm 2012-11-10 15:09:51.000000000 +0000 +++ Net/HTTP/Methods.pm 2013-01-10 14:36:26.387400090 +0000 @@ -288,6 +288,7 @@ # used to just check if the socket is readable without waiting. my $timeout = @_ ? shift : (${*$self}{io_socket_timeout} || undef); + return 1 if $self->can("pending") and $self->pending; my $fbits = ''; vec($fbits, fileno($self), 1) = 1; SELECT: So if there is a pending method and it returns true, there's data ready which may or may not show up via select(); otherwise, it's reasonable to wait until select() shows some content.
RT-Send-CC: jim.driscoll [...] heartinternet.co.uk
Download (untitled) / with headers
text/plain 200b
I'm bumping up the priority on this ticket as it is being talked about in irc now as a reason to *not* upgrade Net::HTTP to the latest version. What do we need to figure out before this can be fixed?
Download (untitled) / with headers
text/plain 373b
Just to confirm that this is happening for us (Net-A-Porter). When we upgraded to 6.05 we started seeing: * the test script visibly take longer to complete * the XML parsing failed - due to the xml response appearing to be incomplete The XML was the response of a call to an external API. The proposed one-line fix above does appear to resolve the issue we're seeing.
Download (untitled) / with headers
text/plain 997b
This bug has been open since November of 2012. There's been a fix for this bug attached to this ticket since 02 January 2013 (and submitted as a Github pull request) - https://github.com/gisle/net-http/pull/2 That *simple one line fix* was essentially duplicated, apparently independently, by a second person, a week later. This bug breaks, essentially, the Internet as far as Perl code is concerned, and does so in an irritatingly non-obvious way. I know, because I traced it out in November, and then, having forgotten about it, traced it out again in January. I have production code with hard-coded version checks for Net::HTTP because of this bug. This is a pretty sad state for the "duct tape of the internet", IMNSAndPrettyAnnoyedO. What do we have to do to get this *one line change* merged and a new release cut? (FWIW, I prefer the first patches' ->isa() check over the second's ->can() check but at this point I can't really be bothered, I just want something that works.)
Download (untitled) / with headers
text/plain 439b
On Thu Mar 07 13:43:57 2013, GENEHACK wrote: Show quoted text
> This bug has been open since November of 2012. > > There's been a fix for this bug attached to this ticket since 02 > January 2013 (and submitted as
I conform the existence of the bug today (March 8th 2013 with fresh v6.05 from CPAN) and the one liner here fixes the problem at least for my implementation: https://github.com/doy/net-http/commit/744fb936829d27c7bf0d95167aa5d89c9afd5dc9
 I've now applied the patch and uploaded Net-HTTP-6.06 to CPAN.


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.