From 7a1a68883546bbfa0b24fc65054b0a7bf4f09b66 Mon Sep 17 00:00:00 2001
From: David Steele <david@pgmasters.net>
Date: Sat, 24 Nov 2018 09:12:44 -0500
Subject: [PATCH] Add EOF detection to content read in HttpClient.

If the connection closed before all content was sent httpClientRead() would get stuck in an infinite loop waiting for it to arrive.

EOF should never be reached during content read so immediately error if EOF is detected.
---
 src/common/io/http/client.c         |  6 ++-
 test/src/module/common/ioHttpTest.c | 57 +++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/src/common/io/http/client.c b/src/common/io/http/client.c
index d6bae8e68..47e308f8c 100644
--- a/src/common/io/http/client.c
+++ b/src/common/io/http/client.c
@@ -96,7 +96,7 @@ httpClientRead(HttpClient *this, Buffer *buffer, bool block)
             // Read if there is content remaining
             if (this->contentRemaining > 0)
             {
-                // If the buffer is larger than the content that needs to read then limit the buffer size so the read won't block
+                // If the buffer is larger than the content that needs to be read then limit the buffer size so the read won't block
                 // or read too far.  Casting to size_t is safe on 32-bit because we know the max buffer size is defined as less than
                 // 2^32 so content remaining can't be more than that.
                 if (bufRemains(buffer) > this->contentRemaining)
@@ -104,6 +104,10 @@ httpClientRead(HttpClient *this, Buffer *buffer, bool block)
 
                 this->contentRemaining -= ioRead(tlsClientIoRead(this->tls), buffer);
 
+                // Error if EOF but content read is not complete
+                if (ioReadEof(tlsClientIoRead(this->tls)))
+                    THROW(FileReadError, "unexpected EOF reading HTTP content");
+
                 // Clear limit (this works even if the limit was not set and it is easier than checking)
                 bufLimitClear(buffer);
             }
diff --git a/test/src/module/common/ioHttpTest.c b/test/src/module/common/ioHttpTest.c
index 325681038..5e5ce08d0 100644
--- a/test/src/module/common/ioHttpTest.c
+++ b/test/src/module/common/ioHttpTest.c
@@ -207,6 +207,48 @@ testHttpServer(void)
 
         harnessTlsServerClose();
 
+        // Request with eof before content complete with retry
+        harnessTlsServerAccept();
+
+        harnessTlsServerExpect(
+            "GET /path/file%201.txt HTTP/1.1\r\n"
+            "\r\n");
+
+        harnessTlsServerReply(
+            "HTTP/1.1 200 OK\r\n"
+            "content-length:32\r\n"
+            "\r\n"
+            "0123456789012345678901234567890");
+
+        harnessTlsServerClose();
+
+        harnessTlsServerAccept();
+
+        harnessTlsServerExpect(
+            "GET /path/file%201.txt HTTP/1.1\r\n"
+            "\r\n");
+
+        harnessTlsServerReply(
+            "HTTP/1.1 200 OK\r\n"
+            "content-length:32\r\n"
+            "\r\n"
+            "01234567890123456789012345678901");
+
+        // Request with eof before content complete
+        harnessTlsServerAccept();
+
+        harnessTlsServerExpect(
+            "GET /path/file%201.txt HTTP/1.1\r\n"
+            "\r\n");
+
+        harnessTlsServerReply(
+            "HTTP/1.1 200 OK\r\n"
+            "content-length:32\r\n"
+            "\r\n"
+            "0123456789012345678901234567890");
+
+        harnessTlsServerClose();
+
         // Request with chunked content
         harnessTlsServerAccept();
 
@@ -425,6 +467,21 @@ testRun(void)
         TEST_RESULT_STR(strPtr(strNewBuf(buffer)),  "01234567890123456789012345678901", "    check response");
         TEST_RESULT_UINT(httpClientRead(client, bufNew(1), true), 0, "    call internal read to check eof");
 
+        // Request with eof before content complete with retry
+        TEST_ASSIGN(
+            buffer, httpClientRequest(client, strNew("GET"), strNew("/path/file 1.txt"), NULL, NULL, true),
+            "request with content length retry");
+        TEST_RESULT_STR(strPtr(strNewBuf(buffer)),  "01234567890123456789012345678901", "    check response");
+        TEST_RESULT_UINT(httpClientRead(client, bufNew(1), true), 0, "    call internal read to check eof");
+
+        // Request with eof before content and error
+        buffer = bufNew(32);
+        TEST_RESULT_VOID(
+            httpClientRequest(client, strNew("GET"), strNew("/path/file 1.txt"), NULL, NULL, false),
+            "request with content length error");
+        TEST_ERROR(
+            ioRead(httpClientIoRead(client), buffer), FileReadError, "unexpected EOF reading HTTP content");
+
         // Request with content using chunked encoding
         TEST_RESULT_VOID(httpClientRequest(client, strNew("GET"), strNew("/"), NULL, NULL, false), "request with chunked encoding");
         TEST_RESULT_STR(