Allow forcing downstream responses to HTTP/1.1

Summary: HTTP1xCodec already had logic to allow forcing the request version to 1.1, which can improve connection re-use.  Now extend the option to the downstream side as well.

Reviewed By: mjoras

Differential Revision: D18377733

fbshipit-source-id: 717d5eb563f04c4794cad057807a5a07404690ae

proxygen/lib/http/codec/HTTP1xCodec.cpp

@@ -77,15 +77,15 @@ appendString(IOBufQueue& queue, size_t& len, StringPiece str) {
 
 namespace proxygen {
 
-HTTP1xCodec::HTTP1xCodec(TransportDirection direction, bool forceUpstream1_1)
+HTTP1xCodec::HTTP1xCodec(TransportDirection direction, bool force1_1)
     : callback_(nullptr),
       ingressTxnID_(0),
       egressTxnID_(0),
       currentIngressBuf_(nullptr),
       headerParseState_(HeaderParseState::kParsingHeaderIdle),
       transportDirection_(direction),
       keepaliveRequested_(KeepaliveRequested::UNSET),
-      forceUpstream1_1_(forceUpstream1_1),
+      force1_1_(force1_1),
       parserActive_(false),
       pendingEOF_(false),
       parserPaused_(false),
@@ -449,6 +449,9 @@ HTTP1xCodec::generateHeader(IOBufQueue& writeBuf,
     if (version == HTTPMessage::kHTTPVersion09) {
       return;
     }
+    if (force1_1_ && version < HTTPMessage::kHTTPVersion11) {
+      version = HTTPMessage::kHTTPVersion11;
+    }
     appendLiteral(writeBuf, len, "HTTP/");
     appendUint(writeBuf, len, version.first);
     appendLiteral(writeBuf, len, ".");
@@ -459,7 +462,7 @@ HTTP1xCodec::generateHeader(IOBufQueue& writeBuf,
     appendString(writeBuf, len, statusMessage);
     break;
   case TransportDirection::UPSTREAM:
-    if (forceUpstream1_1_ && version < HTTPMessage::kHTTPVersion11) {
+    if (force1_1_ && version < HTTPMessage::kHTTPVersion11) {
       version = HTTPMessage::kHTTPVersion11;
     }
     if (msg.isEgressWebsocketUpgrade()) {

proxygen/lib/http/codec/HTTP1xCodec.h

@@ -20,7 +20,7 @@ namespace proxygen {
 class HTTP1xCodec : public HTTPCodec {
  public:
   explicit HTTP1xCodec(TransportDirection direction,
-                       bool forceUpstream1_1 = false);
+                       bool force1_1 = false);
   ~HTTP1xCodec() override;
 
   HTTP1xCodec(HTTP1xCodec&&) = default;
@@ -178,7 +178,7 @@ class HTTP1xCodec : public HTTPCodec {
   TransportDirection transportDirection_;
   KeepaliveRequested keepaliveRequested_; // only used in DOWNSTREAM mode
   std::pair<CodecProtocol, std::string> upgradeResult_; // DOWNSTREAM only
-  bool forceUpstream1_1_:1; // Use HTTP/1.1 upstream even if msg is 1.0
+  bool force1_1_:1; // Use HTTP/1.1 even if msg is 1.0
   bool parserActive_:1;
   bool pendingEOF_:1;
   bool parserPaused_:1;

proxygen/lib/http/codec/test/HTTP1xCodecTest.cpp

@@ -137,7 +137,6 @@ TEST(HTTP1xCodecTest, Test09Resp) {
 
 TEST(HTTP1xCodecTest, TestO9NoVersion) {
   HTTP1xCodec codec(TransportDirection::UPSTREAM);
-  HTTP1xCodecCallback callbacks;
   HTTPMessage req;
   auto id = codec.createStream();
   req.setHTTPVersion(0, 9);
@@ -149,6 +148,87 @@ TEST(HTTP1xCodecTest, TestO9NoVersion) {
       *buf.front(), *folly::IOBuf::copyBuffer("GET /yeah\r\n")));
 }
 
+TEST(HTTP1xCodecTest, TestKeepalive09_10) {
+  HTTP1xCodec codec1(TransportDirection::DOWNSTREAM, true);
+  HTTP1xCodecCallback callbacks1;
+  codec1.setCallback(&callbacks1);
+  auto buffer = folly::IOBuf::copyBuffer(string("GET /yeah\r\n"));
+  codec1.onIngress(*buffer);
+  EXPECT_EQ(callbacks1.headersComplete, 1);
+  EXPECT_EQ(callbacks1.messageComplete, 1);
+  EXPECT_EQ(callbacks1.msg_->getHTTPVersion(), HTTPMessage::kHTTPVersion09);
+  HTTPCodec::StreamID id = 1;
+  HTTPMessage resp;
+  resp.setHTTPVersion(0, 9);
+  resp.setStatusCode(200);
+  resp.getHeaders().set(HTTP_HEADER_CONTENT_LENGTH, "0");
+  resp.getHeaders().set(HTTP_HEADER_DATE, "");
+  folly::IOBufQueue buf{folly::IOBufQueue::cacheChainLength()};
+  codec1.generateHeader(buf, id, resp, true);
+  // Even if forced to HTTP/1.1, HTTP/0.9 has no headers
+  EXPECT_EQ(buf.chainLength(), 0);
+  EXPECT_FALSE(codec1.isReusable());
+
+  HTTP1xCodec codec2(TransportDirection::DOWNSTREAM, true);
+  HTTP1xCodecCallback callbacks2;
+  codec2.setCallback(&callbacks2);
+  buffer = folly::IOBuf::copyBuffer(string("GET /yeah HTTP/1.0\r\n\r\n"));
+  codec2.onIngress(*buffer);
+  EXPECT_EQ(callbacks2.headersComplete, 1);
+  EXPECT_EQ(callbacks2.messageComplete, 1);
+  EXPECT_EQ(callbacks2.msg_->getHTTPVersion(), HTTPMessage::kHTTPVersion10);
+  resp.setHTTPVersion(1, 0);
+  codec2.generateHeader(buf, id, resp, true);
+
+  EXPECT_TRUE(folly::IOBufEqualTo()(
+      *buf.front(), *folly::IOBuf::copyBuffer(
+       "HTTP/1.1 200 \r\n"
+       "Date: \r\n"
+       "Connection: close\r\n"
+       "Content-Length: 0\r\n\r\n")));
+  EXPECT_FALSE(codec2.isReusable());
+  buf.move();
+
+  HTTP1xCodec codec3(TransportDirection::DOWNSTREAM, true);
+  HTTP1xCodecCallback callbacks3;
+  codec3.setCallback(&callbacks3);
+  buffer = folly::IOBuf::copyBuffer(string("GET /yeah HTTP/1.0\r\n"
+                                           "Connection: keep-alive\r\n\r\n"));
+  codec3.onIngress(*buffer);
+  EXPECT_EQ(callbacks3.headersComplete, 1);
+  EXPECT_EQ(callbacks3.messageComplete, 1);
+  EXPECT_EQ(callbacks3.msg_->getHTTPVersion(), HTTPMessage::kHTTPVersion10);
+  codec3.generateHeader(buf, id, resp, true);
+  EXPECT_TRUE(folly::IOBufEqualTo()(
+      *buf.front(), *folly::IOBuf::copyBuffer(
+       "HTTP/1.1 200 \r\n"
+       "Date: \r\n"
+       "Connection: keep-alive\r\n"
+       "Content-Length: 0\r\n\r\n")));
+  EXPECT_TRUE(codec3.isReusable());
+  buf.move();
+
+  HTTP1xCodec codec4(TransportDirection::DOWNSTREAM, true);
+  HTTP1xCodecCallback callbacks4;
+  codec4.setCallback(&callbacks4);
+  buffer = folly::IOBuf::copyBuffer(string("GET /yeah HTTP/1.0\r\n\r\n"));
+  codec4.onIngress(*buffer);
+  EXPECT_EQ(callbacks4.headersComplete, 1);
+  EXPECT_EQ(callbacks4.messageComplete, 1);
+  EXPECT_EQ(callbacks4.msg_->getHTTPVersion(), HTTPMessage::kHTTPVersion10);
+  resp.getHeaders().set(HTTP_HEADER_TRANSFER_ENCODING, "chunked");
+  resp.getHeaders().remove(HTTP_HEADER_CONTENT_LENGTH);
+  resp.setIsChunked(true);
+  codec4.generateHeader(buf, id, resp, true);
+  EXPECT_TRUE(folly::IOBufEqualTo()(
+      *buf.front(), *folly::IOBuf::copyBuffer(
+       "HTTP/1.1 200 \r\n"
+       "Date: \r\n"
+       "Connection: close\r\n\r\n")));
+  EXPECT_FALSE(codec4.isReusable());
+  buf.move();
+}
+
 TEST(HTTP1xCodecTest, TestBadHeaders) {
   HTTP1xCodec codec(TransportDirection::DOWNSTREAM);
   MockHTTPCodecCallback callbacks;

proxygen/lib/http/session/HTTPDefaultSessionCodecFactory.cpp

@@ -37,7 +37,8 @@ std::unique_ptr<HTTPCodec> HTTPDefaultSessionCodecFactory::getCodec(
     return std::make_unique<HTTP2Codec>(direction);
   } else if (nextProtocol.empty() ||
              HTTP1xCodec::supportsNextProtocol(nextProtocol)) {
-    auto codec = std::make_unique<HTTP1xCodec>(direction);
+    auto codec = std::make_unique<HTTP1xCodec>(direction,
+                                               accConfig_.forceHTTP1_0_to_1_1);
     if (!isTLS) {
       codec->setAllowedUpgradeProtocols(
           accConfig_.allowedPlaintextUpgradeProtocols);

proxygen/lib/http/session/test/HQUpstreamSessionTest.cpp

@@ -62,7 +62,7 @@ class HQUpstreamSessionTest : public HQSessionTest {
                   GetParam().prParams.has_value())};
     } else {
       auto codec =
-          std::make_unique<HTTP1xCodec>(TransportDirection::DOWNSTREAM, true);
+          std::make_unique<HTTP1xCodec>(TransportDirection::DOWNSTREAM);
       // When the codec is created, need to fake the request
       FakeHTTPCodecCallback cb;
       codec->setCallback(&cb);

proxygen/lib/services/AcceptorConfiguration.h

@@ -56,6 +56,13 @@ struct AcceptorConfiguration : public wangle::ServerSocketConfig {
    */
   std::list<std::string> allowedPlaintextUpgradeProtocols;
 
+  /**
+   * True if HTTP/1.0 messages should always be serialized as HTTP/1.1
+   *
+   * Maximizes connection re-use
+   */
+  bool forceHTTP1_0_to_1_1{false};
+
   /**
    * HTTP/2 or SPDY settings for this acceptor
    */