QUIC QSM: Get rid of recv_fin_retired in favour of recv_state

Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> (Merged from #21135)
openssl · Jul 16, 2023 · 5ed3a43 · 5ed3a43
1 parent b369515
commit 5ed3a43
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 12 deletions.
diff --git a/include/internal/quic_stream_map.h b/include/internal/quic_stream_map.h
@@ -201,9 +201,6 @@ struct quic_stream_st {
     /* Flags set when frames *we* sent were acknowledged. */
     unsigned int    acked_stop_sending      : 1;
 
-    /* A FIN has been retired from the rstream buffer. */
-    unsigned int    recv_fin_retired        : 1;
-
     /*
      * The stream's XSO has been deleted. Pending GC.
      *

diff --git a/ssl/quic/quic_impl.c b/ssl/quic/quic_impl.c
@@ -2049,9 +2049,10 @@ struct quic_read_again_args {
 QUIC_NEEDS_LOCK
 static int quic_validate_for_read(QUIC_XSO *xso, int *err, int *eos)
 {
-    *eos = 0;
     QUIC_STREAM_MAP *qsm;
 
+    *eos = 0;
+
     if (xso == NULL || xso->stream == NULL) {
         *err = ERR_R_INTERNAL_ERROR;
         return 0;
@@ -2790,7 +2791,7 @@ static void quic_classify_stream(QUIC_CONNECTION *qc,
     } else if (ossl_quic_channel_is_term_any(qc->ch)) {
         /* Connection already closed. */
         *state = SSL_STREAM_STATE_CONN_CLOSED;
-    } else if (!is_write && qs->recv_fin_retired) {
+    } else if (!is_write && qs->recv_state == QUIC_RSTREAM_STATE_DATA_READ) {
         /* Application has read a FIN. */
         *state = SSL_STREAM_STATE_FINISHED;
     } else if ((!is_write && qs->stop_sending)

diff --git a/ssl/quic/quic_tserver.c b/ssl/quic/quic_tserver.c
@@ -237,7 +237,8 @@ int ossl_quic_tserver_read(QUIC_TSERVER *srv,
         return 1;
     }
 
-    if (qs->recv_fin_retired || !ossl_quic_stream_has_recv_buffer(qs))
+    if (qs->recv_state == QUIC_RSTREAM_STATE_DATA_READ
+        || !ossl_quic_stream_has_recv_buffer(qs))
         return 0;
 
     if (!ossl_quic_rstream_read(qs->rstream, buf, buf_len,
@@ -261,7 +262,8 @@ int ossl_quic_tserver_read(QUIC_TSERVER *srv,
     }
 
     if (is_fin)
-        qs->recv_fin_retired = 1;
+        ossl_quic_stream_map_notify_totally_read(ossl_quic_channel_get_qsm(srv->ch),
+                                                 qs);
 
     if (*bytes_read > 0)
         ossl_quic_stream_map_update_state(ossl_quic_channel_get_qsm(srv->ch), qs);
@@ -279,15 +281,18 @@ int ossl_quic_tserver_has_read_ended(QUIC_TSERVER *srv, uint64_t stream_id)
     qs = ossl_quic_stream_map_get_by_id(ossl_quic_channel_get_qsm(srv->ch),
                                         stream_id);
 
-    if (qs == NULL || !ossl_quic_stream_has_recv_buffer(qs))
+    if (qs == NULL)
         return 0;
 
-    if (qs->recv_fin_retired)
+    if (qs->recv_state == QUIC_RSTREAM_STATE_DATA_READ)
         return 1;
 
+    if (!ossl_quic_stream_has_recv_buffer(qs))
+        return 0;
+
     /*
-     * If we do not have recv_fin_retired, it is possible we should still return
-     * 1 if there is a lone FIN (but no more data) remaining to be retired from
+     * If we do not have the DATA_READ, it is possible we should still return 1
+     * if there is a lone FIN (but no more data) remaining to be retired from
      * the RSTREAM, for example because ossl_quic_tserver_read() has not been
      * called since the FIN was received.
      */
@@ -301,8 +306,10 @@ int ossl_quic_tserver_has_read_ended(QUIC_TSERVER *srv, uint64_t stream_id)
         ossl_quic_rstream_read(qs->rstream, buf, sizeof(buf),
                                &bytes_read, &is_fin); /* best effort */
         assert(is_fin && bytes_read == 0);
+        assert(qs->recv_state == QUIC_RSTREAM_STATE_DATA_RECVD);
 
-        qs->recv_fin_retired = 1;
+        ossl_quic_stream_map_notify_totally_read(ossl_quic_channel_get_qsm(srv->ch),
+                                                 qs);
         ossl_quic_stream_map_update_state(ossl_quic_channel_get_qsm(srv->ch), qs);
         return 1;
     }