RFC 7230 obsoletes multiline header from RFC 2616

According to
https://tools.ietf.org/html/rfc7230#section-3.2.4
multiline header can return "400 Bad Request".

See #318

src/civetweb.c

@@ -6927,17 +6927,15 @@ mg_store_body(struct mg_connection *conn, const char *path)
 }
 
 
-/* Parse HTTP headers from the given buffer, advance buffer to the point
- * where parsing stopped. */
-static void
+/* Parse HTTP headers from the given buffer, advance buf pointer
+ * to the point where parsing stopped.
+ * All parameters must be valid pointers (not NULL).
+ * Return <0 on error. */
+static int
 parse_http_headers(char **buf, struct mg_request_info *ri)
 {
 	int i;
 
-	if (!ri) {
-		return;
-	}
-
 	ri->num_headers = 0;
 
 	for (i = 0; i < (int)ARRAY_SIZE(ri->http_headers); i++) {
@@ -6945,24 +6943,31 @@ parse_http_headers(char **buf, struct mg_request_info *ri)
 		while ((*dp != ':') && (*dp >= 32) && (*dp <= 126)) {
 			dp++;
 		}
-		if ((dp == *buf) || (*dp != ':')) {
-			/* This is not a valid field. */
+		if (dp == *buf) {
+			/* End of headers reached. */
 			break;
-		} else {
-			/* (*dp == ':') */
-			*dp = 0;
-			ri->http_headers[i].name = *buf;
-			do {
-				dp++;
-			} while (*dp == ' ');
+		}
+		if (*dp != ':') {
+			/* This is not a valid field. */
+			return -1;
+		}
 
-			ri->http_headers[i].value = dp;
-			*buf = dp + strcspn(dp, "\r\n");
-			if (((*buf)[0] != '\r') || ((*buf)[1] != '\n')) {
-				*buf = NULL;
-			}
+		/* End of header key (*dp == ':') */
+		/* Truncate here and set the key name */
+		*dp = 0;
+		ri->http_headers[i].name = *buf;
+		do {
+			dp++;
+		} while (*dp == ' ');
+
+		/* The rest of the line is the value */
+		ri->http_headers[i].value = dp;
+		*buf = dp + strcspn(dp, "\r\n");
+		if (((*buf)[0] != '\r') || ((*buf)[1] != '\n')) {
+			*buf = NULL;
 		}
 
+
 		ri->num_headers = i + 1;
 		if (*buf) {
 			(*buf)[0] = 0;
@@ -6978,6 +6983,7 @@ parse_http_headers(char **buf, struct mg_request_info *ri)
 			break;
 		}
 	}
+	return ri->num_headers;
 }
 
 
@@ -7012,17 +7018,19 @@ is_valid_http_method(const char *method)
 
 /* Parse HTTP request, fill in mg_request_info structure.
  * This function modifies the buffer by NUL-terminating
- * HTTP request components, header names and header values. */
+ * HTTP request components, header names and header values.
+ * Parameters:
+ *   buf (in/out): pointer to the HTTP header to parse and split
+ *   len (in): length of HTTP header buffer
+ *   re (out): parsed header as mg_request_info
+ * buf and ri must be valid pointers (not NULL), len>0.
+ * Returns <0 on error. */
 static int
 parse_http_message(char *buf, int len, struct mg_request_info *ri)
 {
 	int is_request, request_length;
 	char *start_line;
 
-	if (!ri) {
-		return 0;
-	}
-
 	request_length = get_request_len(buf, len);
 
 	if (request_length > 0) {
@@ -7043,17 +7051,24 @@ parse_http_message(char *buf, int len, struct mg_request_info *ri)
 		ri->request_uri = skip(&start_line, " ");
 		ri->http_version = start_line;
 
-		/* HTTP message could be either HTTP request or HTTP response, e.g.
-		 * "GET / HTTP/1.0 ...." or  "HTTP/1.0 200 OK ..." */
+		/* HTTP message could be either HTTP request:
+		 * "GET / HTTP/1.0 ..."
+		 * or a HTTP response:
+		 *  "HTTP/1.0 200 OK ..."
+		 * otherwise it is invalid.
+		 */
 		is_request = is_valid_http_method(ri->request_method);
 		if ((is_request && memcmp(ri->http_version, "HTTP/", 5) != 0)
 		    || (!is_request && memcmp(ri->request_method, "HTTP/", 5) != 0)) {
-			request_length = -1;
-		} else {
-			if (is_request) {
-				ri->http_version += 5;
-			}
-			parse_http_headers(&buf, ri);
+			/* Not a valid request or response: invalid */
+			return -1;
+		}
+		if (is_request) {
+			ri->http_version += 5;
+		}
+		if (parse_http_headers(&buf, ri) < 0) {
+			/* Error while parsing headers */
+			return -1;
 		}
 	}
 	return request_length;

test/private.c

@@ -57,13 +57,16 @@ START_TEST(test_parse_http_message)
 	char req1[] = "GET / HTTP/1.1\r\n\r\n";
 	char req2[] = "BLAH / HTTP/1.1\r\n\r\n";
 	char req3[] = "GET / HTTP/1.1\r\nBah\r\n";
-	char req4[] = "GET / HTTP/1.1\r\nA: foo bar\r\nB: bar\r\nbaz\r\n\r\n";
+	char req4[] =
+	    "GET / HTTP/1.1\r\nA: foo bar\r\nB: bar\r\nskip\r\nbaz:\r\n\r\n";
 	char req5[] = "GET / HTTP/1.1\r\n\r\n";
 	char req6[] = "G";
 	char req7[] = " blah ";
 	char req8[] = " HTTP/1.1 200 OK \n\n";
 	char req9[] = "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n";
 
+	char req10[] = "GET / HTTP/1.1\r\nA: foo bar\r\nB: bar\r\n\r\n";
+
 	ck_assert_int_eq(sizeof(req9) - 1,
 	                 parse_http_message(req9, sizeof(req9), &ri));
 	ck_assert_int_eq(1, ri.num_headers);
@@ -81,17 +84,19 @@ START_TEST(test_parse_http_message)
 	ck_assert_int_eq(sizeof(req8) - 1,
 	                 parse_http_message(req8, sizeof(req8), &ri));
 
-	/* TODO(lsm): Fix this. Header value may span multiple lines. */
-	ck_assert_int_eq(sizeof(req4) - 1,
-	                 parse_http_message(req4, sizeof(req4), &ri));
+	/* Multiline header are obsolete, so return an error
+	 * (https://tools.ietf.org/html/rfc7230#section-3.2.4). */
+	ck_assert_int_eq(-1, parse_http_message(req4, sizeof(req4), &ri));
+
+	ck_assert_int_eq(sizeof(req10) - 1,
+	                 parse_http_message(req10, sizeof(req10), &ri));
 	ck_assert_str_eq("1.1", ri.http_version);
-	ck_assert_int_eq(3, ri.num_headers);
+	ck_assert_int_eq(2, ri.num_headers);
 	ck_assert_str_eq("A", ri.http_headers[0].name);
 	ck_assert_str_eq("foo bar", ri.http_headers[0].value);
 	ck_assert_str_eq("B", ri.http_headers[1].name);
 	ck_assert_str_eq("bar", ri.http_headers[1].value);
-	ck_assert_str_eq("baz", ri.http_headers[2].name);
-	ck_assert(ri.http_headers[2].value == NULL);
+
 
 	ck_assert_int_eq(sizeof(req5) - 1,
 	                 parse_http_message(req5, sizeof(req5), &ri));
@@ -705,6 +710,7 @@ MAIN_PRIVATE(void)
 	test_alloc_vprintf(0);
 	test_mg_vsnprintf(0);
 	test_parse_date_string(0);
+	test_parse_http_message(0);
 }
 
 #endif