From: Amaury Denoyelle Date: Mon, 13 May 2024 15:44:54 +0000 (+0200) Subject: MINOR: h3: adjust error reporting on receive X-Git-Tag: v3.0-dev12~31 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a6993a669bce3a74b79bb14e0d85199256dc808e;p=thirdparty%2Fhaproxy.git MINOR: h3: adjust error reporting on receive This commit is the second step to simplify HTTP/3 error management. This times it deals with receive side on h3_rcv_buf(). Various internal HTTP/3 to HTX conversion functions does not set H3_INTERNAL_ERROR on h3c err anymore. Only standard error code are set. For every errors, both internal and protocol ones, a negative value is returned. This ensure that h3_rcv_buf() looping is interrupted. This function will then set H3_INTERNAL_ERROR only if no standard error is registered via h3c or h3s. Along the previous commit, this should better reflect internal errors from protocol ones caused by a faulty client. --- diff --git a/src/h3.c b/src/h3.c index 4e36bb8277..1373ca72e8 100644 --- a/src/h3.c +++ b/src/h3.c @@ -504,12 +504,6 @@ static int h3_set_authority(struct qcs *qcs, struct ist *auth, const struct ist return 0; } -/* Return as is or H3_ERR_INTERNAL_ERROR if negative. Useful to prepare a standard error code. */ -static int h3_err(const int value) -{ - return value >= 0 ? value : H3_ERR_INTERNAL_ERROR; -} - /* Parse from buffer a H3 HEADERS frame of length . Data are copied * in a local HTX buffer and transfer to the stream connector layer. must be * set if this is the last data to transfer from this stream. @@ -535,6 +529,7 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf, int cookie = -1, last_cookie = -1, i; const char *ctl; int relaxed = !!(h3c->qcc->proxy->options2 & PR_O2_REQBUG_OK); + int qpack_err; /* RFC 9114 4.1.2. Malformed Requests and Responses * @@ -566,14 +561,14 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf, list, sizeof(list) / sizeof(list[0])); if (ret < 0) { TRACE_ERROR("QPACK decoding error", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); - h3c->err = h3_err(qpack_err_decode(ret)); + if ((qpack_err = qpack_err_decode(ret)) >= 0) + h3c->err = qpack_err; len = -1; goto out; } if (!b_alloc(&htx_buf, DB_SE_RX)) { TRACE_ERROR("HTX buffer alloc failure", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); - h3c->err = H3_ERR_INTERNAL_ERROR; len = -1; goto out; } @@ -706,7 +701,6 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf, sl = htx_add_stline(htx, HTX_BLK_REQ_SL, flags, meth, path, ist("HTTP/3.0")); if (!sl) { - h3c->err = H3_ERR_INTERNAL_ERROR; len = -1; goto out; } @@ -718,7 +712,6 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf, if (isttest(authority)) { if (!htx_add_header(htx, ist("host"), authority)) { - h3c->err = H3_ERR_INTERNAL_ERROR; len = -1; goto out; } @@ -837,7 +830,6 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf, } if (!htx_add_header(htx, list[hdr_idx].n, list[hdr_idx].v)) { - h3c->err = H3_ERR_INTERNAL_ERROR; len = -1; goto out; } @@ -860,14 +852,12 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf, if (cookie >= 0) { if (http_cookie_merge(htx, list, cookie)) { - h3c->err = H3_ERR_INTERNAL_ERROR; len = -1; goto out; } } if (!htx_add_endof(htx, HTX_BLK_EOH)) { - h3c->err = H3_ERR_INTERNAL_ERROR; len = -1; goto out; } @@ -879,7 +869,6 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf, htx = NULL; if (!qcs_attach_sc(qcs, &htx_buf, fin)) { - h3c->err = H3_ERR_INTERNAL_ERROR; len = -1; goto out; } @@ -935,6 +924,7 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf, struct http_hdr list[global.tune.max_http_hdr]; int hdr_idx, ret; const char *ctl; + int qpack_err; int i; TRACE_ENTER(H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); @@ -945,14 +935,14 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf, list, sizeof(list) / sizeof(list[0])); if (ret < 0) { TRACE_ERROR("QPACK decoding error", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); - h3c->err = h3_err(qpack_err_decode(ret)); + if ((qpack_err = qpack_err_decode(ret)) >= 0) + h3c->err = qpack_err; len = -1; goto out; } if (!(appbuf = qcc_get_stream_rxbuf(qcs))) { TRACE_ERROR("HTX buffer alloc failure", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); - h3c->err = H3_ERR_INTERNAL_ERROR; len = -1; goto out; } @@ -1036,7 +1026,6 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf, if (!htx_add_trailer(htx, list[hdr_idx].n, list[hdr_idx].v)) { TRACE_ERROR("cannot add trailer", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); - h3c->err = H3_ERR_INTERNAL_ERROR; len = -1; goto out; } @@ -1046,7 +1035,6 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf, if (!htx_add_endof(htx, HTX_BLK_EOT)) { TRACE_ERROR("cannot add trailer", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); - h3c->err = H3_ERR_INTERNAL_ERROR; len = -1; goto out; } @@ -1072,8 +1060,6 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf, static ssize_t h3_data_to_htx(struct qcs *qcs, const struct buffer *buf, uint64_t len, char fin) { - struct h3s *h3s = qcs->ctx; - struct h3c *h3c = h3s->h3c; struct buffer *appbuf; struct htx *htx = NULL; size_t htx_sent = 0; @@ -1084,9 +1070,7 @@ static ssize_t h3_data_to_htx(struct qcs *qcs, const struct buffer *buf, if (!(appbuf = qcc_get_stream_rxbuf(qcs))) { TRACE_ERROR("data buffer alloc failure", H3_EV_RX_FRAME|H3_EV_RX_DATA, qcs->qcc->conn, qcs); - h3c->err = H3_ERR_INTERNAL_ERROR; - len = -1; - goto out; + goto err; } htx = htx_from_buf(appbuf); @@ -1137,6 +1121,10 @@ static ssize_t h3_data_to_htx(struct qcs *qcs, const struct buffer *buf, TRACE_LEAVE(H3_EV_RX_FRAME|H3_EV_RX_DATA, qcs->qcc->conn, qcs); return htx_sent; + + err: + TRACE_DEVEL("leaving on error", H3_EV_RX_FRAME|H3_EV_RX_DATA, qcs->qcc->conn, qcs); + return -1; } /* Parse a SETTINGS frame of length of payload . @@ -1226,7 +1214,7 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin) { struct h3s *h3s = qcs->ctx; struct h3c *h3c = h3s->h3c; - ssize_t total = 0, ret; + ssize_t total = 0, ret = 0; TRACE_ENTER(H3_EV_RX_FRAME, qcs->qcc->conn, qcs); @@ -1271,24 +1259,28 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin) if (!b_data(b) && fin && quic_stream_is_bidi(qcs->id)) { struct buffer *appbuf; struct htx *htx; + int eom; TRACE_PROTO("received FIN without data", H3_EV_RX_FRAME, qcs->qcc->conn, qcs); if (!(appbuf = qcc_get_stream_rxbuf(qcs))) { TRACE_ERROR("data buffer alloc failure", H3_EV_RX_FRAME, qcs->qcc->conn, qcs); - h3c->err = H3_ERR_INTERNAL_ERROR; + qcc_set_error(qcs->qcc, H3_ERR_INTERNAL_ERROR, 1); goto err; } htx = htx_from_buf(appbuf); - if (!htx_set_eom(htx)) { + eom = htx_set_eom(htx); + htx_to_buf(htx, appbuf); + if (!eom) { TRACE_ERROR("cannot set EOM", H3_EV_RX_FRAME, qcs->qcc->conn, qcs); - h3c->err = H3_ERR_INTERNAL_ERROR; + qcc_set_error(qcs->qcc, H3_ERR_INTERNAL_ERROR, 1); + goto err; } - htx_to_buf(htx, appbuf); + goto done; } - while (b_data(b) && !(qcs->flags & QC_SF_DEM_FULL) && !h3c->err && !h3s->err) { + while (b_data(b) && !(qcs->flags & QC_SF_DEM_FULL) && ret >= 0) { uint64_t ftype, flen; char last_stream_frame = 0; @@ -1413,6 +1405,10 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin) qcc_set_error(qcs->qcc, h3c->err, 1); return b_data(b); } + else if (unlikely(ret < 0)) { + qcc_set_error(qcs->qcc, H3_ERR_INTERNAL_ERROR, 1); + goto err; + } /* TODO may be useful to wakeup the MUX if blocked due to full buffer. * However, currently, io-cb of MUX does not handle Rx.