[GNUnet-SVN] [gnurl] 87/153: http: fix for tiny "HTTP/0.9" response

[gnunet]

This is an automated email from the git hooks/post-receive script.

ng0 pushed a commit to branch master
in repository gnurl.

commit 8440616f53056b2330393d1b0740c89a30376c67
Author: Daniel Stenberg <address@hidden>
AuthorDate: Mon Aug 13 12:12:14 2018 +0200

    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(-)

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, <address@hidden>
+ * Copyright (C) 1998 - 2018, Florin Petriuc, <address@hidden>
  *
  * 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 @@
+<testcase>
+<info>
+<keywords>
+HTTP/0.9
+</keywords>
+</info>
+
+#
+# Server-side
+<reply>
+<data>
+o
+</data>
+<servercmd>
+swsclose
+</servercmd>
+</reply>
+
+#
+# Client-side
+<client>
+<server>
+http
+</server>
+ <name>
+HTTP GET with a single-byte HTTP/0.9 response
+ </name>
+ <command>
+http://%HOSTIP:%HTTPPORT/1266
+</command>
+</client>
+
+#
+# Verify data after the test has been "shot"
+<verify>
+<strip>
+^User-Agent:.*
+</strip>
+<protocol>
+GET /1266 HTTP/1.1
+Host: %HOSTIP:%HTTPPORT
+Accept: */*
+
+</protocol>
+</verify>
+</testcase>
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 @@
+<testcase>
+<info>
+<keywords>
+HTTP/0.9
+</keywords>
+</info>
+
+#
+# Server-side
+<reply>
+<data>
+HTTPr
+</data>
+<servercmd>
+swsclose
+</servercmd>
+</reply>
+
+#
+# Client-side
+<client>
+<server>
+http
+</server>
+ <name>
+HTTP GET with a invalid HTTP/1 response line start
+ </name>
+ <command>
+http://%HOSTIP:%HTTPPORT/1267
+</command>
+</client>
+
+#
+# Verify data after the test has been "shot"
+<verify>
+<strip>
+^User-Agent:.*
+</strip>
+<protocol>
+GET /1267 HTTP/1.1
+Host: %HOSTIP:%HTTPPORT
+Accept: */*
+
+</protocol>
+</verify>
+</testcase>
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, <address@hidden>, et al.
+ * Copyright (C) 1998 - 2018, Daniel Stenberg, <address@hidden>, 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");
   }

-- 
To stop receiving notification emails like this one, please contact
address@hidden