HTTP: Fixed response parser, unified API.

src/HTTP/EnvelopeParser.cpp

@@ -28,12 +28,12 @@ size_t cEnvelopeParser::Parse(const char * a_Data, size_t a_Size)
 	}
 
 	// Start searching 1 char from the end of the already received data, if available:
-	size_t SearchStart = m_IncomingData.size();
-	SearchStart = (SearchStart > 1) ? SearchStart - 1 : 0;
+	auto searchStart = m_IncomingData.size();
+	searchStart = (searchStart > 1) ? searchStart - 1 : 0;
 
 	m_IncomingData.append(a_Data, a_Size);
 
-	size_t idxCRLF = m_IncomingData.find("\r\n", SearchStart);
+	size_t idxCRLF = m_IncomingData.find("\r\n", searchStart);
 	if (idxCRLF == AString::npos)
 	{
 		// Not a complete line yet, all input consumed:

src/HTTP/HTTPRequestParser.h

@@ -25,8 +25,8 @@ public:
 	cHTTPRequestParser(void);
 	
 	/** Parses the request line and then headers from the received data.
-	Returns the number of bytes consumed or AString::npos number for error
-	*/
+	Returns the number of bytes consumed or AString::npos number for error.
+	Once it has fully parsed all the headers, doesn't consume any more data. */
 	size_t ParseHeaders(const char * a_Data, size_t a_Size);
 	
 	/** Returns true if the request did contain a Content-Length header */

src/HTTP/HTTPResponseParser.cpp

@@ -29,17 +29,18 @@ size_t cHTTPResponseParser::Parse(const char * a_Data, size_t a_Size)
 	// If parsing already finished or errorred, let the caller keep all the data:
 	if (m_IsFinished || m_HasHadError)
 	{
-		return a_Size;
+		return 0;
 	}
 
 	// If still waiting for the status line, add to buffer and try parsing it:
+	auto inBufferSoFar = m_Buffer.size();
 	if (m_StatusLine.empty())
 	{
 		m_Buffer.append(a_Data, a_Size);
 		if (!ParseStatusLine())
 		{
 			// All data used, but not a complete status line yet.
-			return 0;
+			return a_Size;
 		}
 		if (m_HasHadError)
 		{
@@ -53,14 +54,20 @@ size_t cHTTPResponseParser::Parse(const char * a_Data, size_t a_Size)
 			m_Callbacks.OnError("Failed to parse the envelope");
 			return AString::npos;
 		}
+		ASSERT(bytesConsumed < inBufferSoFar + a_Size);
 		m_Buffer.erase(0, bytesConsumed);
 		if (!m_Buffer.empty())
 		{
 			// Headers finished and there's still data left in the buffer, process it as message body:
 			HeadersFinished();
-			return ParseBody(m_Buffer.data(), m_Buffer.size());
+			auto res = ParseBody(m_Buffer.data(), m_Buffer.size());
+			if (res == AString::npos)
+			{
+				return AString::npos;
+			}
+			return res + bytesConsumed - inBufferSoFar;
 		}
-		return 0;
+		return a_Size;
 	}  // if (m_StatusLine.empty())
 
 	// If still parsing headers, send them to the envelope parser:
@@ -77,9 +84,9 @@ size_t cHTTPResponseParser::Parse(const char * a_Data, size_t a_Size)
 		{
 			// Headers finished and there's still data left in the buffer, process it as message body:
 			HeadersFinished();
-			return ParseBody(a_Data + bytesConsumed, a_Size - bytesConsumed);
+			return bytesConsumed + ParseBody(a_Data + bytesConsumed, a_Size - bytesConsumed);
 		}
-		return 0;
+		return a_Size;
 	}
 
 	// Already parsing the body
@@ -116,7 +123,8 @@ size_t cHTTPResponseParser::ParseBody(const char * a_Data, size_t a_Size)
 	}
 
 	// Parse the body using the transfer encoding parser:
-	return m_TransferEncodingParser->Parse(a_Data, a_Size);
+	// (Note that TE parser returns the number of bytes left, while we return the number of bytes consumed)
+	return a_Size - m_TransferEncodingParser->Parse(a_Data, a_Size);
 }
 
 

src/HTTP/HTTPResponseParser.h

@@ -51,8 +51,7 @@ public:
 	cHTTPResponseParser(cCallbacks & a_Callbacks);
 
 	/** Parses the incoming data and calls the appropriate callbacks.
-	Returns the number of bytes from the end of a_Data that is already not part of this response.
-	Returns AString::npos on an error. */
+	Returns the number of bytes consumed or AString::npos number for error. */
 	size_t Parse(const char * a_Data, size_t a_Size);
 
 	/** Called when the server indicates no more data will be sent (HTTP 1.0 socket closed).
@@ -97,8 +96,7 @@ protected:
 
 	/** Parses the message body.
 	Processes transfer encoding and calls the callbacks for body data.
-	Returns the number of bytes from the end of a_Data that is already not part of this response.
-	Returns AString::npos on error. */
+	Returns the number of bytes consumed or AString::npos number for error. */
 	size_t ParseBody(const char * a_Data, size_t a_Size);
 
 	/** Called internally when the headers-parsing has just finished. */

tests/HTTP/CMakeLists.txt

@@ -42,5 +42,12 @@ endif()
 # HTTPResponseParser_file: Feed file contents into a cHTTPResponseParser and print the callbacks as they're called:
 add_executable(HTTPResponseParser_file-exe HTTPResponseParser_file.cpp)
 target_link_libraries(HTTPResponseParser_file-exe HTTP)
-add_test(NAME HTTPResponseParser_file-test1 COMMAND HTTPResponseParser_file-exe HTTPResponse1.data)
+
+# Test parsing the file in 2-byte chunks (should go from response line parsing through headers parsing to body parsing, each within a different step):
+add_test(NAME HTTPResponseParser_file-test1-2 COMMAND HTTPResponseParser_file-exe HTTPResponse1.data 2)
+
+# Test parsing the file in 128-byte chunks (should parse response line and part of headers in one step, the rest in another step):
+add_test(NAME HTTPResponseParser_file-test1-128 COMMAND HTTPResponseParser_file-exe HTTPResponse1.data 128)
+
+# Test parsing a chunked-encoding content:
 add_test(NAME HTTPResponseParser_file-test2 COMMAND HTTPResponseParser_file-exe HTTPResponse2.data)

tests/HTTP/HTTPResponseParser_file.cpp

@@ -122,16 +122,16 @@ int main(int argc, char * argv[])
 			printf("Read 0 bytes from file (EOF?), terminating\n");
 			break;
 		}
-		auto numLeft = parser.Parse(buf, numBytes);
-		if (numLeft == AString::npos)
+		auto numConsumed = parser.Parse(buf, numBytes);
+		if (numConsumed == AString::npos)
 		{
 			printf("Parser indicates there was an error, terminating parsing.\n");
 			break;
 		}
-		ASSERT(numLeft <= numBytes);
-		if (numLeft > 0)
+		ASSERT(numConsumed <= numBytes);
+		if (numConsumed < numBytes)
 		{
-			printf("Parser indicates stream end, but there's more data (at least %u bytes) in the file.\n", static_cast<unsigned>(numLeft));
+			printf("Parser indicates stream end, but there's more data (at least %u bytes) in the file.\n", static_cast<unsigned>(numBytes - numConsumed));
 		}
 	}
 	if (!parser.IsFinished())