From 3fb870ac55d691f28f8ab630a8990d17eeb70f0b Mon Sep 17 00:00:00 2001
From: Alex Hultman <alexhultman@gmail.com>
Date: Thu, 26 Dec 2019 17:17:33 +0100
Subject: [PATCH] Clean-up and fix HttpResponse::cork behavior

---
 src/HttpContext.h  |  5 +++--
 src/HttpResponse.h | 21 ++++++++++-----------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/HttpContext.h b/src/HttpContext.h
index ae7ca05..44bb3eb 100644
--- a/src/HttpContext.h
+++ b/src/HttpContext.h
@@ -218,12 +218,13 @@ private:
                 return nullptr;
             });
 
-            // basically we need to uncork in all cases, except for nullptr
+            /* We need to uncork in all cases, except for nullptr (closed socket, or upgraded socket) */
             if (returnedSocket != nullptr) {
                 /* Timeout on uncork failure */
                 auto [written, failed] = ((AsyncSocket<SSL> *) returnedSocket)->uncork();
                 if (failed) {
-                    // do we have the same timeout for websockets?
+                    /* All Http sockets timeout by this, and this behavior match the one in HttpResponse::cork */
+                    /* Warning: both HTTP_IDLE_TIMEOUT_S and HTTP_TIMEOUT_S are 10 seconds and both are used the same */
                     ((AsyncSocket<SSL> *) s)->timeout(HTTP_IDLE_TIMEOUT_S);
                 }
 
diff --git a/src/HttpResponse.h b/src/HttpResponse.h
index c6aafa6..4053a86 100644
--- a/src/HttpResponse.h
+++ b/src/HttpResponse.h
@@ -268,23 +268,22 @@ public:
         return !(httpResponseData->state & HttpResponseData<SSL>::HTTP_RESPONSE_PENDING);
     }
 
-    /* EXPERIMENTAL - corks the response if possible */
+    /* Corks the response if possible. Leaves already corked socket be. */
     HttpResponse *cork(fu2::unique_function<void()> &&handler) {
-        bool corked = Super::isCorked();
-        if (!corked && Super::canCork()) {
+        if (!Super::isCorked() && Super::canCork()) {
             Super::cork();
-            corked = true;
-        }
+            handler();
 
-        handler();
-
-        if (corked) {
-            /* Timeout on uncork failure (EXPERIMENTAL) */
+            /* Timeout on uncork failure, since most writes will succeed while corked */
             auto [written, failed] = Super::uncork();
             if (failed) {
-                // do we have the same timeout for websockets?
-                Super::timeout(10); // this is completely wrong!
+                /* For now we only have one single timeout so let's use it */
+                /* This behavior should equal the behavior in HttpContext when uncorking fails */
+                Super::timeout(HTTP_TIMEOUT_S);
             }
+        } else {
+            /* We are already corked, or can't cork so let's just call the handler */
+            handler();
         }
 
         return this;