quic: fixup set_final_size

jasnell · jasnell · commit def8e7699923 · 2020-06-30T17:50:47.000-07:00
Ignore subsequent calls to set_final_size unless the new size is more than the previous, in which case, we have us a bug. PR-URL: nodejs#34137 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: David Carlier <devnexen@gmail.com>
diff --git a/src/quic/node_quic_stream-inl.h b/src/quic/node_quic_stream-inl.h
@@ -35,7 +35,12 @@ void QuicStream::set_flag(int32_t flag, bool on) {
 }
 
 void QuicStream::set_final_size(uint64_t final_size) {
-  CHECK_EQ(GetStat(&QuicStreamStats::final_size), 0);
+  // Only set the final size once.
+  if (is_flag_set(QUICSTREAM_FLAG_FIN)) {
+    CHECK_LE(final_size, GetStat(&QuicStreamStats::final_size));
+    return;
+  }
+  set_flag(QUICSTREAM_FLAG_FIN);
   SetStat(&QuicStreamStats::final_size, final_size);
 }
 
diff --git a/src/quic/node_quic_stream.cc b/src/quic/node_quic_stream.cc
@@ -355,7 +355,6 @@ void QuicStream::ReceiveData(
   // When fin != 0, we've received that last chunk of data for this
   // stream, indicating that the stream will no longer be readable.
   if (flags & NGTCP2_STREAM_DATA_FLAG_FIN) {
-    set_flag(QUICSTREAM_FLAG_FIN);
     set_final_size(offset + datalen);
     EmitRead(UV_EOF);
   }
diff --git a/src/quic/node_quic_stream.h b/src/quic/node_quic_stream.h
@@ -256,7 +256,11 @@ class QuicStream : public AsyncWrap,
   // Specifies the kind of headers currently being processed.
   inline void set_headers_kind(QuicStreamHeadersKind kind);
 
-  // Set the final size for the QuicStream
+  // Set the final size for the QuicStream. This only works
+  // the first time it is called. Subsequent calls will be
+  // ignored unless the subsequent size is greater than the
+  // prior set size, in which case we have a bug and we'll
+  // assert.
   inline void set_final_size(uint64_t final_size);
 
   // The final size is the maximum amount of data that has been

Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,12 @@ void QuicStream::set_flag(int32_t flag, bool on) {`
`35`	`35`	`}`
`36`	`36`
`37`	`37`	`void QuicStream::set_final_size(uint64_t final_size) {`
`38`		`- CHECK_EQ(GetStat(&QuicStreamStats::final_size), 0);`
	`38`	`+ // Only set the final size once.`
	`39`	`+ if (is_flag_set(QUICSTREAM_FLAG_FIN)) {`
	`40`	`+ CHECK_LE(final_size, GetStat(&QuicStreamStats::final_size));`
	`41`	`+ return;`
	`42`	`+ }`
	`43`	`+ set_flag(QUICSTREAM_FLAG_FIN);`
`39`	`44`	`SetStat(&QuicStreamStats::final_size, final_size);`
`40`	`45`	`}`
`41`	`46`
Original file line number	Diff line number	Diff line change
`@@ -355,7 +355,6 @@ void QuicStream::ReceiveData(`
`355`	`355`	`// When fin != 0, we've received that last chunk of data for this`
`356`	`356`	`// stream, indicating that the stream will no longer be readable.`
`357`	`357`	`if (flags & NGTCP2_STREAM_DATA_FLAG_FIN) {`
`358`		`- set_flag(QUICSTREAM_FLAG_FIN);`
`359`	`358`	`set_final_size(offset + datalen);`
`360`	`359`	`EmitRead(UV_EOF);`
`361`	`360`	`}`