BUG/MEDIUM: mux-quic: fix server chunked encoding response
QUIC MUX was not able to correctly deal with server response using
chunked transfer-encoding. All data will be transfered correctly to the
client but the FIN bit is missing. The transfer will never stop as the
client will wait indefinitely for the FIN bit.
This bug happened because the HTX message representing a chunked encoded
payload contains a final empty block with the EOM flag. However,
emission is skipped by QUIC MUX if there is no data to transfer. To fix
this, the condition was completed to ensure that there is no need to
send the FIN signal. If this is false, data emission will proceed even
if there is no data : this will generate an empty QUIC STREAM frame with
FIN set which will mark the end of the transfer.
To ensure that a FIN STREAM frame is sent only one time,
QC_SF_FIN_STREAM is resetted on send confirmation from the transport in
qcc_streams_sent_done().
This bug was reproduced when dealing with chunked transfer-encoding
response for the HTTP server.
This must be backported up to 2.6.
diff --git a/src/mux_quic.c b/src/mux_quic.c
index 36fdc79..77bce8a 100644
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -954,11 +954,13 @@
total = b_data(out) - head;
BUG_ON(total < 0);
- if (!total) {
+ if (!total && !fin) {
+ /* No need to send anything if total is NULL and no FIN to signal. */
TRACE_LEAVE(QMUX_EV_QCS_SEND, qcc->conn, qcs);
return 0;
}
- BUG_ON(qcs->tx.sent_offset >= qcs->tx.offset);
+ BUG_ON((!total && qcs->tx.sent_offset > qcs->tx.offset) ||
+ (total && qcs->tx.sent_offset >= qcs->tx.offset));
BUG_ON(qcs->tx.sent_offset + total > qcs->tx.offset);
BUG_ON(qcc->tx.sent_offsets + total > qcc->rfctl.md);
@@ -1004,6 +1006,16 @@
return -1;
}
+/* Check after transfering data from qcs.tx.buf if FIN must be set on the next
+ * STREAM frame for <qcs>.
+ *
+ * Returns true if FIN must be set else false.
+ */
+static int qcs_stream_fin(struct qcs *qcs)
+{
+ return qcs->flags & QC_SF_FIN_STREAM && !b_data(&qcs->tx.buf);
+}
+
/* This function must be called by the upper layer to inform about the sending
* of a STREAM frame for <qcs> instance. The frame is of <data> length and on
* <offset>.
@@ -1019,29 +1031,36 @@
/* check if the STREAM frame has already been notified. It can happen
* for retransmission.
*/
- if (offset + data <= qcs->tx.sent_offset)
+ if (offset + data < qcs->tx.sent_offset)
return;
diff = offset + data - qcs->tx.sent_offset;
+ if (diff) {
+ /* increase offset sum on connection */
+ qcc->tx.sent_offsets += diff;
+ BUG_ON_HOT(qcc->tx.sent_offsets > qcc->rfctl.md);
+ if (qcc->tx.sent_offsets == qcc->rfctl.md)
+ qcc->flags |= QC_CF_BLK_MFCTL;
- /* increase offset sum on connection */
- qcc->tx.sent_offsets += diff;
- BUG_ON_HOT(qcc->tx.sent_offsets > qcc->rfctl.md);
- if (qcc->tx.sent_offsets == qcc->rfctl.md)
- qcc->flags |= QC_CF_BLK_MFCTL;
+ /* increase offset on stream */
+ qcs->tx.sent_offset += diff;
+ BUG_ON_HOT(qcs->tx.sent_offset > qcs->tx.msd);
+ BUG_ON_HOT(qcs->tx.sent_offset > qcs->tx.offset);
+ if (qcs->tx.sent_offset == qcs->tx.msd)
+ qcs->flags |= QC_SF_BLK_SFCTL;
- /* increase offset on stream */
- qcs->tx.sent_offset += diff;
- BUG_ON_HOT(qcs->tx.sent_offset > qcs->tx.msd);
- BUG_ON_HOT(qcs->tx.sent_offset > qcs->tx.offset);
- if (qcs->tx.sent_offset == qcs->tx.msd)
- qcs->flags |= QC_SF_BLK_SFCTL;
+ if (qcs->tx.offset == qcs->tx.sent_offset &&
+ b_full(&qcs->stream->buf->buf)) {
+ qc_stream_buf_release(qcs->stream);
+ /* prepare qcs for immediate send retry if data to send */
+ if (b_data(&qcs->tx.buf))
+ LIST_APPEND(&qcc->send_retry_list, &qcs->el);
+ }
+ }
- if (qcs->tx.offset == qcs->tx.sent_offset && b_full(&qcs->stream->buf->buf)) {
- qc_stream_buf_release(qcs->stream);
- /* prepare qcs for immediate send retry if data to send */
- if (b_data(&qcs->tx.buf))
- LIST_APPEND(&qcc->send_retry_list, &qcs->el);
+ if (qcs->tx.offset == qcs->tx.sent_offset && qcs_stream_fin(qcs)) {
+ /* Reset flag to not emit multiple FIN STREAM frames. */
+ qcs->flags &= ~QC_SF_FIN_STREAM;
}
}
@@ -1134,6 +1153,7 @@
struct buffer *buf = &qcs->tx.buf;
struct buffer *out = qc_stream_buf_get(qcs->stream);
int xfer = 0;
+ char fin = 0;
/* Allocate <out> buffer if necessary. */
if (!out) {
@@ -1164,14 +1184,12 @@
/* out buffer cannot be emptied if qcs offsets differ. */
BUG_ON(!b_data(out) && qcs->tx.sent_offset != qcs->tx.offset);
+ /* FIN is set if all incoming data were transfered. */
+ fin = qcs_stream_fin(qcs);
+
/* Build a new STREAM frame with <out> buffer. */
- if (qcs->tx.sent_offset != qcs->tx.offset) {
+ if (qcs->tx.sent_offset != qcs->tx.offset || fin) {
int ret;
- char fin = !!(qcs->flags & QC_SF_FIN_STREAM);
-
- /* FIN is set if all incoming data were transfered. */
- fin = !!(fin && !b_data(buf));
-
ret = qcs_build_stream_frm(qcs, out, fin, frms);
if (ret < 0) { ABORT_NOW(); /* TODO handle this properly */ }
}