maskit commented on code in PR #9987:
URL: https://github.com/apache/trafficserver/pull/9987#discussion_r1454017382


##########
proxy/http2/Http2Stream.cc:
##########
@@ -315,19 +309,36 @@ Http2Stream::send_request(Http2ConnectionState &cstate)
     }
   } while (!done);
 
-  if (bufindex == 0) {
+  if (dumpoffset == 0) {
     // No data to signal read event
     return;
   }
 
   // Is the _sm ready to process the header?
   if (this->read_vio.nbytes > 0) {
     if (this->receive_end_stream) {
-      this->read_vio.nbytes = bufindex;
-      this->read_vio.ndone  = bufindex;
+      // This is dangerous. The _sm has possibly not read
+      // the data yet, so we end up with a mismatch if there
+      // is something async in between.
+      // Better would be to get the actual difference between
+      // the start of the headers and the actual position in
+      // the buffer for the ndone part.
+      // Howerver, during testing we have not seen any issues.
+      this->read_vio.nbytes = this->read_vio.ndone + dumpoffset;

Review Comment:
   Just to confirm, does this mean `nbytes` is reset after processing the 
standard headers? Are we sure the first `nbytes` is fully processed before 
trailer header overwrites it? 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to