Update recommended HTTP headers

Summary:
Updates the generated common-header files.

Diff resulted from pipeline (D115638777) and is first in plan to generate a monthly bot to update the common headers lists. Diff consists of updates to the headers list files, and resulting build errors from callers of it.

In HTTPCommonHeaders.txt: Added 4 headers, removed 16

In FacebookHTTPHeaders.txt: Added 63 Headers, removed 51

The net total between the two lists stayed as 254 headers.
For details on the decision of how headers were chosen to be removed this diff explains (D115508426), but tldr is:

- Headers counts were aggregated (1/10^6 sampling) to a scuba table
- Through the actual count of each header itself, the tiers each header appearance occurred on (slightly more value for edge and fna tiers), and the amount a header enum appears in code (for listed headers), a score was assigned to each header (higher score -> more valuable)
- The scores were sorted such that if [highest scored non-list header / lowest scored listed header] was greater than a certain ratio (chose 2.0 for first run), the header would replace it on the list
- This kept going until all of the non-listed headers were checked with the lowest scored non-swapped listed headers

Additionally: Theres a DENY_LIST for headers that likely shouldn't be removed (some psuedoheaders, connection headers, etc...) and for headers used more than 50 times in code as I deemed that traffic over a few weeks period of time likely isn't enough to justify removing them.

If you believe that a header was removed that you don't think should be removed from the list or if you believe anything gets broken from this change please lmk.

But advantages of this change can be seen below in the test plan when testing on different canary teirs.

Reviewed By: hanidamlaj

Differential Revision: D116961031

fbshipit-source-id: 9d6c281ba22ed3732fddb3f860527deb20f756fa

proxygen/httpserver/RequestHandlerAdaptor.cpp

@@ -13,6 +13,13 @@
 #include <proxygen/httpserver/RequestHandler.h>
 #include <proxygen/httpserver/ResponseBuilder.h>
 
