Alexander Bluhm([email protected]) on 2017.09.22 19:54:56 +0200:
> Hi,
>
> The relayd regression tests for chunked http traffic fail sometimes.
> Ktrace reveals that this happens when the \r and \n at the end of
> a header line are read in separate chunks. Then evbuffer_readline()
> interpretes the \r as the end of the first line and the \n as an
> additional empty line. So relayd gets out of sync with the http
> protocol.
>
> The function evbuffer_readln() has been added with libevent 1.4.13
> in 2010. With the parameter EVBUFFER_EOL_CRLF the parsing works
> reliably.
>
> ok?
looks good, ok benno@
> bluhm
>
> Index: usr.sbin/relayd/relay.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/relayd/relay.c,v
> retrieving revision 1.226
> diff -u -p -r1.226 relay.c
> --- usr.sbin/relayd/relay.c 28 Aug 2017 17:31:00 -0000 1.226
> +++ usr.sbin/relayd/relay.c 22 Sep 2017 16:50:51 -0000
> @@ -1679,8 +1679,10 @@ relay_close(struct rsession *con, const
> (void)print_host(&con->se_in.ss, ibuf, sizeof(ibuf));
> (void)print_host(&con->se_out.ss, obuf, sizeof(obuf));
> if (EVBUFFER_LENGTH(con->se_log) &&
> - evbuffer_add_printf(con->se_log, "\r\n") != -1)
> - ptr = evbuffer_readline(con->se_log);
> + evbuffer_add_printf(con->se_log, "\r\n") != -1) {
> + ptr = evbuffer_readln(con->se_log, NULL,
> + EVBUFFER_EOL_CRLF);
> + }
> log_info("relay %s, "
> "session %d (%d active), %s, %s -> %s:%d, "
> "%s%s%s", rlay->rl_conf.name, con->se_id, relay_sessions,
> Index: usr.sbin/relayd/relay_http.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/relayd/relay_http.c,v
> retrieving revision 1.66
> diff -u -p -r1.66 relay_http.c
> --- usr.sbin/relayd/relay_http.c 28 May 2017 10:39:15 -0000 1.66
> +++ usr.sbin/relayd/relay_http.c 22 Sep 2017 16:44:05 -0000
> @@ -169,14 +169,16 @@ relay_read_http(struct bufferevent *bev,
> goto done;
> }
>
> - while (!cre->done && (line = evbuffer_readline(src)) != NULL) {
> - linelen = strlen(line);
> + while (!cre->done) {
> + line = evbuffer_readln(src, &linelen, EVBUFFER_EOL_CRLF);
> + if (line == NULL)
> + break;
>
> /*
> * An empty line indicates the end of the request.
> * libevent already stripped the \r\n for us.
> */
> - if (!linelen) {
> + if (linelen == 0) {
> cre->done = 1;
> free(line);
> break;
> @@ -594,7 +596,7 @@ relay_read_httpchunks(struct bufferevent
> struct evbuffer *src = EVBUFFER_INPUT(bev);
> char *line;
> long long llval;
> - size_t size;
> + size_t size, linelen;
>
> getmonotime(&con->se_tv_last);
> cre->timedout = 0;
> @@ -625,13 +627,13 @@ relay_read_httpchunks(struct bufferevent
> }
> switch (cre->toread) {
> case TOREAD_HTTP_CHUNK_LENGTH:
> - line = evbuffer_readline(src);
> + line = evbuffer_readln(src, &linelen, EVBUFFER_EOL_CRLF);
> if (line == NULL) {
> /* Ignore empty line, continue */
> bufferevent_enable(bev, EV_READ);
> return;
> }
> - if (strlen(line) == 0) {
> + if (linelen == 0) {
> free(line);
> goto next;
> }
> @@ -660,7 +662,7 @@ relay_read_httpchunks(struct bufferevent
> break;
> case TOREAD_HTTP_CHUNK_TRAILER:
> /* Last chunk is 0 bytes followed by trailer and empty line */
> - line = evbuffer_readline(src);
> + line = evbuffer_readln(src, &linelen, EVBUFFER_EOL_CRLF);
> if (line == NULL) {
> /* Ignore empty line, continue */
> bufferevent_enable(bev, EV_READ);
> @@ -671,7 +673,7 @@ relay_read_httpchunks(struct bufferevent
> free(line);
> goto fail;
> }
> - if (strlen(line) == 0) {
> + if (linelen == 0) {
> /* Switch to HTTP header mode */
> cre->toread = TOREAD_HTTP_HEADER;
> bev->readcb = relay_read_http;
> @@ -680,7 +682,7 @@ relay_read_httpchunks(struct bufferevent
> break;
> case 0:
> /* Chunk is terminated by an empty newline */
> - line = evbuffer_readline(src);
> + line = evbuffer_readln(src, &linelen, EVBUFFER_EOL_CRLF);
> free(line);
> if (relay_bufferevent_print(cre->dst, "\r\n") == -1)
> goto fail;
>
