From 8440616f53056b2330393d1b0740c89a30376c67 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Mon, 13 Aug 2018 12:12:14 +0200 Subject: http: fix for tiny "HTTP/0.9" response Deal with tiny "HTTP/0.9" (header-less) responses by checking the status-line early, even before a full "HTTP/" is received to allow detecting 0.9 properly. Test 1266 and 1267 added to verify. Fixes #2420 Closes #2872 --- lib/curl_setup.h | 5 ++++ lib/http.c | 68 ++++++++++++++++++++++++++++++++----------------- lib/http2.c | 11 ++++---- lib/sha256.c | 7 ++--- lib/urldata.h | 5 ---- tests/FILEFORMAT | 1 + tests/data/Makefile.inc | 2 +- tests/data/test1266 | 46 +++++++++++++++++++++++++++++++++ tests/data/test1267 | 46 +++++++++++++++++++++++++++++++++ tests/runtests.pl | 2 +- tests/server/sws.c | 15 ++++++++--- 11 files changed, 164 insertions(+), 44 deletions(-) create mode 100644 tests/data/test1266 create mode 100644 tests/data/test1267 diff --git a/lib/curl_setup.h b/lib/curl_setup.h index 799d5fab9..24989874e 100644 --- a/lib/curl_setup.h +++ b/lib/curl_setup.h @@ -801,6 +801,11 @@ endings either CRLF or LF so 't' is appropriate. #define CURL_SA_FAMILY_T unsigned short #endif +/* Some convenience macros to get the larger/smaller value out of two given. + We prefix with CURL to prevent name collisions. */ +#define CURLMAX(x,y) ((x)>(y)?(x):(y)) +#define CURLMIN(x,y) ((x)<(y)?(x):(y)) + /* Some versions of the Android SDK is missing the declaration */ #if defined(HAVE_GETPWUID_R) && defined(HAVE_DECL_GETPWUID_R_MISSING) struct passwd; diff --git a/lib/http.c b/lib/http.c index 01ccd4ade..2e9f40719 100644 --- a/lib/http.c +++ b/lib/http.c @@ -2899,17 +2899,32 @@ CURLcode Curl_http(struct connectdata *conn, bool *done) return result; } +typedef enum { + STATUS_UNKNOWN, /* not enough data to tell yet */ + STATUS_DONE, /* a status line was read */ + STATUS_BAD /* not a status line */ +} statusline; + + +/* Check a string for a prefix. Check no more than 'len' bytes */ +static bool checkprefixmax(const char *prefix, const char *buffer, size_t len) +{ + size_t ch = CURLMIN(strlen(prefix), len); + return curl_strnequal(prefix, buffer, ch); +} + /* * checkhttpprefix() * * Returns TRUE if member of the list matches prefix of string */ -static bool +static statusline checkhttpprefix(struct Curl_easy *data, - const char *s) + const char *s, size_t len) { struct curl_slist *head = data->set.http200aliases; - bool rc = FALSE; + statusline rc = STATUS_BAD; + statusline onmatch = len >= 5? STATUS_DONE : STATUS_UNKNOWN; #ifdef CURL_DOES_CONVERSIONS /* convert from the network encoding using a scratch area */ char *scratch = strdup(s); @@ -2926,15 +2941,15 @@ checkhttpprefix(struct Curl_easy *data, #endif /* CURL_DOES_CONVERSIONS */ while(head) { - if(checkprefix(head->data, s)) { - rc = TRUE; + if(checkprefixmax(head->data, s, len)) { + rc = onmatch; break; } head = head->next; } - if(!rc && (checkprefix("HTTP/", s))) - rc = TRUE; + if((rc != STATUS_DONE) && (checkprefixmax("HTTP/", s, len))) + rc = onmatch; #ifdef CURL_DOES_CONVERSIONS free(scratch); @@ -2943,11 +2958,12 @@ checkhttpprefix(struct Curl_easy *data, } #ifndef CURL_DISABLE_RTSP -static bool +static statusline checkrtspprefix(struct Curl_easy *data, - const char *s) + const char *s, size_t len) { - bool result = FALSE; + statusline result = STATUS_BAD; + statusline onmatch = len >= 5? STATUS_DONE : STATUS_UNKNOWN; #ifdef CURL_DOES_CONVERSIONS /* convert from the network encoding using a scratch area */ @@ -2960,30 +2976,31 @@ checkrtspprefix(struct Curl_easy *data, /* Curl_convert_from_network calls failf if unsuccessful */ result = FALSE; /* can't return CURLE_foobar so return FALSE */ } - else - result = checkprefix("RTSP/", scratch)? TRUE: FALSE; + else if(checkprefixmax("RTSP/", scratch, len)) + result = onmatch; free(scratch); #else (void)data; /* unused */ - result = checkprefix("RTSP/", s)? TRUE: FALSE; + if(checkprefixmax("RTSP/", s, len)) + result = onmatch; #endif /* CURL_DOES_CONVERSIONS */ return result; } #endif /* CURL_DISABLE_RTSP */ -static bool +static statusline checkprotoprefix(struct Curl_easy *data, struct connectdata *conn, - const char *s) + const char *s, size_t len) { #ifndef CURL_DISABLE_RTSP if(conn->handler->protocol & CURLPROTO_RTSP) - return checkrtspprefix(data, s); + return checkrtspprefix(data, s, len); #else (void)conn; #endif /* CURL_DISABLE_RTSP */ - return checkhttpprefix(data, s); + return checkhttpprefix(data, s, len); } /* @@ -3097,12 +3114,15 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data, if(result) return result; - if(!k->headerline && (k->hbuflen>5)) { - /* make a first check that this looks like a protocol header */ - if(!checkprotoprefix(data, conn, data->state.headerbuff)) { + if(!k->headerline) { + /* check if this looks like a protocol header */ + statusline st = checkprotoprefix(data, conn, data->state.headerbuff, + k->hbuflen); + if(st == STATUS_BAD) { /* this is not the beginning of a protocol first header line */ k->header = FALSE; k->badheader = HEADER_ALLBAD; + streamclose(conn, "bad HTTP: No end-of-message indicator"); break; } } @@ -3131,8 +3151,10 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data, if(!k->headerline) { /* the first read header */ - if((k->hbuflen>5) && - !checkprotoprefix(data, conn, data->state.headerbuff)) { + statusline st = checkprotoprefix(data, conn, data->state.headerbuff, + k->hbuflen); + if(st == STATUS_BAD) { + streamclose(conn, "bad HTTP: No end-of-message indicator"); /* this is not the beginning of a protocol first header line */ k->header = FALSE; if(*nread) @@ -3501,7 +3523,7 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data, compare header line against list of aliases */ if(!nc) { - if(checkhttpprefix(data, k->p)) { + if(checkhttpprefix(data, k->p, k->hbuflen) == STATUS_DONE) { nc = 1; k->httpcode = 200; conn->httpversion = 10; diff --git a/lib/http2.c b/lib/http2.c index ae5fa4a11..1750dd357 100644 --- a/lib/http2.c +++ b/lib/http2.c @@ -42,7 +42,6 @@ #include "memdebug.h" #define H2_BUFSIZE 32768 -#define MIN(x,y) ((x)<(y)?(x):(y)) #if (NGHTTP2_VERSION_NUM < 0x010000) #error too old nghttp2 version, upgrade! @@ -667,7 +666,7 @@ static int on_frame_recv(nghttp2_session *session, const nghttp2_frame *frame, Curl_add_buffer(stream->header_recvbuf, "\r\n", 2); left = stream->header_recvbuf->size_used - stream->nread_header_recvbuf; - ncopy = MIN(stream->len, left); + ncopy = CURLMIN(stream->len, left); memcpy(&stream->mem[stream->memlen], stream->header_recvbuf->buffer + stream->nread_header_recvbuf, @@ -753,7 +752,7 @@ static int on_data_chunk_recv(nghttp2_session *session, uint8_t flags, if(!stream) return NGHTTP2_ERR_CALLBACK_FAILURE; - nread = MIN(stream->len, len); + nread = CURLMIN(stream->len, len); memcpy(&stream->mem[stream->memlen], data, nread); stream->len -= nread; @@ -1076,7 +1075,7 @@ static ssize_t data_source_read_callback(nghttp2_session *session, else return NGHTTP2_ERR_INVALID_ARGUMENT; - nread = MIN(stream->upload_len, length); + nread = CURLMIN(stream->upload_len, length); if(nread > 0) { memcpy(buf, stream->upload_mem, nread); stream->upload_mem += nread; @@ -1534,7 +1533,7 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex, /* If there is body data pending for this stream to return, do that */ size_t left = stream->header_recvbuf->size_used - stream->nread_header_recvbuf; - size_t ncopy = MIN(len, left); + size_t ncopy = CURLMIN(len, left); memcpy(mem, stream->header_recvbuf->buffer + stream->nread_header_recvbuf, ncopy); stream->nread_header_recvbuf += ncopy; @@ -1570,7 +1569,7 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex, } else if(stream->pausedata) { DEBUGASSERT(httpc->pause_stream_id == stream->stream_id); - nread = MIN(len, stream->pauselen); + nread = CURLMIN(len, stream->pauselen); memcpy(mem, stream->pausedata, nread); stream->pausedata += nread; diff --git a/lib/sha256.c b/lib/sha256.c index 3ac129612..f9287af23 100644 --- a/lib/sha256.c +++ b/lib/sha256.c @@ -5,7 +5,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2016, Florin Petriuc, + * Copyright (C) 1998 - 2018, Florin Petriuc, * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -123,9 +123,6 @@ static const unsigned long K[64] = { #define Sigma1(x) (S(x, 6) ^ S(x, 11) ^ S(x, 25)) #define Gamma0(x) (S(x, 7) ^ S(x, 18) ^ R(x, 3)) #define Gamma1(x) (S(x, 17) ^ S(x, 19) ^ R(x, 10)) -#ifndef MIN -#define MIN(x, y) (((x) < (y)) ? (x) : (y)) -#endif /* compress 512-bits */ static int sha256_compress(struct sha256_state *md, unsigned char *buf) @@ -200,7 +197,7 @@ static int SHA256_Update(struct sha256_state *md, inlen -= block_size; } else { - n = MIN(inlen, (block_size - md->curlen)); + n = CURLMIN(inlen, (block_size - md->curlen)); memcpy(md->buf + md->curlen, in, n); md->curlen += n; in += n; diff --git a/lib/urldata.h b/lib/urldata.h index 7d396f3f2..1d309e12d 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -157,11 +157,6 @@ typedef ssize_t (Curl_recv)(struct connectdata *conn, /* connection data */ #define GOOD_EASY_HANDLE(x) \ ((x) && ((x)->magic == CURLEASY_MAGIC_NUMBER)) -/* Some convenience macros to get the larger/smaller value out of two given. - We prefix with CURL to prevent name collisions. */ -#define CURLMAX(x,y) ((x)>(y)?(x):(y)) -#define CURLMIN(x,y) ((x)<(y)?(x):(y)) - #ifdef HAVE_GSSAPI /* Types needed for krb5-ftp connections */ struct krb5buffer { diff --git a/tests/FILEFORMAT b/tests/FILEFORMAT index d584ac163..135ded6c1 100644 --- a/tests/FILEFORMAT +++ b/tests/FILEFORMAT @@ -169,6 +169,7 @@ connection-monitor When used, this will log [DISCONNECT] to the server.input log when the connection is disconnected. upgrade when an HTTP upgrade header is found, the server will upgrade to http2 +swsclose instruct server to close connection after response For TFTP: writedelay: [secs] delay this amount between reply packets (each packet being diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc index 66d86a6cd..20274b37c 100644 --- a/tests/data/Makefile.inc +++ b/tests/data/Makefile.inc @@ -139,7 +139,7 @@ test1228 test1229 test1230 test1231 test1232 test1233 test1234 test1235 \ test1236 test1237 test1238 test1239 test1240 test1241 test1242 test1243 \ test1244 test1245 test1246 test1247 test1248 test1249 test1250 test1251 \ test1252 test1253 test1254 test1255 test1256 test1257 test1258 test1259 \ -test1260 test1261 test1262 test1263 test1264 test1265 \ +test1260 test1261 test1262 test1263 test1264 test1265 test1266 test1267 \ \ test1280 test1281 test1282 test1283 test1284 test1285 test1286 test1287 \ test1288 test1289 test1290 test1291 test1292 \ diff --git a/tests/data/test1266 b/tests/data/test1266 new file mode 100644 index 000000000..75ed7bdc9 --- /dev/null +++ b/tests/data/test1266 @@ -0,0 +1,46 @@ + + + +HTTP/0.9 + + + +# +# Server-side + + +o + + +swsclose + + + +# +# Client-side + + +http + + +HTTP GET with a single-byte HTTP/0.9 response + + +http://%HOSTIP:%HTTPPORT/1266 + + + +# +# Verify data after the test has been "shot" + + +^User-Agent:.* + + +GET /1266 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* + + + + diff --git a/tests/data/test1267 b/tests/data/test1267 new file mode 100644 index 000000000..8f2a63b78 --- /dev/null +++ b/tests/data/test1267 @@ -0,0 +1,46 @@ + + + +HTTP/0.9 + + + +# +# Server-side + + +HTTPr + + +swsclose + + + +# +# Client-side + + +http + + +HTTP GET with a invalid HTTP/1 response line start + + +http://%HOSTIP:%HTTPPORT/1267 + + + +# +# Verify data after the test has been "shot" + + +^User-Agent:.* + + +GET /1267 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* + + + + diff --git a/tests/runtests.pl b/tests/runtests.pl index 7e066fd4d..71625b1f3 100755 --- a/tests/runtests.pl +++ b/tests/runtests.pl @@ -152,7 +152,7 @@ my $NEGTELNETPORT; # TELNET server port with negotiation my $srcdir = $ENV{'srcdir'} || '.'; my $CURL="../src/curl".exe_ext(); # what curl executable to run on the tests -my $VCURL=$CURL; # what curl binary to use to verify the servers with +my $VCURL="curl"; # what curl binary to use to verify the servers with # VCURL is handy to set to the system one when the one you # just built hangs or crashes and thus prevent verification my $DBGCURL=$CURL; #"../src/.libs/curl"; # alternative for debugging diff --git a/tests/server/sws.c b/tests/server/sws.c index 4879cfa8b..fbe7761d8 100644 --- a/tests/server/sws.c +++ b/tests/server/sws.c @@ -5,7 +5,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2017, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2018, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -124,6 +124,8 @@ struct httprequest { bool connmon; /* monitor the state of the connection, log disconnects */ bool upgrade; /* test case allows upgrade to http2 */ bool upgrade_request; /* upgrade request found and allowed */ + bool close; /* similar to swsclose in response: close connection after + response is sent */ int done_processing; }; @@ -177,6 +179,9 @@ const char *serverlogfile = DEFAULT_LOGFILE; /* upgrade to http2 */ #define CMD_UPGRADE "upgrade" +/* close connection */ +#define CMD_SWSCLOSE "swsclose" + #define END_OF_HEADERS "\r\n\r\n" enum { @@ -361,7 +366,7 @@ static int parse_servercmd(struct httprequest *req) int error; filename = test2file(req->testno); - + req->close = FALSE; stream = fopen(filename, "rb"); if(!stream) { error = errno; @@ -414,6 +419,10 @@ static int parse_servercmd(struct httprequest *req) logmsg("enabled upgrade to http2"); req->upgrade = TRUE; } + else if(!strncmp(CMD_SWSCLOSE, cmd, strlen(CMD_SWSCLOSE))) { + logmsg("swsclose: close this connection after response"); + req->close = TRUE; + } else if(1 == sscanf(cmd, "pipe: %d", &num)) { logmsg("instructed to allow a pipe size of %d", num); if(num < 0) @@ -1194,7 +1203,7 @@ static int send_doc(curl_socket_t sock, struct httprequest *req) /* If the word 'swsclose' is present anywhere in the reply chunk, the connection will be closed after the data has been sent to the requesting client... */ - if(strstr(buffer, "swsclose") || !count) { + if(strstr(buffer, "swsclose") || !count || req->close) { persistent = FALSE; logmsg("connection close instruction \"swsclose\" found in response"); } -- cgit v1.2.3