QUIC TXP: Fix bug in send stream handling, cleanup

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19703)

diff --git a/include/internal/quic_stream.h b/include/internal/quic_stream.h
index 76c2238a9006ab5167cbda833ed13584d2cf096f..7b48d0b304c51f092595da262525da2c96fa235a 100644 (file)
--- a/include/internal/quic_stream.h
+++ b/include/internal/quic_stream.h
@@ -131,6 +131,12 @@ int ossl_quic_sstream_get_stream_frame(QUIC_SSTREAM *qss,
                                        OSSL_QTX_IOVEC *iov,
                                        size_t *num_iov);
 
+/*
+ * Returns 1 if there is data pending transmission. Equivalent to calling
+ * ossl_quic_sstream_get_stream_frame and seeing if it succeeds.
+ */
+int ossl_quic_sstream_has_pending(QUIC_SSTREAM *qss);
+
 /*
  * Returns the current size of the stream; i.e., the number of bytes which have
  * been appended to the stream so far.

diff --git a/ssl/quic/quic_sstream.c b/ssl/quic/quic_sstream.c
index 47d7ab1d21043b20094cc5d3ea996eaeae570c5e..a5c4b9282387941b41a097ed4b349d2c1286eeff 100644 (file)
--- a/ssl/quic/quic_sstream.c
+++ b/ssl/quic/quic_sstream.c
@@ -280,6 +280,10 @@ int ossl_quic_sstream_get_stream_frame(QUIC_SSTREAM *qss,
         range = ossl_list_uint_set_next(range);
 
     if (range == NULL) {
+        if (i < skip)
+            /* Don't return FIN for infinitely increasing skip */
+            return 0;
+
         /* No new bytes to send, but we might have a FIN */
         if (!qss->have_final_size || qss->sent_final_size)
             return 0;
@@ -333,6 +337,15 @@ int ossl_quic_sstream_get_stream_frame(QUIC_SSTREAM *qss,
     return 1;
 }
 
+int ossl_quic_sstream_has_pending(QUIC_SSTREAM *qss)
+{
+    OSSL_QUIC_FRAME_STREAM shdr;
+    OSSL_QTX_IOVEC iov[2];
+    size_t num_iov = OSSL_NELEM(iov);
+
+    return ossl_quic_sstream_get_stream_frame(qss, 0, &shdr, iov, &num_iov);
+}
+
 uint64_t ossl_quic_sstream_get_cur_size(QUIC_SSTREAM *qss)
 {
     return qss->ring_buf.head_offset;

diff --git a/ssl/quic/quic_txp.c b/ssl/quic/quic_txp.c
index 6ebe2283ade2e55e69fa847d49457e8fbe8cf294..9c8fd9b547ae48c4dccf676b3cbfea3422126dc5 100644 (file)
--- a/ssl/quic/quic_txp.c
+++ b/ssl/quic/quic_txp.c
@@ -1409,6 +1409,7 @@ static int txp_generate_stream_frames(OSSL_QUIC_TX_PACKETISER *txp,
     size_t i, j, space_left;
     int needs_padding_if_implicit, can_fill_payload, use_explicit_len;
     int could_have_following_chunk;
+    uint64_t orig_len;
     uint64_t hdr_len_implicit, payload_len_implicit;
     uint64_t hdr_len_explicit, payload_len_explicit;
     uint64_t fc_swm, fc_new_hwm;
@@ -1459,6 +1460,7 @@ static int txp_generate_stream_frames(OSSL_QUIC_TX_PACKETISER *txp,
             goto err;
 
         shdr = &chunks[i % 2].shdr;
+        orig_len = shdr->len;
         if (i > 0)
             /* Load next chunk for lookahead. */
             if (!txp_plan_stream_chunk(txp, h, sstream, stream_txfc, i + 1,
@@ -1577,6 +1579,16 @@ static int txp_generate_stream_frames(OSSL_QUIC_TX_PACKETISER *txp,
         chunk.has_reset_stream  = 0;
         if (!ossl_quic_txpim_pkt_append_chunk(tpkt, &chunk))
             goto err; /* alloc error */
+
+        if (shdr->len < orig_len) {
+            /*
+             * If we did not serialize all of this chunk we definitely do not
+             * want to try the next chunk (and we must not mark the stream
+             * as drained).
+             */
+            rc = 1;
+            goto err;
+        }
     }
 
 err:
@@ -2093,11 +2105,8 @@ static int txp_generate_for_el_actual(OSSL_QUIC_TX_PACKETISER *txp,
          */
         ossl_quic_stream_map_update_state(txp->args.qsm, stream);
 
-        if (!stream->want_max_stream_data
-            && !stream->want_stop_sending
-            && !stream->want_reset_stream
-            && (stream->txp_drained || stream->txp_blocked))
-            assert(!stream->active);
+        if (stream->txp_drained)
+            assert(!ossl_quic_sstream_has_pending(stream->sstream));
     }
 
     /* We have now sent the packet, so update state accordingly. */

diff --git a/test/quic_fifd_test.c b/test/quic_fifd_test.c
index dfcabfa7cdbcc40ca494526002cc0408647e4164..c86ee83860626ee4f924627a381d56212bbf2352 100644 (file)
--- a/test/quic_fifd_test.c
+++ b/test/quic_fifd_test.c
@@ -277,7 +277,7 @@ static int test_generic(INFO *info, int kind)
 
         /* FIN should have been marked as lost */
         num_iov = OSSL_NELEM(iov);
-        if (!TEST_true(ossl_quic_sstream_get_stream_frame(info->sstream[1], 10,
+        if (!TEST_true(ossl_quic_sstream_get_stream_frame(info->sstream[1], 1,
                                                           &hdr, iov, &num_iov))
             || !TEST_true(hdr.is_fin)
             || !TEST_uint64_t_eq(hdr.len, 0))