Skip to content

Commit 2fe93ab

Browse files
committed
MINOR: h3: use stream error when needed instead of connection
Use a stream error when possible instead of always closing the whole connection. This requires a new field <err> in h3s structure. Change slightly the decoding loop to facilitate error propagation. It will be interrupted as soon as <h3s.err> or <h3c.err> is non null. In the later case, a CONNECTION_CLOSE is requested through qcc_emit_cc_app(). For stream error, H3 layer uses qcc_abort_stream_read() coupled with qcc_reset_stream(). This is in conformance with RFC 9114 which recommends to use STOP_SENDING + RESET_STREAM emission on stream error. This commit is part of implementing H3 errors at the stream level. This should be backported up to 2.7.
1 parent 663e872 commit 2fe93ab

1 file changed

Lines changed: 33 additions & 22 deletions

File tree

src/h3.c

Lines changed: 33 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ struct h3s {
153153
unsigned long long data_len; /* total length of all parsed DATA */
154154

155155
int flags;
156+
int err; /* used for stream reset */
156157
};
157158

158159
DECLARE_STATIC_POOL(pool_head_h3s, "h3s", sizeof(struct h3s));
@@ -369,6 +370,7 @@ static int h3_check_body_size(struct qcs *qcs, int fin)
369370
if (h3s->data_len > h3s->body_len ||
370371
(fin && h3s->data_len < h3s->body_len)) {
371372
TRACE_ERROR("Content-length does not match DATA frame size", H3_EV_RX_FRAME|H3_EV_RX_DATA, qcs->qcc->conn, qcs);
373+
h3s->err = H3_MESSAGE_ERROR;
372374
ret = -1;
373375
}
374376

