From: Willy Tarreau Date: Tue, 8 Sep 2026 12:23:54 +0000 (+0200) Subject: MEDIUM: h3: rely on the generic trailers parser X-Git-Tag: v3.5-dev7~58 X-Git-Url: https://git.kaiwu.me/?a=commitdiff_plain;h=25a11d345425be08cb01fec0eae5dfe8b423ee57;p=haproxy.git MEDIUM: h3: rely on the generic trailers parser h3_trailers_to_htx() performed exactly the same controls as the generic http_trailers_to_htx() parser, but was relying on traces and glitches for each failure case that was codified on the latter parser. Let's just drop the parsing loop for a direct call to the function and a check on the return codes to decide what trace/glitch to emit. It's worth noting that H3_ERR_MESSAGE_ERROR is not set when failing to add a header, not sure whether this is a design decision or an overlook. In any case this behavior was preserved. Note that pseudo-headers are detected on their syntax here, while h2 used to detect them on their assigned number, so this remains a tiny difference between the two parsers at this point. Regtests continue to pass. --- diff --git a/src/h3.c b/src/h3.c index aab7666b0..50f90a3d1 100644 --- a/src/h3.c +++ b/src/h3.c @@ -1499,11 +1499,9 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf, struct htx_sl *sl; uint32_t data_ofs = 0; /* used to rollback on error */ struct http_hdr list[global.tune.max_http_hdr * 2]; - int hdr_idx, ret; - const char *ctl; + enum http_parser_status prs_status; int qpack_err; - struct ist v; - int i; + int ret; TRACE_ENTER(H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); @@ -1545,51 +1543,28 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf, TRACE_ERROR("cannot notify missing body after trailers", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); } - hdr_idx = 0; - while (1) { - if (isteq(list[hdr_idx].n, ist(""))) - break; + prs_status = http_trailers_to_htx(list, htx); + switch (prs_status) { + case HTTP_PRS_SUCCESS: + break; + case HTTP_PRS_PHDR_TRL: /* RFC 9114 4.3. HTTP Control Data * - * Pseudo-header - * fields MUST NOT appear in trailer sections. + * Pseudo-header fields MUST NOT appear in trailer sections. */ - if (istmatch(list[hdr_idx].n, ist(":"))) { - TRACE_ERROR("pseudo-header field in trailers", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); - h3s->err = H3_ERR_MESSAGE_ERROR; - qcc_report_glitch(h3c->qcc, 1); - len = -1; - goto out; - } + TRACE_ERROR("pseudo-header field in trailers", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); + goto fail_with_glitch; - for (i = 0; i < list[hdr_idx].n.len; ++i) { - const char c = list[hdr_idx].n.ptr[i]; - if ((uint8_t)(c - 'A') <= 'Z' - 'A' || !HTTP_IS_TOKEN(c)) { - TRACE_ERROR("invalid characters in field name", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); - h3s->err = H3_ERR_MESSAGE_ERROR; - qcc_report_glitch(h3c->qcc, 1); - len = -1; - goto out; - } - } + case HTTP_PRS_INV_HNAME: + TRACE_ERROR("invalid characters in field name", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); + goto fail_with_glitch; - /* forbidden HTTP/3 headers, cf h3_req_headers_to_htx() */ - if (isteq(list[hdr_idx].n, ist("host")) || - isteq(list[hdr_idx].n, ist("content-length")) || - isteq(list[hdr_idx].n, ist("connection")) || - isteq(list[hdr_idx].n, ist("proxy-connection")) || - isteq(list[hdr_idx].n, ist("keep-alive")) || - isteq(list[hdr_idx].n, ist("upgrade")) || - isteq(list[hdr_idx].n, ist("te")) || - isteq(list[hdr_idx].n, ist("transfer-encoding"))) { - TRACE_ERROR("forbidden HTTP/3 headers", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); - h3s->err = H3_ERR_MESSAGE_ERROR; - qcc_report_glitch(h3c->qcc, 1); - len = -1; - goto out; - } + case HTTP_PRS_FORB_TRL: + TRACE_ERROR("forbidden HTTP/3 headers", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); + goto fail_with_glitch; + case HTTP_PRS_INV_HVAL: /* RFC 9114 10.3 Intermediary-Encapsulation Attacks * * While most values that can be encoded will not alter field @@ -1599,44 +1574,16 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf, * response that contains a character not permitted in a field * value MUST be treated as malformed */ + TRACE_ERROR("control character present in trailer value", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); + goto fail_with_glitch; - /* look for forbidden control characters in the trailer value */ - ctl = ist_find_ctl(list[hdr_idx].v); - if (unlikely(ctl) && http_header_has_forbidden_char(list[hdr_idx].v, ctl)) { - TRACE_ERROR("control character present in trailer value", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); - h3s->err = H3_ERR_MESSAGE_ERROR; - qcc_report_glitch(h3c->qcc, 1); - len = -1; - goto out; - } - - /* trim leading/trailing LWS */ - for (v = list[hdr_idx].v; v.len; v.len--) { - if (unlikely(HTTP_IS_LWS(*v.ptr))) - v.ptr++; - else if (!unlikely(HTTP_IS_LWS(v.ptr[v.len - 1]))) - break; - } - - if (!htx_add_trailer(htx, list[hdr_idx].n, v)) { - TRACE_ERROR("cannot add trailer", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); - len = -1; - goto out; - } - - ++hdr_idx; - } - - /* Check the number of blocks against "tune.http.maxhdr" value before adding EOT block */ - if (hdr_idx > global.tune.max_http_hdr) { - len = -1; - goto out; - } - - if (!htx_add_endof(htx, HTX_BLK_EOT)) { + default: + /* Not a protocol error, only the trailers block could not + * be emitted, so do not count a glitch (note: this might be + * caused by too large trailers). + */ TRACE_ERROR("cannot add trailer", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); - len = -1; - goto out; + goto fail; } if (fin) @@ -1659,6 +1606,13 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf, TRACE_LEAVE(H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); return len; + + fail_with_glitch: + qcc_report_glitch(h3c->qcc, 1); + h3s->err = H3_ERR_MESSAGE_ERROR; + fail: + TRACE_DEVEL("leaving on error", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); + return -1; } /* Copy from buffer a H3 DATA frame of length in QUIC stream