Retry a request after getting a 408 on a persisted connection.

* src/ne_request.c (send_request): Retry for a 408 response. * test/request.c (retry_408, dont_retry_408): New tests.
notroj · Oct 23, 2023 · a0d1998 · a0d1998
1 parent 31906e9
commit a0d1998
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 2 deletions.
diff --git a/src/ne_request.c b/src/ne_request.c
@@ -1090,6 +1090,14 @@ static int send_request(ne_request *req, const ne_buffer *request)
         return aborted(req, _("Too many interim responses"), 0);
     }
 
+    /* Per RFC 9110ẞ15.5.9 a client MAY retry an outstanding request
+     * after a 408. Some modern servers generate this. */
+    if (sess->persisted && status->code == 408) {
+        NE_DEBUG(NE_DBG_HTTP, "req: Retrying after 408.\n");
+        ne_close_connection(sess);
+        return NE_RETRY;
+    }
+
     return ret;
 }
 
@@ -1291,7 +1299,7 @@ int ne_begin_request(ne_request *req)
     ret = send_request(req, data);
     /* Retry this once after a persistent connection timeout. */
     if (ret == NE_RETRY) {
-	NE_DEBUG(NE_DBG_HTTP, "Persistent connection timed out, retrying.\n");
+	NE_DEBUG(NE_DBG_HTTP, "req: Persistent connection timed out, retrying.\n");
 	ret = send_request(req, data);
     }
     ne_buffer_destroy(data);

diff --git a/test/request.c b/test/request.c
@@ -89,7 +89,7 @@ static ne_request *construct_get(ne_session *sess, void *userdata)
     ne_request *r = ne_request_create(sess, "GET", "/");
     ne_buffer *buf = userdata;
 
-    ne_add_response_body_reader(r, ne_accept_2xx, collector, buf);
+    if (buf) ne_add_response_body_reader(r, ne_accept_2xx, collector, buf);
 
     return r;
 }
@@ -2360,6 +2360,44 @@ static int interims(void)
     return OK;
 }
 
+static int retry_408(void)
+{
+    ne_session *sess;
+
+    /* Serve two responses down a single persistent connection, the
+     * second of which is invalid and will cause the request to be
+     * aborted. */
+    CALL(multi_session_server(&sess, "http", "localhost",
+                              2, single_serve_string,
+                              EMPTY_RESP
+                              "HTTP/1.1 408 Request Timeout\r\n"
+                              "Server: neon-test-server\r\n"
+                              "Content-Length: 0\r\n\r\n"));
+
+    CALL(any_2xx_request(sess, "/first408"));
+    CALL(any_2xx_request(sess, "/second408"));
+
+    return destroy_and_wait(sess);
+}
+
+/* Ensure that only a 408 on a persisted connection is
+ * retried. Otherwise it should just be handled as a final
+ * response. */
+static int dont_retry_408(void)
+{
+    ne_session *sess;
+
+    CALL(make_session(&sess, single_serve_string,
+                      "HTTP/1.1 408 Request Timeout\r\n"
+                      "Server: neon-test-server\r\n"
+                      "Content-Length: 0\r\n\r\n"));
+
+    /* Run any request, ensure it gets a 408 response. */
+    CALL(run_request(sess, 408, construct_get, NULL));
+
+    return destroy_and_wait(sess);
+}
+
 /* TODO: test that ne_set_notifier(, NULL, NULL) DTRT too. */
 
 ne_test tests[] = {
@@ -2444,5 +2482,7 @@ ne_test tests[] = {
     T(fail_excess_1xx),
     T(target_forms),
     T(interims),
+    T(retry_408),
+    T(dont_retry_408),
     T(NULL)
 };