@@ -380,7 +382,9 @@ static int h3_check_body_size(struct qcs *qcs, int fin)
380382
* in a local HTX buffer and transfer to the stream connector layer. <fin> must be
381383
* set if this is the last data to transfer from this stream.
382384
*
383-
* Returns the number of consumed bytes or a negative error code.
385+
* Returns the number of consumed bytes or a negative error code. On error
386+
* either the connection should be closed or the stream reset using codes
387+
* provided in h3c.err / h3s.err.
384388
*/
385389
static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
386390
uint64_t len, char fin)
@@ -461,6 +465,7 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
461465
if (isteq(list[hdr_idx].n, ist(":method"))) {
462466
if (isttest(meth)) {
463467
TRACE_ERROR("duplicated method pseudo-header", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
468+
h3s->err = H3_MESSAGE_ERROR;
464469
len = -1;
465470
goto out;
466471
}
@@ -469,6 +474,7 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
469474
else if (isteq(list[hdr_idx].n, ist(":path"))) {
470475
if (isttest(path)) {
471476
TRACE_ERROR("duplicated path pseudo-header", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
477+
h3s->err = H3_MESSAGE_ERROR;
472478
len = -1;
473479
goto out;
474480
}
@@ -478,6 +484,7 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
478484
if (isttest(scheme)) {
479485
/* duplicated pseudo-header */
480486
TRACE_ERROR("duplicated scheme pseudo-header", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
487+
h3s->err = H3_MESSAGE_ERROR;
481488
len = -1;
482489
goto out;
483490
}
@@ -486,13 +493,15 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
486493
else if (isteq(list[hdr_idx].n, ist(":authority"))) {
487494
if (isttest(authority)) {
488495
TRACE_ERROR("duplicated authority pseudo-header", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
496+
h3s->err = H3_MESSAGE_ERROR;
489497
len = -1;
490498
goto out;
491499
}
492500
authority = list[hdr_idx].v;
493501
}
494502
else {
495503
TRACE_ERROR("unknown pseudo-header", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
504+
h3s->err = H3_MESSAGE_ERROR;
496505
len = -1;
497506
goto out;
498507
}
@@ -509,6 +518,7 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
509518
*/
510519
if (!isttest(meth) || !isttest(scheme) || !isttest(path)) {
511520
TRACE_ERROR("missing mandatory pseudo-header", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
521+
h3s->err = H3_MESSAGE_ERROR;
512522
len = -1;
513523
goto out;
514524
}
@@ -544,6 +554,7 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
544554

545555
if (istmatch(list[hdr_idx].n, ist(":"))) {
546556
TRACE_ERROR("pseudo-header field after fields", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
557+
h3s->err = H3_MESSAGE_ERROR;
547558
len = -1;
548559
goto out;
549560
}
@@ -552,6 +563,7 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
552563
const char c = list[hdr_idx].n.ptr[i];
553564
if ((uint8_t)(c - 'A') < 'Z' - 'A' || !HTTP_IS_TOKEN(c)) {
554565
TRACE_ERROR("invalid characters in field name", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
566+
h3s->err = H3_MESSAGE_ERROR;
555567
len = -1;
556568
goto out;
557569
}
@@ -568,6 +580,7 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
568580
h3s->flags & H3_SF_HAVE_CLEN);
569581
if (ret < 0) {
570582
TRACE_ERROR("invalid content-length", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
583+
h3s->err = H3_MESSAGE_ERROR;
571584
len = -1;
572585
goto out;
573586
}
@@ -840,7 +853,7 @@ static ssize_t h3_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
840853
return -1;
841854
}
842855

843-
while (b_data(b) && !(qcs->flags & QC_SF_DEM_FULL)) {
856+
while (b_data(b) && !(qcs->flags & QC_SF_DEM_FULL) && !h3c->err && !h3s->err) {
844857
uint64_t ftype, flen;
845858
char last_stream_frame = 0;
846859

@@ -860,10 +873,8 @@ static ssize_t h3_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
860873
/* Check that content-length is not exceeded on a new DATA frame. */
861874
if (ftype == H3_FT_DATA) {
862875
h3s->data_len += flen;
863-
if (h3s->flags & H3_SF_HAVE_CLEN && h3_check_body_size(qcs, fin)) {
864-
qcc_emit_cc_app(qcs->qcc, h3c->err, 1);
865-
return -1;
866-
}
876+
if (h3s->flags & H3_SF_HAVE_CLEN && h3_check_body_size(qcs, fin))
877+
break;
867878
}
868879

869880
if (!h3_is_frame_valid(h3c, qcs, ftype)) {
@@ -896,31 +907,19 @@ static ssize_t h3_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
896907
}
897908

898909
/* Check content-length equality with DATA frames length on the last frame. */
899-
if (fin && h3s->flags & H3_SF_HAVE_CLEN && h3_check_body_size(qcs, fin)) {
900-
qcc_emit_cc_app(qcs->qcc, h3c->err, 1);
901-
return -1;
902-
}
910+
if (fin && h3s->flags & H3_SF_HAVE_CLEN && h3_check_body_size(qcs, fin))
911+
break;
903912

904913
last_stream_frame = (fin && flen == b_data(b));
905914

906915
h3_inc_frame_type_cnt(h3c->prx_counters, ftype);
907916
switch (ftype) {
908917
case H3_FT_DATA:
909918
ret = h3_data_to_htx(qcs, b, flen, last_stream_frame);
910-
/* TODO handle error reporting. Stream closure required. */
911-
if (ret < 0) { ABORT_NOW(); }
912919
h3s->st_req = H3S_ST_REQ_DATA;
913920
break;
914921
case H3_FT_HEADERS:
915922
ret = h3_headers_to_htx(qcs, b, flen, last_stream_frame);
916-
if (ret < 0) {
917-
/* TODO for some error, it may be preferable to
918-
* only close the stream once RESET_STREAM is
919-
* supported.
920-
*/
921-
qcc_emit_cc_app(qcs->qcc, h3c->err, 1);
922-
return -1;
923-
}
924923
h3s->st_req = (h3s->st_req == H3S_ST_REQ_BEFORE) ?
925924
H3S_ST_REQ_HEADERS : H3S_ST_REQ_TRAILERS;
926925
break;
@@ -950,14 +949,25 @@ static ssize_t h3_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
950949
break;
951950
}
952951

953-
if (ret) {
952+
if (ret > 0) {
954953
BUG_ON(h3s->demux_frame_len < ret);
955954
h3s->demux_frame_len -= ret;
956955
b_del(b, ret);
957956
total += ret;
958957
}
959958
}
960959

960+
/* Interrupt decoding on stream/connection error detected. */
961+
if (h3s->err) {
962+
qcc_abort_stream_read(qcs);
963+
qcc_reset_stream(qcs, h3s->err);
964+
return b_data(b);
965+
}
966+
else if (h3c->err) {
967+
qcc_emit_cc_app(qcs->qcc, h3c->err, 1);
968+
return b_data(b);
969+
}
970+
961971
/* TODO may be useful to wakeup the MUX if blocked due to full buffer.
962972
* However, currently, io-cb of MUX does not handle Rx.
963973
*/
@@ -1289,6 +1299,7 @@ static int h3_attach(struct qcs *qcs, void *conn_ctx)
12891299
h3s->body_len = 0;
12901300
h3s->data_len = 0;
12911301
h3s->flags = 0;
1302+
h3s->err = 0;
12921303

12931304
if (quic_stream_is_bidi(qcs->id)) {
12941305
h3s->type = H3S_T_REQ;
@@ -1376,7 +1387,7 @@ static int h3_init(struct qcc *qcc)
13761387

13771388
h3c->qcc = qcc;
13781389
h3c->ctrl_strm = NULL;
1379-
h3c->err = H3_NO_ERROR;
1390+
h3c->err = 0;
13801391
h3c->flags = 0;
13811392
h3c->id_goaway = 0;
13821393

0 commit comments

Comments
 (0)