Try to handle incomplete HTTP. See #5401. Please test.

Approach: Keep filling the receive buffer until we have found enough data to extract the first HTTP header and body

2 files changed, 66 insertions, 65 deletions

diff --git a/src/common/xmpp/transports_nb.py b/src/common/xmpp/transports_nb.py
index e23348bba..aafc57dfb 100644
--- a/src/common/xmpp/transports_nb.py
+++ b/src/common/xmpp/transports_nb.py
@@ -645,44 +645,39 @@ class NonBlockingHTTP(NonBlockingTCP):
 		if self.get_state() == PROXY_CONNECTING:
 			NonBlockingTCP._on_receive(self, data)
 			return
-		if not self.recvbuff:
-			# recvbuff empty - fresh HTTP message was received
-			try:
-				statusline, headers, self.recvbuff = self.parse_http_message(data)
-			except ValueError:
-				self.disconnect()
-				return
-			if statusline[1] != '200':
-				log.error('HTTP Error: %s %s' % (statusline[1], statusline[2]))
-				self.disconnect()
-				return
-			self.expected_length = int(headers['Content-Length'])
-			if 'Connection' in headers and headers['Connection'].strip()=='close':
-				self.close_current_connection = True
-		else:
-			#sth in recvbuff - append currently received data to HTTP msg in buffer
-			self.recvbuff = '%s%s' % (self.recvbuff, data)
 
-		if self.expected_length > len(self.recvbuff):
+		# append currently received data to HTTP msg in buffer
+		self.recvbuff = '%s%s' % (self.recvbuff or '', data)
+		statusline, headers, httpbody, self.recvbuff = self.parse_http_message(self.recvbuff)
+		
+		if not (statusline and headers and httpbody):
+			log.debug('Received incomplete HTTP response')
+			return
+
+		if statusline[1] != '200':
+			log.error('HTTP Error: %s %s' % (statusline[1], statusline[2]))
+			self.disconnect()
+			return
+		self.expected_length = int(headers['Content-Length'])
+		if 'Connection' in headers and headers['Connection'].strip()=='close':
+			self.close_current_connection = True
+			
+		if self.expected_length > len(httpbody):
 			# If we haven't received the whole HTTP mess yet, let's end the thread.
 			# It will be finnished from one of following recvs on plugged socket.
 			log.info('not enough bytes in HTTP response - %d expected, %d got' %
 				(self.expected_length, len(self.recvbuff)))
-			return
-
-		# everything was received
-		httpbody = self.recvbuff
-
-		self.recvbuff = ''
-		self.expected_length = 0
+		else:
+			# everything was received
+			self.expected_length = 0
 
-		if not self.http_persistent or self.close_current_connection:
-			# not-persistent connections disconnect after response
-			self.disconnect(do_callback=False)
-		self.close_current_connection = False
-		self.last_recv_time = time.time()
-		self.on_receive(data=httpbody, socket=self)
-		self.on_http_request_possible()
+			if not self.http_persistent or self.close_current_connection:
+				# not-persistent connections disconnect after response
+				self.disconnect(do_callback=False)
+			self.close_current_connection = False
+			self.last_recv_time = time.time()
+			self.on_receive(data=httpbody, socket=self)
+			self.on_http_request_possible()
 
 	def build_http_message(self, httpbody, method='POST'):
 		'''
@@ -718,18 +713,24 @@ class NonBlockingHTTP(NonBlockingTCP):
 			httpbody - string with http body)
 		'''
 		message = message.replace('\r','')
