BUG/MINOR: mux-quic: fix transport VS app CONNECTION_CLOSE
A recent series of patch were introduced to streamline error generation
by QUIC MUX. However, a regression was introduced : every error
generated by the MUX was built as CONNECTION_CLOSE_APP frame, whereas it
should be only for H3/QPACK errors.
Fix this by adding an argument <app> in qcc_set_error. When false, a
standard CONNECTION_CLOSE is used as error.
This bug was detected by QUIC tracker with the following tests
"stop_sending" and "server_flow_control" which requires a
CONNECTION_CLOSE frame.
This must be backported up to 2.7.
diff --git a/include/haproxy/mux_quic.h b/include/haproxy/mux_quic.h
index e425e61..0e408bd 100644
--- a/include/haproxy/mux_quic.h
+++ b/include/haproxy/mux_quic.h
@@ -12,7 +12,7 @@
#include <haproxy/mux_quic-t.h>
#include <haproxy/stream.h>
-void qcc_set_error(struct qcc *qcc, int err);
+void qcc_set_error(struct qcc *qcc, int err, int app);
struct qcs *qcc_init_stream_local(struct qcc *qcc, int bidi);
struct buffer *qc_get_buf(struct qcs *qcs, struct buffer *bptr);
diff --git a/src/h3.c b/src/h3.c
index a364212..e686f0d 100644
--- a/src/h3.c
+++ b/src/h3.c
@@ -191,7 +191,7 @@
case H3_UNI_S_T_CTRL:
if (h3c->flags & H3_CF_UNI_CTRL_SET) {
TRACE_ERROR("duplicated control stream", H3_EV_H3S_NEW, qcs->qcc->conn, qcs);
- qcc_set_error(qcs->qcc, H3_STREAM_CREATION_ERROR);
+ qcc_set_error(qcs->qcc, H3_STREAM_CREATION_ERROR, 1);
goto err;
}
h3c->flags |= H3_CF_UNI_CTRL_SET;
@@ -206,7 +206,7 @@
case H3_UNI_S_T_QPACK_DEC:
if (h3c->flags & H3_CF_UNI_QPACK_DEC_SET) {
TRACE_ERROR("duplicated qpack decoder stream", H3_EV_H3S_NEW, qcs->qcc->conn, qcs);
- qcc_set_error(qcs->qcc, H3_STREAM_CREATION_ERROR);
+ qcc_set_error(qcs->qcc, H3_STREAM_CREATION_ERROR, 1);
goto err;
}
h3c->flags |= H3_CF_UNI_QPACK_DEC_SET;
@@ -217,7 +217,7 @@
case H3_UNI_S_T_QPACK_ENC:
if (h3c->flags & H3_CF_UNI_QPACK_ENC_SET) {
TRACE_ERROR("duplicated qpack encoder stream", H3_EV_H3S_NEW, qcs->qcc->conn, qcs);
- qcc_set_error(qcs->qcc, H3_STREAM_CREATION_ERROR);
+ qcc_set_error(qcs->qcc, H3_STREAM_CREATION_ERROR, 1);
goto err;
}
h3c->flags |= H3_CF_UNI_QPACK_ENC_SET;
@@ -1031,7 +1031,7 @@
*/
if (h3s->type == H3S_T_CTRL && fin) {
TRACE_ERROR("control stream closed by remote peer", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
- qcc_set_error(qcs->qcc, H3_CLOSED_CRITICAL_STREAM);
+ qcc_set_error(qcs->qcc, H3_CLOSED_CRITICAL_STREAM, 1);
goto err;
}
@@ -1067,7 +1067,7 @@
if (!h3_is_frame_valid(h3c, qcs, ftype)) {
TRACE_ERROR("received an invalid frame", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
- qcc_set_error(qcs->qcc, H3_FRAME_UNEXPECTED);
+ qcc_set_error(qcs->qcc, H3_FRAME_UNEXPECTED, 1);
goto err;
}
@@ -1090,7 +1090,7 @@
*/
if (flen > QC_S_RX_BUF_SZ) {
TRACE_ERROR("received a too big frame", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
- qcc_set_error(qcs->qcc, H3_EXCESSIVE_LOAD);
+ qcc_set_error(qcs->qcc, H3_EXCESSIVE_LOAD, 1);
goto err;
}
break;
@@ -1129,7 +1129,7 @@
ret = h3_parse_settings_frm(qcs->qcc->ctx, b, flen);
if (ret < 0) {
TRACE_ERROR("error on SETTINGS parsing", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
- qcc_set_error(qcs->qcc, h3c->err);
+ qcc_set_error(qcs->qcc, h3c->err, 1);
goto err;
}
h3c->flags |= H3_CF_SETTINGS_RECV;
@@ -1163,7 +1163,7 @@
return b_data(b);
}
else if (h3c->err) {
- qcc_set_error(qcs->qcc, h3c->err);
+ qcc_set_error(qcs->qcc, h3c->err, 1);
return b_data(b);
}
@@ -1670,7 +1670,7 @@
*/
if (qcs == h3c->ctrl_strm || h3s->type == H3S_T_CTRL) {
TRACE_ERROR("closure detected on control stream", H3_EV_H3S_END, qcs->qcc->conn, qcs);
- qcc_set_error(qcs->qcc, H3_CLOSED_CRITICAL_STREAM);
+ qcc_set_error(qcs->qcc, H3_CLOSED_CRITICAL_STREAM, 1);
return 1;
}
diff --git a/src/mux_quic.c b/src/mux_quic.c
index e816613..7050088 100644
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -497,10 +497,11 @@
}
/* A fatal error is detected locally for <qcc> connection. It should be closed
- * with a CONNECTION_CLOSE using <err> code. This function must not be called
- * more than once by connection.
+ * with a CONNECTION_CLOSE using <err> code. Set <app> to true to indicate that
+ * the code must be considered as an application level error. This function
+ * must not be called more than once by connection.
*/
-void qcc_set_error(struct qcc *qcc, int err)
+void qcc_set_error(struct qcc *qcc, int err, int app)
{
/* This must not be called multiple times per connection. */
BUG_ON(qcc->flags & QC_CF_ERRL);
@@ -508,7 +509,7 @@
TRACE_STATE("connection on error", QMUX_EV_QCC_ERR, qcc->conn);
qcc->flags |= QC_CF_ERRL;
- qcc->err = quic_err_app(err);
+ qcc->err = app ? quic_err_app(err) : quic_err_transport(err);
}
/* Open a locally initiated stream for the connection <qcc>. Set <bidi> for a
@@ -541,7 +542,7 @@
qcs = qcs_new(qcc, *next, type);
if (!qcs) {
TRACE_LEAVE(QMUX_EV_QCS_NEW, qcc->conn);
- qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR);
+ qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR, 0);
return NULL;
}
@@ -588,7 +589,7 @@
qcc->lfctl.ms_uni * 4;
if (id >= max_id) {
TRACE_ERROR("flow control error", QMUX_EV_QCS_NEW|QMUX_EV_PROTO_ERR, qcc->conn);
- qcc_set_error(qcc, QC_ERR_STREAM_LIMIT_ERROR);
+ qcc_set_error(qcc, QC_ERR_STREAM_LIMIT_ERROR, 0);
goto err;
}
@@ -602,7 +603,7 @@
qcs = qcs_new(qcc, *largest, type);
if (!qcs) {
TRACE_ERROR("stream fallocation failure", QMUX_EV_QCS_NEW, qcc->conn);
- qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR);
+ qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR, 0);
goto err;
}
@@ -661,13 +662,13 @@
if (!receive_only && quic_stream_is_uni(id) && quic_stream_is_remote(qcc, id)) {
TRACE_ERROR("receive-only stream not allowed", QMUX_EV_QCC_RECV|QMUX_EV_QCC_NQCS|QMUX_EV_PROTO_ERR, qcc->conn, NULL, &id);
- qcc_set_error(qcc, QC_ERR_STREAM_STATE_ERROR);
+ qcc_set_error(qcc, QC_ERR_STREAM_STATE_ERROR, 0);
goto err;
}
if (!send_only && quic_stream_is_uni(id) && quic_stream_is_local(qcc, id)) {
TRACE_ERROR("send-only stream not allowed", QMUX_EV_QCC_RECV|QMUX_EV_QCC_NQCS|QMUX_EV_PROTO_ERR, qcc->conn, NULL, &id);
- qcc_set_error(qcc, QC_ERR_STREAM_STATE_ERROR);
+ qcc_set_error(qcc, QC_ERR_STREAM_STATE_ERROR, 0);
goto err;
}
@@ -699,7 +700,7 @@
* stream.
*/
TRACE_ERROR("locally initiated stream not yet created", QMUX_EV_QCC_RECV|QMUX_EV_QCC_NQCS|QMUX_EV_PROTO_ERR, qcc->conn, NULL, &id);
- qcc_set_error(qcc, QC_ERR_STREAM_STATE_ERROR);
+ qcc_set_error(qcc, QC_ERR_STREAM_STATE_ERROR, 0);
goto err;
}
else {
@@ -755,7 +756,7 @@
TRACE_DATA("increase stream credit via MAX_STREAM_DATA", QMUX_EV_QCS_RECV, qcc->conn, qcs);
frm = qc_frm_alloc(QUIC_FT_MAX_STREAM_DATA);
if (!frm) {
- qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR);
+ qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR, 0);
return;
}
@@ -774,7 +775,7 @@
TRACE_DATA("increase conn credit via MAX_DATA", QMUX_EV_QCS_RECV, qcc->conn, qcs);
frm = qc_frm_alloc(QUIC_FT_MAX_DATA);
if (!frm) {
- qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR);
+ qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR, 0);
return;
}
@@ -989,7 +990,7 @@
if (qcs->flags & QC_SF_SIZE_KNOWN &&
(offset + len > qcs->rx.offset_max || (fin && offset + len < qcs->rx.offset_max))) {
TRACE_ERROR("final size error", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV|QMUX_EV_PROTO_ERR, qcc->conn, qcs);
- qcc_set_error(qcc, QC_ERR_FINAL_SIZE_ERROR);
+ qcc_set_error(qcc, QC_ERR_FINAL_SIZE_ERROR, 0);
goto err;
}
@@ -1022,7 +1023,7 @@
*/
TRACE_ERROR("flow control error", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV|QMUX_EV_PROTO_ERR,
qcc->conn, qcs);
- qcc_set_error(qcc, QC_ERR_FLOW_CONTROL_ERROR);
+ qcc_set_error(qcc, QC_ERR_FLOW_CONTROL_ERROR, 0);
goto err;
}
}
@@ -1061,7 +1062,7 @@
*/
TRACE_ERROR("overlapping data rejected", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV|QMUX_EV_PROTO_ERR,
qcc->conn, qcs);
- qcc_set_error(qcc, QC_ERR_PROTOCOL_VIOLATION);
+ qcc_set_error(qcc, QC_ERR_PROTOCOL_VIOLATION, 0);
return 1;
case NCB_RET_GAP_SIZE:
@@ -1194,7 +1195,7 @@
*/
if (qcc_get_qcs(qcc, id, 1, 0, &qcs)) {
TRACE_ERROR("RESET_STREAM for send-only stream received", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV, qcc->conn, qcs);
- qcc_set_error(qcc, QC_ERR_STREAM_STATE_ERROR);
+ qcc_set_error(qcc, QC_ERR_STREAM_STATE_ERROR, 0);
goto err;
}
@@ -1214,7 +1215,7 @@
if (qcs->rx.offset_max > final_size ||
((qcs->flags & QC_SF_SIZE_KNOWN) && qcs->rx.offset_max != final_size)) {
TRACE_ERROR("final size error on RESET_STREAM", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV, qcc->conn, qcs);
- qcc_set_error(qcc, QC_ERR_FINAL_SIZE_ERROR);
+ qcc_set_error(qcc, QC_ERR_FINAL_SIZE_ERROR, 0);
goto err;
}
@@ -1341,7 +1342,7 @@
TRACE_DATA("increase max stream limit with MAX_STREAMS_BIDI", QMUX_EV_QCC_SEND, qcc->conn);
frm = qc_frm_alloc(QUIC_FT_MAX_STREAMS_BIDI);
if (!frm) {
- qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR);
+ qcc_set_error(qcc, QC_ERR_INTERNAL_ERROR, 0);
goto err;
}
diff --git a/src/qpack-dec.c b/src/qpack-dec.c
index 1ae2ef3..97392bb 100644
--- a/src/qpack-dec.c
+++ b/src/qpack-dec.c
@@ -111,7 +111,7 @@
* connection error of type H3_CLOSED_CRITICAL_STREAM.
*/
if (fin) {
- qcc_set_error(qcs->qcc, H3_CLOSED_CRITICAL_STREAM);
+ qcc_set_error(qcs->qcc, H3_CLOSED_CRITICAL_STREAM, 1);
return -1;
}
@@ -158,7 +158,7 @@
* connection error of type H3_CLOSED_CRITICAL_STREAM.
*/
if (fin) {
- qcc_set_error(qcs->qcc, H3_CLOSED_CRITICAL_STREAM);
+ qcc_set_error(qcs->qcc, H3_CLOSED_CRITICAL_STREAM, 1);
return -1;
}