Add Indeterminate support for generating headers

Summary: Context: https://docs.google.com/document/d/1rtw4JJJojO9NpUKlIUO0_oCRjWftusTXbajEJ0Zf8mo/edit In this diff, I add indeterminate mode support to the Binary HTTP codec (see https://www.rfc-editor.org/rfc/rfc9292.html#name-indeterminate-length-messag). The next step is to add support for a indeterminate mode when we are generating headers. Reviewed By: lnicco Differential Revision: D64713594 fbshipit-source-id: b01d19bbdc3b23e3b609ab41861641bd67e8a69b
facebook · Nov 4, 2024 · b3efccd · b3efccd
1 parent 32ed847
commit b3efccd
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 17 deletions.
diff --git a/proxygen/lib/http/codec/HTTPBinaryCodec.cpp b/proxygen/lib/http/codec/HTTPBinaryCodec.cpp
@@ -567,23 +567,31 @@ void HTTPBinaryCodec::onIngressEOF() {
 
 size_t HTTPBinaryCodec::generateHeaderHelper(folly::io::QueueAppender& appender,
                                              const HTTPHeaders& headers) {
-  // Calculate the number of bytes it will take to encode all the headers
+
   size_t headersLength = 0;
-  headers.forEach([&](folly::StringPiece name, folly::StringPiece value) {
-    auto nameSize = name.size();
-    auto valueSize = value.size();
-    headersLength += quic::getQuicIntegerSize(nameSize).value() + nameSize +
-                     quic::getQuicIntegerSize(valueSize).value() + valueSize;
-  });
+  if (knownLength_) {
+    // Calculate the number of bytes it will take to encode all the headers
+    headers.forEach([&](folly::StringPiece name, folly::StringPiece value) {
+      auto nameSize = name.size();
+      auto valueSize = value.size();
+      headersLength += quic::getQuicIntegerSize(nameSize).value() + nameSize +
+                       quic::getQuicIntegerSize(valueSize).value() + valueSize;
+    });
+
+    // Encode all the headers
+    auto lengthOfAllHeaders = encodeInteger(headersLength, appender);
+    headersLength += lengthOfAllHeaders.value();
+  }
 
-  // Encode all the headers
-  auto lengthOfAllHeaders = encodeInteger(headersLength, appender);
-  headersLength += lengthOfAllHeaders.value();
   headers.forEach([&](folly::StringPiece name, folly::StringPiece value) {
     encodeString(name, appender);
     encodeString(value, appender);
   });
 
+  if (!knownLength_) {
+    encodeInteger(0, appender);
+  }
+
   return headersLength;
 }
 
@@ -597,9 +605,14 @@ void HTTPBinaryCodec::generateHeader(
   folly::io::QueueAppender appender(&writeBuf, queueAppenderMaxGrowth);
   if (transportDirection_ == TransportDirection::UPSTREAM) {
     // Encode Framing Indicator for Request
-    encodeInteger(folly::to<uint64_t>(
-                      HTTPBinaryCodec::FramingIndicator::REQUEST_KNOWN_LENGTH),
-                  appender);
+    encodeInteger(
+        folly::to<uint64_t>(
+            knownLength_
+                ? HTTPBinaryCodec::FramingIndicator::REQUEST_KNOWN_LENGTH
+                : HTTPBinaryCodec::FramingIndicator::
+                      REQUEST_INDETERMINATE_LENGTH),
+        appender);
+
     // Encode Request Control Data
     encodeString(msg.getMethodString(), appender);
     encodeString(msg.isSecure() ? "https" : "http", appender);
@@ -612,9 +625,13 @@ void HTTPBinaryCodec::generateHeader(
     }
     encodeString(pathWithQueryString, appender);
   } else {
-    encodeInteger(folly::to<uint64_t>(
-                      HTTPBinaryCodec::FramingIndicator::RESPONSE_KNOWN_LENGTH),
-                  appender);
+    encodeInteger(
+        folly::to<uint64_t>(
+            knownLength_
+                ? HTTPBinaryCodec::FramingIndicator::RESPONSE_KNOWN_LENGTH
+                : HTTPBinaryCodec::FramingIndicator::
+                      RESPONSE_INDETERMINATE_LENGTH),
+        appender);
     // Response Control Data
     encodeInteger(msg.getStatusCode(), appender);
   }

diff --git a/proxygen/lib/http/codec/test/HTTPBinaryCodecTest.cpp b/proxygen/lib/http/codec/test/HTTPBinaryCodecTest.cpp
@@ -734,7 +734,7 @@ TEST_F(HTTPBinaryCodecTest, testOnIngressFailureMalformedMessage) {
             "Incomplete message received");
 }
 
-TEST_F(HTTPBinaryCodecTest, testGenerateHeaders) {
+TEST_F(HTTPBinaryCodecTest, testGenerateKnownLengthHeaders) {
   // Create HTTPMessage and encode it to a buffer
   HTTPMessage msgEncoded;
   msgEncoded.setMethod("GET");
@@ -767,6 +767,40 @@ TEST_F(HTTPBinaryCodecTest, testGenerateHeaders) {
   });
 }
 
+TEST_F(HTTPBinaryCodecTest, testGenerateIndeterminateLengthHeaders) {
+  // Create HTTPMessage and encode it to a buffer
+  HTTPMessage msgEncoded;
+  msgEncoded.setMethod("GET");
+  msgEncoded.setSecure(true);
+  msgEncoded.setURL("/hello.txt");
+  HTTPHeaders& headersEncoded = msgEncoded.getHeaders();
+  headersEncoded.set("user-agent",
+                     "curl/7.16.3 libcurl/7.16.3 OpenSSL/0.9.7l zlib/1.2.3");
+  headersEncoded.set("host", "www.example.com");
+  headersEncoded.set("accept-language", "en, mi");
+
+  folly::IOBufQueue writeBuffer;
+  upstreamBinaryCodecIndeterminateLength_->generateHeader(
+      writeBuffer, 0, msgEncoded);
+
+  // Now, decode the HTTPMessage from the buffer and check values
+  FakeHTTPCodecCallback callback;
+  upstreamBinaryCodecIndeterminateLength_->setCallback(&callback);
+  upstreamBinaryCodecIndeterminateLength_->onIngress(*writeBuffer.front());
+  upstreamBinaryCodecIndeterminateLength_->onIngressEOF();
+
+  EXPECT_EQ(callback.msg->getMethod(), msgEncoded.getMethod());
+  EXPECT_EQ(callback.msg->isSecure(), msgEncoded.isSecure());
+  EXPECT_EQ(callback.msg->getURL(), msgEncoded.getURL());
+  auto headersDecoded = callback.msg->getHeaders();
+  EXPECT_EQ(headersDecoded.size(), headersEncoded.size());
+  headersEncoded.forEach([&headersDecoded](const std::string& headerName,
+                                           const std::string& headerValue) {
+    EXPECT_EQ(headersDecoded.exists(headerName), true);
+    EXPECT_EQ(headersDecoded.getSingleOrEmpty(headerName), headerValue);
+  });
+}
+
 TEST_F(HTTPBinaryCodecTest, testGenerateBody) {
   // Create Test Body and encode
   std::string body = "Sample Test Body!";