diff options
| -rw-r--r-- | src/HTTP/EnvelopeParser.cpp | 6 | ||||
| -rw-r--r-- | src/HTTP/HTTPRequestParser.h | 4 | ||||
| -rw-r--r-- | src/HTTP/HTTPResponseParser.cpp | 22 | ||||
| -rw-r--r-- | src/HTTP/HTTPResponseParser.h | 6 | ||||
| -rw-r--r-- | tests/HTTP/CMakeLists.txt | 9 | ||||
| -rw-r--r-- | tests/HTTP/HTTPResponseParser_file.cpp | 10 |
6 files changed, 35 insertions, 22 deletions
diff --git a/src/HTTP/EnvelopeParser.cpp b/src/HTTP/EnvelopeParser.cpp index 407e9dcfc..1c49b643f 100644 --- a/src/HTTP/EnvelopeParser.cpp +++ b/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: diff --git a/src/HTTP/HTTPRequestParser.h b/src/HTTP/HTTPRequestParser.h index f3d3add91..1b06d7b8b 100644 --- a/src/HTTP/HTTPRequestParser.h +++ b/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 */ diff --git a/src/HTTP/HTTPResponseParser.cpp b/src/HTTP/HTTPResponseParser.cpp index 9411208e2..5469666b6 100644 --- a/src/HTTP/HTTPResponseParser.cpp +++ b/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); } diff --git a/src/HTTP/HTTPResponseParser.h b/src/HTTP/HTTPResponseParser.h index 19652ccef..1d867ecc5 100644 --- a/src/HTTP/HTTPResponseParser.h +++ b/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. */ diff --git a/tests/HTTP/CMakeLists.txt b/tests/HTTP/CMakeLists.txt index f6989d831..c11c601d9 100644 --- a/tests/HTTP/CMakeLists.txt +++ b/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) diff --git a/tests/HTTP/HTTPResponseParser_file.cpp b/tests/HTTP/HTTPResponseParser_file.cpp index 48ff928bc..7e8d127b7 100644 --- a/tests/HTTP/HTTPResponseParser_file.cpp +++ b/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()) |