From f4cf9723ede19d72aac4b339253421dca5594700 Mon Sep 17 00:00:00 2001 From: Joe Orton Date: Sat, 27 Jan 2024 11:40:14 +0000 Subject: [PATCH] Rather than appling a maximum count, apply an overall timeout to reading interim responses to match the read timeout, if a read timeout is configured and the NE_SESSFLAG_1XXTIMEOUT flag is set - which defaults to set (issue #94): * src/ne_session.h (NE_SESSFLAG_1XXTIMEOUT): New flag. * src/ne_session.c (ne_session_create): Set NE_SESSFLAG_1XXTIMEOUT to default on. * src/ne_request.c (send_request): Use the session read timeout to avoid looping reading interim responses indefinitely, rather than hard-coding a maximum number of responses. * test/request.c (fail_request_with_error): Set read timeout. (fail_excess_1xx): Adjust expected error, send more interim responses. --- src/ne_request.c | 28 ++++++++++++++++------------ src/ne_session.c | 1 + src/ne_session.h | 4 ++++ test/request.c | 9 +++++++-- 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/ne_request.c b/src/ne_request.c index 63813257..10dd3074 100644 --- a/src/ne_request.c +++ b/src/ne_request.c @@ -41,6 +41,7 @@ #ifdef HAVE_UNISTD_H #include #endif +#include #include "ne_internal.h" @@ -78,8 +79,6 @@ struct field { struct field *next; }; -/* Maximum number of interim responses. */ -#define MAX_INTERIM_RESPONSES (128) /* Maximum number of header fields per response: */ #define MAX_HEADER_FIELDS (100) /* Size of hash table; 43 is the smallest prime for which the common @@ -1016,6 +1015,10 @@ static int read_status_line(ne_request *req, ne_status *status, int retry) return 0; } +#define INTERIM_TIMEOUT(sess_) \ + (((sess_)->flags[NE_SESSFLAG_1XXTIMEOUT] && (sess_)->rdtimeout) \ + ? time(NULL) + (sess_)->rdtimeout : 0) + /* Send the request, and read the response Status-Line. Returns: * NE_RETRY connection closed by server; persistent connection * timeout @@ -1030,7 +1033,7 @@ static int send_request(ne_request *req, const ne_buffer *request) ne_status *const status = &req->status; int sentbody = 0; /* zero until body has been sent. */ int ret, retry; /* retry non-zero whilst the request should be retried */ - unsigned count; + time_t timeout = INTERIM_TIMEOUT(sess); ssize_t sret; /* Send the Request-Line and headers */ @@ -1061,13 +1064,11 @@ static int send_request(ne_request *req, const ne_buffer *request) /* Loop eating interim 1xx responses; RFC 7231§6.2 says clients * MUST be able to parse unsolicited interim responses. */ - for (count = 0; count < MAX_INTERIM_RESPONSES - && (ret = read_status_line(req, status, retry)) == NE_OK - && status->klass == 1; count++) { + while ((ret = read_status_line(req, status, retry)) == NE_OK + && status->klass == 1) { struct interim_handler *ih; - NE_DEBUG(NE_DBG_HTTP, "[req] Interim %d response %d.\n", - status->code, count); + NE_DEBUG(NE_DBG_HTTP, "[req] Interim %d response.\n", status->code); retry = 0; /* successful read() => never retry now. */ /* Discard headers with the interim response. */ @@ -1083,11 +1084,14 @@ static int send_request(ne_request *req, const ne_buffer *request) /* Send the body after receiving the first 100 Continue */ if ((ret = send_request_body(req, 0)) != NE_OK) break; sentbody = 1; + /* Reset read timeout. */ + timeout = INTERIM_TIMEOUT(sess); } - } - - if (count == MAX_INTERIM_RESPONSES) { - return aborted(req, _("Too many interim responses"), 0); + else if (sess->flags[NE_SESSFLAG_1XXTIMEOUT] && sess->rdtimeout + && time(NULL) > timeout) { + NE_DEBUG(NE_DBG_HTTP, "[req] Timeout after %d\n", sess->rdtimeout); + return aborted(req, _("Timed out reading interim responses"), 0); + } } /* Per RFC 9110ẞ15.5.9 a client MAY retry an outstanding request diff --git a/src/ne_session.c b/src/ne_session.c index 44fa7bc9..242fc72a 100644 --- a/src/ne_session.c +++ b/src/ne_session.c @@ -199,6 +199,7 @@ ne_session *ne_session_create(const char *scheme, /* Set flags which default to on: */ sess->flags[NE_SESSFLAG_PERSIST] = 1; + sess->flags[NE_SESSFLAG_1XXTIMEOUT] = 1; #ifdef NE_ENABLE_AUTO_LIBPROXY ne_session_system_proxy(sess, 0); diff --git a/src/ne_session.h b/src/ne_session.h index dce99a01..1a162dab 100644 --- a/src/ne_session.h +++ b/src/ne_session.h @@ -104,6 +104,10 @@ typedef enum ne_session_flag_e { * to improve interoperability with * SharePoint */ + NE_SESSFLAG_1XXTIMEOUT, /* disable this flag to apply no overall + * timeout when reading interim + * responses. */ + NE_SESSFLAG_LAST /* enum sentinel value */ } ne_session_flag; diff --git a/test/request.c b/test/request.c index 210180d0..4b46ca69 100644 --- a/test/request.c +++ b/test/request.c @@ -1057,6 +1057,10 @@ static int fail_request_with_error(int with_body, server_fn fn, void *ud, CALL(new_spawn_server(forever ? 100 : 1, fn, ud, &port)); sess = ne_session_create("http", "localhost", port); + + /* Set default timeout, required by e.g. fail_excess_1xx. */ + ne_set_read_timeout(sess, 2); + req = ne_request_create(sess, "GET", "/"); if (with_body) { @@ -2267,11 +2271,12 @@ static int safe_flags(void) return OK; } +/* Hit the timeout (2 seconds) for reading interim responses. */ static int fail_excess_1xx(void) { - struct s1xx_args args = {200, 0}; + struct s1xx_args args = {2000000, 0}; return fail_request_with_error(0, serve_1xx, &args, 0, - "Too many interim responses"); + "Timed out reading interim responses"); } static int serve_check_reqline(ne_socket *sock, void *userdata)