HTTP chunked encoding allows for trailer fields at the end of the
transmission. Currently rpki-client's http code does not handle them well.

This diff changes the code so that the chunked transfer is more like the
one defined in RFC 9112.
In short the algorithm is:
read and parse the chunked header
while datasize > 0 {
        read the amount of data
        read a single CRLF
        read and parse the chunked header
} 
read trailer
while (field not empty)
        read trailer
done

I introduce a new state STATE_RESPONSE_CHUNKED_CRLF which just expects a
CRLF (or in our case empty line). STATE_RESPONSE_CHUNKED_TRAILER is now
here to consume and discard all possible trailer fields.
As with chunked header extensions there is currently no interest in these
trailer fields.

I think this split makes the code a bit easier to read over all.
-- 
:wq Claudio

? ktrace.out
? multiproc.diff
? obj
Index: http.c
===================================================================
RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v
retrieving revision 1.67
diff -u -p -r1.67 http.c
--- http.c      8 Sep 2022 13:52:36 -0000       1.67
+++ http.c      8 Sep 2022 14:12:17 -0000
@@ -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,
@@ -1301,10 +1302,6 @@ http_parse_chunked(struct http_connectio
        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 +1443,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 +1468,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 +1697,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 +1736,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:

Reply via email to