-		# Remove latest \n
-		if message.endswith('\n'):
-			message = message[:-1]
-		(header, httpbody) = message.split('\n\n', 1)
-		header = header.split('\n')
-		statusline = header[0].split(' ', 2)
-		header = header[1:]
-		headers = {}
-		for dummy in header:
-			row = dummy.split(' ', 1)
-			headers[row[0][:-1]] = row[1]
-		return (statusline, headers, httpbody)
+		splitted = message.split('\n\n')
+		if len(splitted) < 2:
+			# no complete http message. Keep filling the buffer until we find one
+			buffer_rest = message
+			return ('', '', '', buffer_rest)
+		else:
+			(header, httpbody)  = splitted[:2]
+			if httpbody.endswith('\n'):
+				httpbody = httpbody[:-1]
+			buffer_rest = "\n\n".join(splitted[2:])
+			header = header.split('\n')
+			statusline = header[0].split(' ', 2)
+			header = header[1:]
+			headers = {}
+			for dummy in header:
+				row = dummy.split(' ', 1)
+				headers[row[0][:-1]] = row[1]
+			return (statusline, headers, httpbody, buffer_rest)
 
 
 class NonBlockingHTTPBOSH(NonBlockingHTTP):
diff --git a/test/integration/test_xmpp_transports_nb.py b/test/integration/test_xmpp_transports_nb.py
index f6cbab510..0e42ab855 100644
--- a/test/integration/test_xmpp_transports_nb.py
+++ b/test/integration/test_xmpp_transports_nb.py
@@ -227,9 +227,10 @@ class TestNonBlockingHTTP(AbstractTransportTest):
 
 		data = "<test>Please don't fail!</test>"
 		http_message = transport.build_http_message(data)
-		statusline, headers, http_body = transport.parse_http_message(
+		statusline, headers, http_body, buffer_rest = transport.parse_http_message(
 			http_message)
 
+		self.assertFalse(bool(buffer_rest))
 		self.assertTrue(statusline and isinstance(statusline, list))
 		self.assertTrue(headers and isinstance(headers, dict))
 		self.assertEqual(data, http_body, msg='Input and output are different')
@@ -250,26 +251,25 @@ class TestNonBlockingHTTP(AbstractTransportTest):
 		transport._on_receive(message)
 		self.assertTrue(self.have_received_expected(), msg='Failed: In one go')
 
-# FIXME: Not yet implemented.
-#	def test_receive_http_message_in_chunks(self):
-#		''' Let _on_receive handle some chunked http messages  '''
-#		transport = self._get_transport(self.bosh_http_dict)
-#		
-#		header = ("HTTP/1.1 200 OK\r\nContent-Type: text/xml; charset=utf-8\r\n" +
-#			"Content-Length: 88\r\n\r\n")
-#		payload = "<test>Please don't fail!</test>"
-#		body = "<body xmlns='http://jabber.org/protocol/httpbind'>%s</body>" \
-#			% payload
-#		message = "%s%s" % (header, body)
-#
-#		chunk1, chunk2, chunk3  = message[:20], message[20:73], message[73:]
-#		nextmessage_chunk = "\r\n\r\nHTTP/1.1 200 OK\r\nContent-Type: text/x"
-#		chunks = (chunk1, chunk2, chunk3, nextmessage_chunk)
-#
-#		transport.onreceive(self.expect_receive(body, msg='Failed: In chunks'))
-#		for chunk in chunks:
-#			transport._on_receive(chunk)
-#		self.assertTrue(self.have_received_expected(), msg='Failed: In chunks')
+	def test_receive_http_message_in_chunks(self):
+		''' Let _on_receive handle some chunked http messages  '''
+		transport = self._get_transport(self.bosh_http_dict)
+		
+		header = ("HTTP/1.1 200 OK\r\nContent-Type: text/xml; charset=utf-8\r\n" +
+			"Content-Length: 88\r\n\r\n")
+		payload = "<test>Please don't fail!</test>"
+		body = "<body xmlns='http://jabber.org/protocol/httpbind'>%s</body>" \
+			% payload
+		message = "%s%s" % (header, body)
+
+		chunk1, chunk2, chunk3  = message[:20], message[20:73], message[73:]
+		nextmessage_chunk = "\r\n\r\nHTTP/1.1 200 OK\r\nContent-Type: text/x"
+		chunks = (chunk1, chunk2, chunk3, nextmessage_chunk)
+
+		transport.onreceive(self.expect_receive(body, msg='Failed: In chunks'))
+		for chunk in chunks:
+			transport._on_receive(chunk)
+		self.assertTrue(self.have_received_expected(), msg='Failed: In chunks')
 
 if __name__ == '__main__':
 	unittest.main()