+#include <string_view>
+
+namespace {
+// Not in the common header list, so addressed by name.
+constexpr std::string_view kExpect{"Expect"};
+} // namespace
+
 namespace proxygen {
 
 RequestHandlerAdaptor::RequestHandlerAdaptor(RequestHandler* requestHandler)
@@ -47,9 +54,8 @@ void RequestHandlerAdaptor::onHeadersComplete(
   if (!upstream_) {
     return;
   }
-  if (msg->getHeaders().exists(HTTP_HEADER_EXPECT) &&
-      !upstream_->canHandleExpect()) {
-    auto expectation = msg->getHeaders().getSingleOrEmpty(HTTP_HEADER_EXPECT);
+  if (msg->getHeaders().exists(kExpect) && !upstream_->canHandleExpect()) {
+    auto expectation = msg->getHeaders().getSingleOrEmpty(kExpect);
     if (!k100Continue.equals(expectation, folly::AsciiCaseInsensitive())) {
       setError(kErrorUnsupportedExpectation);
 

proxygen/httpserver/samples/hq/SampleHandlers.h

@@ -372,8 +372,7 @@ class ContinueHandler : public EchoHandler {
     proxygen::HTTPMessage resp;
     VLOG(10) << "Setting http-version to " << getHttpVersion();
     resp.setVersionString(getHttpVersion());
-    if (msg->getHeaders().getSingleOrEmpty(proxygen::HTTP_HEADER_EXPECT) ==
-        "100-continue") {
+    if (msg->getHeaders().getSingleOrEmpty("Expect") == "100-continue") {
       resp.setStatusCode(100);
       resp.setStatusMessage("Continue");
       maybeAddAltSvcHeader(resp);

proxygen/lib/http/HTTPCommonHeaders.txt

@@ -5,18 +5,13 @@
 :Status
 :Protocol
 Accept
-Accept-Charset
 Accept-Encoding
 Accept-Language
 Accept-Ranges
 Access-Control-Allow-Credentials
-Access-Control-Allow-Headers
 Access-Control-Allow-Methods
 Access-Control-Allow-Origin
 Access-Control-Expose-Headers
-Access-Control-Max-Age
-Access-Control-Request-Headers
-Access-Control-Request-Method
 Age
 Alt-Svc
 Authorization
@@ -39,18 +34,14 @@ Date
 document-policy
 Edge-Control
 ETag
-Expect
 Expires
-facebook-api-version
 Host
 If-Modified-Since
 If-None-Match
 Keep-Alive
 Last-Modified
-Link
 Location
-Origin
-origin-trial
+no-vary-search
 permissions-policy
 Pragma
 Priority
@@ -62,15 +53,14 @@ queue_timeout
 Range
 Referer
 report-to
-Retry-After
+reporting-endpoints
 rpckind
-Sec-WebSocket-Key
 Sec-WebSocket-Accept
+Sec-WebSocket-Key
 Server
 Set-Cookie
 Strict-Transport-Security
 TE
-thrift_priority
 timing-allow-origin
 Trailer
 Transfer-Encoding
@@ -78,10 +68,8 @@ Upgrade
 User-Agent
 Vary
 Via
-WWW-Authenticate
 X-Content-Type-Options
 X-Forwarded-For
-X-Forwarded-Proto
 X-Frame-Options
-X-Thrift-Protocol
+x-robots-tag
 X-XSS-Protection

proxygen/lib/http/codec/compress/test/HPACKContextTests.cpp

@@ -75,6 +75,9 @@ TEST_F(HPACKContextTests, StaticTable) {
 
 TEST_F(HPACKContextTests, StaticTableHeaderNamesAreCommon) {
   auto& table = StaticHeaderTable::get();
+  // Static table entries that are not in the common header list. A header
+  // dropped from that list keeps working here, it just loses the O(1)
+  // lookup, so the trade is worth it once the header is rare enough.
   std::set<std::string> uncommonStaticEntries{"allow",
                                               "content-location",
                                               "from",
@@ -82,7 +85,12 @@ TEST_F(HPACKContextTests, StaticTableHeaderNamesAreCommon) {
                                               "if-unmodified-since",
                                               "max-forwards",
                                               "if-range",
-                                              "refresh"};
+                                              "refresh",
+                                              "accept-charset",
+                                              "expect",
+                                              "link",
+                                              "retry-after",
+                                              "www-authenticate"};
   for (const auto& [header, _] : table.names()) {
     EXPECT_TRUE(header.isCommonHeader() ||
                 uncommonStaticEntries.find(header.get()) !=

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

@@ -17,6 +17,13 @@
 #include <proxygen/lib/http/codec/test/MockHTTPCodec.h>
 #include <proxygen/lib/http/codec/test/TestUtils.h>
 
+#include <string_view>
+
+namespace {
+// Not in the common header list, so addressed by name.
+constexpr std::string_view kExpect{"Expect"};
+} // namespace
+
 using namespace proxygen;
 using namespace std;
 using namespace testing;
@@ -1764,7 +1771,7 @@ TEST(HTTP1xCodecTest, Chunkify100) {
   req.setHTTPVersion(1, 1);
   req.setURL("/");
   req.setMethod(HTTPMethod::GET);
-  req.getHeaders().add(HTTP_HEADER_EXPECT, "100-continue");
+  req.getHeaders().add(kExpect, "100-continue");
   HTTPCodec::StreamID id = upCodec.createStream();
   upCodec.generateHeader(buf, id, req, false);
 

proxygen/lib/http/coro/server/handlers/ExpectContinueWrapperHandler.cpp

@@ -8,11 +8,15 @@
 
 #include "proxygen/lib/http/coro/server/handlers/ExpectContinueWrapperHandler.h"
 #include "proxygen/lib/http/coro/HTTPFixedSource.h"
-#include <proxygen/lib/http/HTTPCommonHeaders.h>
 
 #include <folly/logging/xlog.h>
 #include <string_view>
 
+namespace {
+// Not in the common header list, so addressed by name.
+constexpr std::string_view kExpect{"Expect"};
+} // namespace
+
 namespace proxygen::coro {
 
 constexpr folly::StringPiece k100ContinueExpectation = "100-continue";
@@ -72,15 +76,15 @@ folly::coro::Task<HTTPSourceHolder> ExpectContinueWrapperHandler::handleRequest(
   auto headerEvent = co_await requestSource.readHeaderEvent();
   XCHECK(headerEvent.headers);
   auto headers = headerEvent.headers->getHeaders();
-  if (headers.exists(HTTP_HEADER_EXPECT)) {
-    auto& expectVal = headers.getSingleOrEmpty(HTTP_HEADER_EXPECT);
+  if (headers.exists(kExpect)) {
+    auto& expectVal = headers.getSingleOrEmpty(kExpect);
     if (k100ContinueExpectation.equals(expectVal,
                                        folly::AsciiCaseInsensitive{})) {
       // Request has an expectation with the value 100-continue
       // We need to wrap the next handler in an ExpectContinueWrapperResponse
       // to inject the 100 Continue response
       XLOG(DBG8) << "ExpectContinueWrapper found 100-continue expectation";
-      headerEvent.headers->getHeaders().remove(HTTP_HEADER_EXPECT);
+      headerEvent.headers->getHeaders().remove(kExpect);
       auto expectWrapperResponse =
           new ExpectContinueWrapperResponse(evb,
                                             ctx,

proxygen/lib/http/coro/server/handlers/test/ExpectContinueWrapperHandlerTest.cpp

@@ -13,6 +13,13 @@
 #include <proxygen/lib/http/coro/HTTPFixedSource.h>
 #include <proxygen/lib/http/coro/test/Mocks.h>
 
+#include <string_view>
+
+namespace {
+// Not in the common header list, so addressed by name.
+constexpr std::string_view kExpect{"Expect"};
+} // namespace
+
 using namespace testing;
 using namespace proxygen;
 using namespace proxygen::coro;
@@ -41,7 +48,7 @@ CO_TEST_F(ExpectContinueWrapperHandlerTest, SendContinueResponse) {
   auto request = std::make_unique<HTTPMessage>();
   request->setMethod("POST");
   request->setURL("https://test.facebook.com/continue");
-  request->getHeaders().add(HTTP_HEADER_EXPECT, "100-continue");
+  request->getHeaders().add(kExpect, "100-continue");
   request->getHeaders().add(HTTP_HEADER_HOST, "test.facebook.com");
   auto requestSource = std::make_unique<HTTPFixedSource>(std::move(request));
   HTTPSourceHolder requestSourceHolder(requestSource.get());
@@ -71,7 +78,7 @@ CO_TEST_F(ExpectContinueWrapperHandlerTest, SendContinueResponse) {
             auto propagatedHeader =
                 co_await requestSourceHolder.readHeaderEvent();
             EXPECT_EQ(propagatedHeader.headers->getHeaders().getSingleOrEmpty(
-                          HTTP_HEADER_EXPECT),
+                          kExpect),
                       "");
             EXPECT_EQ(propagatedHeader.headers->getHeaders().getSingleOrEmpty(
                           HTTP_HEADER_HOST),
@@ -130,7 +137,7 @@ CO_TEST_F(ExpectContinueWrapperHandlerTest, InvalidExpectation) {
   auto request = std::make_unique<HTTPMessage>();
   request->setMethod("POST");
   request->setURL("https://test.facebook.com/continue");
-  request->getHeaders().add(HTTP_HEADER_EXPECT, "blah");
+  request->getHeaders().add(kExpect, "blah");
   auto requestSource = std::make_unique<HTTPFixedSource>(std::move(request));
   HTTPSourceHolder requestSourceHolder(requestSource.get());
 
@@ -162,7 +169,7 @@ CO_TEST_F(ExpectContinueWrapperHandlerTest,
   auto request = std::make_unique<HTTPMessage>();
   request->setMethod("POST");
   request->setURL("https://test.facebook.com/continue");
-  request->getHeaders().add(HTTP_HEADER_EXPECT, "100-continue");
+  request->getHeaders().add(kExpect, "100-continue");
   auto requestSource = std::make_unique<HTTPFixedSource>(std::move(request));
   HTTPSourceHolder requestSourceHolder(requestSource.get());
 
@@ -191,7 +198,7 @@ CO_TEST_F(ExpectContinueWrapperHandlerTest,
   auto request = std::make_unique<HTTPMessage>();
   request->setMethod("POST");
   request->setURL("https://test.facebook.com/continue");
-  request->getHeaders().add(HTTP_HEADER_EXPECT, "100-continue");
+  request->getHeaders().add(kExpect, "100-continue");
   auto body = folly::IOBuf::copyBuffer("REQUEST BODY");
   auto requestSource =
       std::make_unique<HTTPFixedSource>(std::move(request), std::move(body));
@@ -221,7 +228,7 @@ CO_TEST_F(ExpectContinueWrapperHandlerTest, StopReadingAfterInvoke) {
   auto request = std::make_unique<HTTPMessage>();
   request->setMethod("POST");
   request->setURL("https://test.facebook.com/continue");
-  request->getHeaders().add(HTTP_HEADER_EXPECT, "100-continue");
+  request->getHeaders().add(kExpect, "100-continue");
   auto requestSource = std::make_unique<HTTPFixedSource>(std::move(request));
   HTTPSourceHolder requestSourceHolder(requestSource.get());
 

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

@@ -28,6 +28,15 @@
 #include <folly/futures/Future.h>
 #include <folly/portability/GTest.h>
 
+#include <cstddef>
+#include <string_view>
+#include <utility>
+
+namespace {
+// Not in the common header list, so addressed by name.
+constexpr std::string_view kExpect{"Expect"};
+} // namespace
+
 using namespace proxygen;
 using namespace proxygen::hq;
 using namespace quic;
@@ -1618,7 +1627,7 @@ TEST_P(HQDownstreamSessionTest, SendOnFlowControlPaused) {
 
 TEST_P(HQDownstreamSessionTest, Http_100Continue) {
   auto req = getPostRequest(100);
-  req.getHeaders().add(HTTP_HEADER_EXPECT, "100-continue");
+  req.getHeaders().add(kExpect, "100-continue");
   auto id = sendRequest(req, false);
   auto handler = addSimpleStrictHandler();
   handler->expectHeaders([&handler] {

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

@@ -28,6 +28,14 @@
 #include <quic/api/test/MockQuicSocket.h>
 #include <quic/priority/HTTPPriorityQueue.h>
 
+#include <string_view>
+#include <utility>
+
+namespace {
+// Not in the common header list, so addressed by name.
+constexpr std::string_view kExpect{"Expect"};
+} // namespace
+
 using namespace proxygen;
 using namespace proxygen::test;
 using namespace proxygen::hq;
@@ -552,7 +560,7 @@ TEST_P(HQUpstreamSessionTest, Test100Continue) {
   InSequence enforceOrder;
   auto handler = openTransaction();
   auto req = getPostRequest(10);
-  req.getHeaders().add(HTTP_HEADER_EXPECT, "100-continue");
+  req.getHeaders().add(kExpect, "100-continue");
   handler->txn_->sendHeaders(req);
   handler->txn_->sendEOM();
   handler->expectHeaders();

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

@@ -27,6 +27,14 @@
 #include <proxygen/lib/test/TestAsyncTransport.h>
 #include <wangle/acceptor/ConnectionManager.h>
 
+#include <string_view>
+#include <utility>
+
+namespace {
+// Not in the common header list, so addressed by name.
+constexpr std::string_view kExpect{"Expect"};
+} // namespace
+
 using folly::test::MockAsyncTransport;
 
 using namespace proxygen;
@@ -1239,7 +1247,7 @@ TEST_F(HTTPUpstreamSessionTest, 100ContinueKeepalive) {
   // Test a request with 100 continue on a keepalive connection. Then make
   // another request.
   HTTPMessage req = getGetRequest();
-  req.getHeaders().set(HTTP_HEADER_EXPECT, "100-continue");
+  req.getHeaders().set(kExpect, "100-continue");
 
   InSequence enforceOrder;
 
@@ -1274,7 +1282,7 @@ TEST_F(HTTPUpstreamSessionTest, 417Keepalive) {
   // Test a request with 100 continue on a keepalive connection. Then make
   // another request after the expectation fails.
   HTTPMessage req = getGetRequest();
-  req.getHeaders().set(HTTP_HEADER_EXPECT, "100-continue");
+  req.getHeaders().set(kExpect, "100-continue");
 
   InSequence enforceOrder;