Overview
Examples
Screenshots
Comparisons
Applications
Download
Documentation
Tutorials
Bazaar
Status & Roadmap
FAQ
Authors & License
Forums
Funding Ultimate++
Search on this site
Search in forums












SourceForge.net Logo
Home » U++ Library support » U++ MT-multithreading and servers » [FIXED]HttpRequest hangs when Chunked response, without trailer, and KeepAlive is set. (patch & TC)
[FIXED]HttpRequest hangs when Chunked response, without trailer, and KeepAlive is set. (patch & TC) [message #46292] Fri, 08 April 2016 19:44 Go to next message
omari is currently offline  omari
Messages: 213
Registered: March 2010
Experienced Member
Hi,

When HttpRequest is in TRAILER phase, he call ReadingHeader().

buf if there is no trailer, the socket contain only two charachter: \r\n.
because ReadingHeader() read at least three characters, il hangs waiting a third one.

Here a Test Case:
	HttpRequest r("http://dev.alt.cloudappsportal.com/_api/web/lists");
	r.KeepAlive().Execute();


A solution is to check if there is a trailer, and if not, read only \r\n and finish:

in the file Core\Http.cpp
Line : 385
replace :
	case TRAILER:		
		if(ReadingHeader())
			break;
		header.ParseAdd(data);
		Finish();
		break;
	case FINISHED:


by
	case TRAILER:
		if(TcpSocket::Peek() == '\r') // if the next line is empty (then no trailer)
		{
			TcpSocket::Get();
			c2 = TcpSocket::Get();
			if(c2 != '\n')
				HttpError("missing ending CRLF");
			Finish();
			break;
		}
		
		if(ReadingHeader())
			break;
		header.ParseAdd(data);
		Finish();
		break;
	case FINISHED:


regards
omari.

[Updated on: Sat, 18 March 2017 16:01]

Report message to a moderator

Re: [BUG]HttpRequest hangs when Chunked response, without trailer, and KeepAlive is set. (patch & TC) [message #46313 is a reply to message #46292] Sun, 17 April 2016 08:58 Go to previous messageGo to next message
mirek is currently offline  mirek
Messages: 12337
Registered: November 2005
Ultimate Member
Thanks. In the end, I have used a little bit more complete fix (appears that using ReadingHeader was wrong, so added new ReadingTrailer function sintead).

Should be now fixed. Please check.

Mirek
Re: [BUG]HttpRequest hangs when Chunked response, without trailer, and KeepAlive is set. (patch & TC) [message #46314 is a reply to message #46313] Sun, 17 April 2016 11:37 Go to previous messageGo to next message
omari is currently offline  omari
Messages: 213
Registered: March 2010
Experienced Member
Thanks Mirek,

it missing a call to Finish() when ReadingTrailer() returns true.

for example :
 	case TRAILER:
		if(!ReadingTrailer())
 			header.ParseAdd(data);
 		Finish();
 		break;

instead of
 	case TRAILER:
		if(ReadingTrailer())
 			break;
 		header.ParseAdd(data);
 		Finish();
 		break;


regards
omari.
Re: [BUG]HttpRequest hangs when Chunked response, without trailer, and KeepAlive is set. (patch & TC) [message #46316 is a reply to message #46314] Sun, 17 April 2016 14:43 Go to previous messageGo to next message
mirek is currently offline  mirek
Messages: 12337
Registered: November 2005
Ultimate Member
I am not sure that what you say is correct...

Finish sets "http request finished" state. ReadingTrailer returns true if trailer is not yet read (or error, which is handled elsewhere) - basically it means ReadingTrailer should be called again on next Do (or error handled).

Mirek
Re: [BUG]HttpRequest hangs when Chunked response, without trailer, and KeepAlive is set. (patch & TC) [message #46318 is a reply to message #46316] Sun, 17 April 2016 22:10 Go to previous messageGo to next message
omari is currently offline  omari
Messages: 213
Registered: March 2010
Experienced Member
the trailer part of http response is optional.

an empty line("\r\n") after the last chunk, mean that there is no trailer at all.


in this case, we should go to the next phase : Finish().


regards
omari.

[Updated on: Sun, 17 April 2016 22:12]

Report message to a moderator

Re: [BUG]HttpRequest hangs when Chunked response, without trailer, and KeepAlive is set. (patch & TC) [message #46320 is a reply to message #46318] Mon, 18 April 2016 17:44 Go to previous messageGo to next message
mirek is currently offline  mirek
Messages: 12337
Registered: November 2005
Ultimate Member
omari wrote on Sun, 17 April 2016 22:10
the trailer part of http response is optional.

an empty line("\r\n") after the last chunk, mean that there is no trailer at all.


in this case, we should go to the next phase : Finish().


bool HttpRequest::ReadingTrailer()
{
	for(;;) {
		int c = TcpSocket::Get();
		if(c < 0)
			return !IsEof();


If there is no trailer (or less than 2 characters at the end of stream), it should return false, right?

I believe that calling Finish in all cases is simply wrong - if there is not trailer, it is possible it does not get read. Now obviously, it perhaps does not matter much, unless you are in KeepAlive mode, where you IMO really need trailer to tell the end of each HTTP request.

Mirek
Re: [BUG]HttpRequest hangs when Chunked response, without trailer, and KeepAlive is set. (patch & TC) [message #46321 is a reply to message #46320] Mon, 18 April 2016 19:02 Go to previous messageGo to next message
omari is currently offline  omari
Messages: 213
Registered: March 2010
Experienced Member
yes i am in KeepAlive mode.

in this case, TcpSocket::Get() does not return (-1), but it block.




regards
omari.
Re: [BUG]HttpRequest hangs when Chunked response, without trailer, and KeepAlive is set. (patch & TC) [message #46323 is a reply to message #46321] Tue, 19 April 2016 11:01 Go to previous messageGo to next message
mirek is currently offline  mirek
Messages: 12337
Registered: November 2005
Ultimate Member
Thanks. You were (mostly) right all the time, I was wrong. I got messed up the trailer and the empty line.

Anyway, deeper digging into HTTP specs revealed that in theory, (main) HTTP header can be empty just as well as chunked trailer. So perhaps the really correct fix should be in ReadingHeader:

bool HttpRequest::ReadingHeader()
{
	for(;;) {
		int c = TcpSocket::Get();
		if(c < 0)
			return !IsEof();
		else
			data.Cat(c);
		if(data.GetCount() == 2 && data[0] == '\r' && data[1] == '\n') // header is empty
			return false;
		if(data.GetCount() >= 3) {
			const char *h = data.Last();
			if(h[0] == '\n' && h[-1] == '\r' && h[-2] == '\n') // empty ending line after non-empty header
				return false;
		}
		if(data.GetCount() > max_header_size) {
			HttpError("HTTP header exceeded " + AsString(max_header_size));
			return true;
		}
	}
}


(now on svn...)

Do you think this is correct?
Re: [BUG]HttpRequest hangs when Chunked response, without trailer, and KeepAlive is set. (patch & TC) [message #46324 is a reply to message #46292] Tue, 19 April 2016 11:36 Go to previous messageGo to next message
omari is currently offline  omari
Messages: 213
Registered: March 2010
Experienced Member
Mirek, you are right,

when ReadingTrailer return true, we do not call Finish().


but the actual implementation of ReadingTrailer fail when a Trailer is present.

it miss the part :

	if(data.GetCount() > 3) {
			const char *h = data.Last();
			if(h[0] == '\n' && (h[-1] == '\r' && h[-2] == '\n' || h[-1] == '\n'))
				return false;
		}

(as ReadingHeader)

bool HttpRequest::ReadingTrailer()
{
	for(;;) {
		int c = TcpSocket::Get();
		if(c < 0)
			return !IsEof();
		else
			data.Cat(c);
		if(data.GetCount() == 2) {
			if(data[0] == '\r' && data[1] == '\n')
				return false;
		}
		if(data.GetCount() > 3) {
			const char *h = data.Last();
			if(h[0] == '\n' && (h[-1] == '\r' && h[-2] == '\n' || h[-1] == '\n'))
				return false;
		}
	}
}




regards
omari.

[Updated on: Tue, 19 April 2016 11:36]

Report message to a moderator

Re: [BUG]HttpRequest hangs when Chunked response, without trailer, and KeepAlive is set. (patch & TC) [message #46325 is a reply to message #46292] Tue, 19 April 2016 11:42 Go to previous messageGo to next message
omari is currently offline  omari
Messages: 213
Registered: March 2010
Experienced Member
here a test case for chunker response with and without trailer:

#include <CtrlLib/CtrlLib.h>

using namespace Upp;

// base source : http://www.tcpipguide.com/free/t_HTTPDataLengthIssuesChunkedTransfersandMessageTrai-3.htm
String chunked_with_trailer = "HTTP/1.1 200 OK\r\nDate: Mon, 22 Mar 2004 11:15:03 GMT\r\nContent-Type: text/html\r\nTransfer-Encoding: chunked\r\nTrailer: Expires\r\n\r\n29\r\n<html><body><p>The file you requested is \r\n5\r\n3,400\r\n23\r\nbytes long and was last modified:\r\n\r\n1d\r\nSat, 20 Mar 2004 21:12:00 GMT\r\n13\r\n.</p></body></html>\r\n0\r\nExpires: Sat, 27 Mar 2004 21:12:00 GMT\r\n\r\n";

// base source : https://en.wikipedia.org/wiki/Chunked_transfer_encoding?oldid=430331176
String chunked_without_trailer = "HTTP/1.1 200 OK\r\nContent-Type: text/plain\r\nTransfer-Encoding: chunked\r\n\r\n26\r\nThis is the data in the first chunk\r\n6\r\n1C\r\nand this is the second one\r\n\r\n3\r\ncon\r\n8\r\nsequence\r\n0\r\n\r\n";

static void Server(String r)
{
	TcpSocket   server;
	if(server.Listen(4000, 10)) {

		TcpSocket socket;
		LOG("Waiting...");
		bool b = socket.Accept(server);
		if(b) {
			LOG("Connection accepted");
			HttpHeader http;
			http.Read(socket);
			socket.Put(r);
			socket.Close();
		}
	}
}


GUI_APP_MAIN
{
	StdLogSetup(LOG_COUT|LOG_FILE);
	Thread a;

	LOG("chunked_without_trailer");
	LOG("****************");
	LOG(chunked_without_trailer);
	LOG("---------------");

	a.Run(callback1(Server, chunked_without_trailer));
	HttpRequest r1("localhost:4000");/*r1.Trace();*/	LOG(r1.GET().Execute());
	a.Wait();
	
	
	LOG("chunked_with_trailer");
	LOG("****************");
	LOG(chunked_with_trailer);
	LOG("---------------");

	a.Run(callback1(Server, chunked_with_trailer));
	HttpRequest r2("localhost:4000");/*r2.Trace();*/	LOG(r2.GET().Execute());
	a.Wait();

	LOG("=============== OK");
	
}



regards
omari.
Re: [BUG]HttpRequest hangs when Chunked response, without trailer, and KeepAlive is set. (patch & TC) [message #46332 is a reply to message #46324] Fri, 22 April 2016 09:33 Go to previous messageGo to next message
mirek is currently offline  mirek
Messages: 12337
Registered: November 2005
Ultimate Member
omari wrote on Tue, 19 April 2016 11:36
Mirek, you are right,

when ReadingTrailer return true, we do not call Finish().


but the actual implementation of ReadingTrailer fail when a Trailer is present.

it miss the part :

	if(data.GetCount() > 3) {
			const char *h = data.Last();
			if(h[0] == '\n' && (h[-1] == '\r' && h[-2] == '\n' || h[-1] == '\n'))
				return false;
		}

(as ReadingHeader)

bool HttpRequest::ReadingTrailer()
{
	for(;;) {
		int c = TcpSocket::Get();
		if(c < 0)
			return !IsEof();
		else
			data.Cat(c);
		if(data.GetCount() == 2) {
			if(data[0] == '\r' && data[1] == '\n')
				return false;
		}
		if(data.GetCount() > 3) {
			const char *h = data.Last();
			if(h[0] == '\n' && (h[-1] == '\r' && h[-2] == '\n' || h[-1] == '\n'))
				return false;
		}
	}
}




I suspect you were not testing with latest svn - there is no ReadingTrailer anymore...

I have tested with your testcase and it works as expected (great testcase BTW! adding it to autotest).

Mirek
Re: [BUG]HttpRequest hangs when Chunked response, without trailer, and KeepAlive is set. (patch & TC) [message #46335 is a reply to message #46332] Fri, 22 April 2016 17:08 Go to previous messageGo to next message
omari is currently offline  omari
Messages: 213
Registered: March 2010
Experienced Member
Thanks Mirek, it work well for this scenario.

there are two other small bugs that appears with KeepAlive:

1 - Finish() calls Close() (at line 833), but in keep_alive mode, it should not
	if(!keep_alive)Close();


2 - there is a blocking, when response has Content-Length = 0, for example:
HTTP/1.0 200 OK
Date: Fri, 31 Dec 1999 23:59:59 GMT
Server: Apache/0.8.4
Content-Type: text/html
Content-Length: 0
Expires: Sat, 01 Jan 2000 00:59:59 GMT
Last-modified: Fri, 09 Aug 1996 14:21:40 GMT




a possible solution (tested) is in ReadingBody()
replace:
	if(count > 0)
		n = (int)min((int64)n, count);


by
	if(count == 0)
		return false;
	n = (int)min((int64)n, count);


regards
omari.
Re: [BUG]HttpRequest hangs when Chunked response, without trailer, and KeepAlive is set. (patch & TC) [message #46348 is a reply to message #46335] Mon, 25 April 2016 13:52 Go to previous messageGo to next message
mirek is currently offline  mirek
Messages: 12337
Registered: November 2005
Ultimate Member
Thanks. I have checked the code and this simple version should work:

bool HttpRequest::ReadingBody()
{
	LLOG("HTTP reading body " << count);
	String s = TcpSocket::Get((int)min((int64)chunk, count));
	if(s.GetCount() == 0)
		return !IsEof() && count;



BTW, if you have some fixed testing server that returns zero content, please let me know - I would like to ass it to nightly tests.
Re: [BUG]HttpRequest hangs when Chunked response, without trailer, and KeepAlive is set. (patch & TC) [message #46349 is a reply to message #46348] Mon, 25 April 2016 13:57 Go to previous message
mirek is currently offline  mirek
Messages: 12337
Registered: November 2005
Ultimate Member
Ah, here we go:

https://httpbin.org/bytes/0
Previous Topic: HttpRequest : Add custon authentication capability
Next Topic: NTLM Authentication for HttpRequest
Goto Forum:
  


Current Time: Thu Apr 09 00:11:05 CEST 2020

Total time taken to generate the page: 0.02561 seconds