Bug #81237 for Net-HTTP: Long header(longer than 1024bytes) may cause timeout.

Fri Nov 16 12:52:44 2012 ishitoya [...] engraphia.com - Ticket created

Subject:

Long header(longer than 1024bytes) may cause timeout.

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.

Wed Dec 05 16:53:20 2012 ether [...] cpan.org - Cc ETHER added

Wed Dec 05 17:16:34 2012 rsrchboy [...] cpan.org - Cc RSRCHBOY added

Wed Dec 12 13:03:57 2012 ARODLAND [...] cpan.org - Cc ARODLAND added

Wed Jan 02 22:34:16 2013 DOY [...] cpan.org - Correspondence added

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.

Wed Jan 02 22:34:17 2013 The RT System itself - Status changed from 'new' to 'open'

Wed Jan 02 22:39:42 2013 DOY [...] cpan.org - Correspondence added

And, possible fix here https://github.com/gisle/net-http/pull/2 although I don't know if I re-broke other things.

Thu Jan 10 09:42:58 2013 jim.driscoll [...] heartinternet.co.uk - Correspondence added

From:

jim.driscoll [...] heartinternet.co.uk

On Wed Jan 02 22:39:42 2013, DOY wrote: Show quoted text

> And, possible fix here https://github.com/gisle/net-http/pull/2

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.

Fri Feb 08 18:02:48 2013 ether [...] cpan.org - Severity Normal changed to Important

Fri Feb 08 18:03:43 2013 ether [...] cpan.org - Correspondence added

RT-Send-CC:

jim.driscoll [...] heartinternet.co.uk

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?

Tue Feb 19 07:06:37 2013 chisel [...] chizography.net - Correspondence added 180 min

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.

Tue Feb 19 07:17:40 2013 chisel [...] chizography.net - Cc CHISEL added

Thu Mar 07 13:43:57 2013 GENEHACK [...] cpan.org - Correspondence added

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.)

Fri Mar 08 23:03:17 2013 AIMASS [...] cpan.org - Correspondence added

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

Sun Mar 10 18:51:33 2013 GAAS [...] cpan.org - Correspondence added

I've now applied the patch and uploaded Net-HTTP-6.06 to CPAN.

Sun Mar 10 18:51:34 2013 GAAS [...] cpan.org - Status changed from 'open' to 'resolved'