Fix Message failed: ' Timeout' SWARN in SHTTPSManager::postPoll

libstuff/SHTTPSManager.cpp

@@ -112,24 +112,19 @@ void SStandaloneHTTPSManager::postPoll(fd_map& fdm, SStandaloneHTTPSManager::Tra
         transaction.s->recvBuffer.consumeFront(size);
         transaction.finished = now;
 
-        // This is supposed to check for a "200" or "204 No Content"response, which it does very poorly. It also checks for message
-        // content. Why this is the what constitutes a valid response is lost to time. Any well-formed response should
-        // be valid here, and this should get cleaned up. However, this requires testing anything that might rely on
-        // the existing behavior, which is an exercise for later.
-        if (
-            SContains(transaction.fullResponse.methodLine, " 200") ||
-            SContains(transaction.fullResponse.methodLine, "204") ||
-            SContains(transaction.fullResponse.methodLine, "202") ||
-            transaction.fullResponse.content.size()
-        ) {
-            // Pass the transaction down to the subclass.
-            _onRecv(transaction);
-        } else {
-            // Coercing anything that's not 200 to 500 makes no sense, and should be abandoned with the above.
-            SWARN("Message failed: '" << transaction.fullResponse.methodLine << "'");
-            transaction.response = getHTTPResponseCode(transaction.fullResponse.methodLine, 500);
+        // Log a hint for non-standard responses that don't follow HTTP format (e.g., a load balancer
+        // returning a plain-text " Timeout" instead of a proper "HTTP/1.1 504 Gateway Timeout").
+        // These are not errors in our code, but they are unexpected from the remote service.
+        // Note: non-standard responses have no Content-Length and an unparseable statusCode, so they
+        // only reach this branch once the socket is closed (making completeRequest true via CLOSED state).
+        if (!SStartsWith(transaction.fullResponse.methodLine, "HTTP/")) {
+            SHMMM("Received non-standard response: '" << transaction.fullResponse.methodLine << "'");
         }
 
+        // Pass the transaction down to the subclass for all complete responses. Each subclass's _onRecv
+        // is responsible for parsing the response code and handling errors appropriately.
+        _onRecv(transaction);
+
         // Finished with the socket, free it up.
         delete transaction.s;
         transaction.s = nullptr;
@@ -155,6 +150,10 @@ void SStandaloneHTTPSManager::postPoll(fd_map& fdm, SStandaloneHTTPSManager::Tra
                 transaction.finished = now;
                 if (!transaction.fullResponse.methodLine.empty()) {
                     _onRecv(transaction);
+                } else {
+                    // Got bytes but no parseable methodLine (e.g., bare "\r\n\r\n"). Use a safe
+                    // fallback rather than leaving response == 0, which would stall wait loops.
+                    transaction.response = 400;
                 }
 
                 // Clean up the socket

test/lib/TestHTTPS.cpp

@@ -21,6 +21,9 @@ bool TestHTTPS::_onRecv(Transaction& transaction)
             transaction.response = status;
         }
     }
+    if (!transaction.response) {
+        transaction.response = 400;
+    }
 
     return false;
 }

test/tests/LibStuffTest.cpp

@@ -2,6 +2,7 @@
 
 #include <libstuff/libstuff.h>
 #include <libstuff/SData.h>
+#include <libstuff/SHTTPSManager.h>
 #include <libstuff/SQResult.h>
 #include <libstuff/SRandom.h>
 #include <sqlitecluster/SQLite.h>
@@ -10,6 +11,7 @@
 struct LibStuff : tpunit::TestFixture
 {
     LibStuff() : tpunit::TestFixture(true, "LibStuff",
+                                     TEST(LibStuff::testGetHTTPResponseCode),
                                      TEST(LibStuff::testEncryptDecrpyt),
                                      TEST(LibStuff::testSHMACSHA1),
                                      TEST(LibStuff::testSHMACSHA256),
@@ -992,4 +994,25 @@ struct LibStuff : tpunit::TestFixture
         string methodLineWithControlChars = "500 Internal Server Error\r\nContent-Type: application/json";
         ASSERT_THROW(SComposeHTTP(methodLineWithControlChars, {}, ""), SException);
     }
+
+    void testGetHTTPResponseCode()
+    {
+        // Standard HTTP response lines parse correctly.
+        ASSERT_EQUAL(SStandaloneHTTPSManager::getHTTPResponseCode("HTTP/1.1 200 OK"), 200);
+        ASSERT_EQUAL(SStandaloneHTTPSManager::getHTTPResponseCode("HTTP/1.1 204 No Content"), 204);
+        ASSERT_EQUAL(SStandaloneHTTPSManager::getHTTPResponseCode("HTTP/1.1 202 Accepted"), 202);
+        ASSERT_EQUAL(SStandaloneHTTPSManager::getHTTPResponseCode("HTTP/1.1 400 Bad Request"), 400);
+        ASSERT_EQUAL(SStandaloneHTTPSManager::getHTTPResponseCode("HTTP/1.1 500 Internal Server Error"), 500);
+        ASSERT_EQUAL(SStandaloneHTTPSManager::getHTTPResponseCode("HTTP/1.1 504 Gateway Timeout"), 504);
+
+        // Non-standard responses (e.g., from a load balancer sending " Timeout" instead of a proper
+        // HTTP status line) fall back to 400 rather than crashing or producing a misleading result.
+        ASSERT_EQUAL(SStandaloneHTTPSManager::getHTTPResponseCode(" Timeout"), 400);
+        ASSERT_EQUAL(SStandaloneHTTPSManager::getHTTPResponseCode("Timeout"), 400);
+        ASSERT_EQUAL(SStandaloneHTTPSManager::getHTTPResponseCode(""), 400);
+
+        // Explicit defaultStatusCode is used when parsing fails.
+        ASSERT_EQUAL(SStandaloneHTTPSManager::getHTTPResponseCode("garbage", 502), 502);
+        ASSERT_EQUAL(SStandaloneHTTPSManager::getHTTPResponseCode("", 500), 500);
+    }
 } __LibStuff;