From 10e48212608f1fa75fed1dade9551408b30f484e Mon Sep 17 00:00:00 2001 From: Fabio Alessandrelli Date: Wed, 20 Feb 2019 03:06:02 +0100 Subject: [PATCH 1/2] HTTPClient read until EOF fixes --- core/io/http_client.cpp | 55 ++++++++++++----------------------------- 1 file changed, 16 insertions(+), 39 deletions(-) diff --git a/core/io/http_client.cpp b/core/io/http_client.cpp index b25b1934ff9..abf4dd200e6 100644 --- a/core/io/http_client.cpp +++ b/core/io/http_client.cpp @@ -504,6 +504,7 @@ PoolByteArray HTTPClient::read_response_body_chunk() { ERR_FAIL_COND_V(status != STATUS_BODY, PoolByteArray()); + PoolByteArray ret; Error err = OK; if (chunked) { @@ -524,7 +525,7 @@ PoolByteArray HTTPClient::read_response_body_chunk() { if (chunk.size() > 32) { ERR_PRINT("HTTP Invalid chunk hex len"); status = STATUS_CONNECTION_ERROR; - return PoolByteArray(); + break; } if (chunk.size() > 2 && chunk[chunk.size() - 2] == '\r' && chunk[chunk.size() - 1] == '\n') { @@ -542,14 +543,14 @@ PoolByteArray HTTPClient::read_response_body_chunk() { else { ERR_PRINT("HTTP Chunk len not in hex!!"); status = STATUS_CONNECTION_ERROR; - return PoolByteArray(); + break; } len <<= 4; len |= v; if (len > (1 << 24)) { ERR_PRINT("HTTP Chunk too big!! >16mb"); status = STATUS_CONNECTION_ERROR; - return PoolByteArray(); + break; } } @@ -557,7 +558,7 @@ PoolByteArray HTTPClient::read_response_body_chunk() { // End reached! status = STATUS_CONNECTED; chunk.clear(); - return PoolByteArray(); + break; } chunk_left = len + 2; @@ -577,18 +578,13 @@ PoolByteArray HTTPClient::read_response_body_chunk() { if (chunk[chunk.size() - 2] != '\r' || chunk[chunk.size() - 1] != '\n') { ERR_PRINT("HTTP Invalid chunk terminator (not \\r\\n)"); status = STATUS_CONNECTION_ERROR; - return PoolByteArray(); + break; } - PoolByteArray ret; ret.resize(chunk.size() - 2); - { - PoolByteArray::Write w = ret.write(); - copymem(w.ptr(), chunk.ptr(), chunk.size() - 2); - } + PoolByteArray::Write w = ret.write(); + copymem(w.ptr(), chunk.ptr(), chunk.size() - 2); chunk.clear(); - - return ret; } break; @@ -598,45 +594,26 @@ PoolByteArray HTTPClient::read_response_body_chunk() { } else { int to_read = !read_until_eof ? MIN(body_left, read_chunk_size) : read_chunk_size; - PoolByteArray ret; ret.resize(to_read); int _offset = 0; - while (read_until_eof || to_read > 0) { + while (to_read > 0) { int rec = 0; { PoolByteArray::Write w = ret.write(); err = _get_http_data(w.ptr() + _offset, to_read, rec); } - if (rec < 0) { - if (to_read > 0) // Ended up reading less - ret.resize(_offset); + if (rec <= 0) { // Ended up reading less + ret.resize(_offset); break; } else { _offset += rec; + to_read -= rec; if (!read_until_eof) { body_left -= rec; - to_read -= rec; - } else { - if (rec < to_read) { - ret.resize(_offset); - err = ERR_FILE_EOF; - break; - } - ret.resize(_offset + to_read); } } - } - if (!read_until_eof) { - if (body_left == 0) { - status = STATUS_CONNECTED; - } - return ret; - } else { - if (err == ERR_FILE_EOF) { - err = OK; // EOF is expected here - close(); - return ret; - } + if (err != OK) + break; } } @@ -651,12 +628,12 @@ PoolByteArray HTTPClient::read_response_body_chunk() { status = STATUS_CONNECTION_ERROR; } - } else if (body_left == 0 && !chunked) { + } else if (body_left == 0 && !chunked && !read_until_eof) { status = STATUS_CONNECTED; } - return PoolByteArray(); + return ret; } HTTPClient::Status HTTPClient::get_status() const { From 0e5655694c844f93e1fdd044d873c2661cb00742 Mon Sep 17 00:00:00 2001 From: Fabio Alessandrelli Date: Wed, 20 Feb 2019 04:33:31 +0100 Subject: [PATCH 2/2] Fix HTTPClient keep alive with chunked encoding. We need to consume the trailer part and final CRLF after last chunk as per RFC 7230 section 4.1: ``` chunked-body = *chunk last-chunk trailer-part CRLF ``` We do not return the trailer part, just consume it allowing following requests to work as expected when using keep alive. --- core/io/http_client.cpp | 30 ++++++++++++++++++++++++++++-- core/io/http_client.h | 1 + 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/core/io/http_client.cpp b/core/io/http_client.cpp index abf4dd200e6..e5c6d2a4f29 100644 --- a/core/io/http_client.cpp +++ b/core/io/http_client.cpp @@ -278,6 +278,7 @@ void HTTPClient::close() { body_size = -1; body_left = 0; chunk_left = 0; + chunk_trailer_part = 0; read_until_eof = false; response_num = 0; handshaking = false; @@ -421,6 +422,7 @@ Error HTTPClient::poll() { chunked = false; body_left = 0; chunk_left = 0; + chunk_trailer_part = false; read_until_eof = false; response_str.clear(); response_headers.clear(); @@ -511,7 +513,30 @@ PoolByteArray HTTPClient::read_response_body_chunk() { while (true) { - if (chunk_left == 0) { + if (chunk_trailer_part) { + // We need to consume the trailer part too or keep-alive will break + uint8_t b; + int rec = 0; + err = _get_http_data(&b, 1, rec); + + if (rec == 0) + break; + + chunk.push_back(b); + int cs = chunk.size(); + if ((cs >= 2 && chunk[cs - 2] == '\r' && chunk[cs - 1] == '\n')) { + if (cs == 2) { + // Finally over + chunk_trailer_part = false; + status = STATUS_CONNECTED; + chunk.clear(); + break; + } else { + // We do not process nor return the trailer data + chunk.clear(); + } + } + } else if (chunk_left == 0) { // Reading length uint8_t b; int rec = 0; @@ -556,7 +581,7 @@ PoolByteArray HTTPClient::read_response_body_chunk() { if (len == 0) { // End reached! - status = STATUS_CONNECTED; + chunk_trailer_part = true; chunk.clear(); break; } @@ -695,6 +720,7 @@ HTTPClient::HTTPClient() { body_left = 0; read_until_eof = false; chunk_left = 0; + chunk_trailer_part = false; response_num = 0; ssl = false; blocking = false; diff --git a/core/io/http_client.h b/core/io/http_client.h index 1ad44d5f018..85ee1959a22 100644 --- a/core/io/http_client.h +++ b/core/io/http_client.h @@ -172,6 +172,7 @@ private: bool chunked; Vector chunk; int chunk_left; + bool chunk_trailer_part; int body_size; int body_left; bool read_until_eof;