From: claudio <claudio@openbsd.org>
Date: Fri, 9 Sep 2022 08:11:06 +0000 (+0000)
Subject: Adjust chunked encoding handling.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=fd522c34be5cdd9c45f52649bfcf949d573e438a;p=openbsd

Adjust chunked encoding handling.

Add an extra state to distinguish in between chunks CRLF handling from
the last chunk which can optionally have some trailer fields included.
rpki-client ignores these trailer header fields (they are also not common
it seems).
Also remove the empty line handling in http_parse_chunked() for explicit
checks in http_read(). Because of the extra state the switch back to
non-chunked mode can now be delayed until the transfer is over.

OK tb@
---

diff --git a/usr.sbin/rpki-client/http.c b/usr.sbin/rpki-client/http.c
index f60b22ddc5b..154511a5da1 100644
--- a/usr.sbin/rpki-client/http.c
+++ b/usr.sbin/rpki-client/http.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: http.c,v 1.67 2022/09/08 13:52:36 claudio Exp $ */
+/*	$OpenBSD: http.c,v 1.68 2022/09/09 08:11:06 claudio Exp $ */
 /*
  * Copyright (c) 2020 Nils Fisher <nils_fisher@hotmail.com>
  * Copyright (c) 2020 Claudio Jeker <claudio@openbsd.org>
@@ -91,6 +91,7 @@ enum http_state {
 	STATE_RESPONSE_HEADER,
 	STATE_RESPONSE_DATA,
 	STATE_RESPONSE_CHUNKED_HEADER,
+	STATE_RESPONSE_CHUNKED_CRLF,
 	STATE_RESPONSE_CHUNKED_TRAILER,
 	STATE_WRITE_DATA,
 	STATE_IDLE,
@@ -1291,7 +1292,6 @@ http_get_line(struct http_connection *conn)
 /*
  * Parse the header between data chunks during chunked transfers.
  * Returns 0 if a new chunk size could be correctly read.
- * Returns 1 for the empty trailer lines.
  * If the chuck size could not be converted properly -1 is returned.
  */
 static int
@@ -1301,10 +1301,6 @@ http_parse_chunked(struct http_connection *conn, char *buf)
 	char *end;
 	unsigned long chunksize;
 
-	/* empty lines are used as trailer */
-	if (*header == '\0')
-		return 1;
-
 	/* strip any optional chunk extension */
 	header[strcspn(header, "; \t")] = '\0';
 	errno = 0;
@@ -1446,7 +1442,7 @@ again:
 				conn->bufpos = 0;
 			}
 			if (conn->chunked)
-				conn->state = STATE_RESPONSE_CHUNKED_TRAILER;
+				conn->state = STATE_RESPONSE_CHUNKED_CRLF;
 			else
 				conn->state = STATE_RESPONSE_DATA;
 			goto read_more;
@@ -1471,31 +1467,36 @@ again:
 		 * check if transfer is done, in which case the last trailer
 		 * still needs to be processed.
 		 */
-		if (conn->iosz == 0) {
-			conn->chunked = 0;
+		if (conn->iosz == 0)
 			conn->state = STATE_RESPONSE_CHUNKED_TRAILER;
-			goto again;
-		}
-
-		conn->state = STATE_RESPONSE_DATA;
+		else
+			conn->state = STATE_RESPONSE_DATA;
 		goto again;
-	case STATE_RESPONSE_CHUNKED_TRAILER:
+	case STATE_RESPONSE_CHUNKED_CRLF:
 		buf = http_get_line(conn);
 		if (buf == NULL)
 			goto read_more;
-		if (http_parse_chunked(conn, buf) != 1) {
+		/* expect empty line to finish a chunk of data */
+		if (*buf != '\0') {
 			warnx("%s: bad chunk encoding", http_info(conn->host));
 			free(buf);
 			return http_failed(conn);
 		}
 		free(buf);
-
-		/* if chunked got cleared then the transfer is over */
-		if (conn->chunked == 0)
-			return http_done(conn, HTTP_OK);
-
 		conn->state = STATE_RESPONSE_CHUNKED_HEADER;
 		goto again;
+	case STATE_RESPONSE_CHUNKED_TRAILER:
+		buf = http_get_line(conn);
+		if (buf == NULL)
+			goto read_more;
+		/* the trailer may include extra headers, just ignore them */
+		if (*buf != '\0') {
+			free(buf);
+			goto again;
+		}
+		free(buf);
+		conn->chunked = 0;
+		return http_done(conn, HTTP_OK);
 	default:
 		errx(1, "unexpected http state");
 	}
@@ -1695,7 +1696,7 @@ data_write(struct http_connection *conn)
 	/* all data written, switch back to read */
 	if (conn->bufpos == 0 || conn->iosz == 0) {
 		if (conn->chunked && conn->iosz == 0)
-			conn->state = STATE_RESPONSE_CHUNKED_TRAILER;
+			conn->state = STATE_RESPONSE_CHUNKED_CRLF;
 		else
 			conn->state = STATE_RESPONSE_DATA;
 		return http_read(conn);
@@ -1734,6 +1735,7 @@ http_handle(struct http_connection *conn)
 	case STATE_RESPONSE_HEADER:
 	case STATE_RESPONSE_DATA:
 	case STATE_RESPONSE_CHUNKED_HEADER:
+	case STATE_RESPONSE_CHUNKED_CRLF:
 	case STATE_RESPONSE_CHUNKED_TRAILER:
 		return http_read(conn);
 	case STATE_WRITE_DATA: