Merge pull request #1347 from nghttp2/nghttpx-ignore-cl-te-on-upgrade

Ignore Content-Length and Transfer-Encoding in 200 response to CONNECT request

lib/nghttp2_http.c

@@ -263,11 +263,14 @@ static int http_response_on_header(nghttp2_stream *stream, nghttp2_hd_nv *nv,
       stream->content_length = 0;
       return NGHTTP2_ERR_REMOVE_HTTP_HEADER;
     }
-    if (stream->status_code / 100 == 1 ||
+    if (stream->status_code / 100 == 1) {
-        (stream->status_code / 100 == 2 &&
-         (stream->http_flags & NGHTTP2_HTTP_FLAG_METH_CONNECT))) {
       return NGHTTP2_ERR_HTTP_HEADER;
     }
+    /* https://tools.ietf.org/html/rfc7230#section-3.3.3 */
+    if (stream->status_code / 100 == 2 &&
+        (stream->http_flags & NGHTTP2_HTTP_FLAG_METH_CONNECT)) {
+      return NGHTTP2_ERR_REMOVE_HTTP_HEADER;
+    }
     if (stream->content_length != -1) {
       return NGHTTP2_ERR_HTTP_HEADER;
     }

src/shrpx_downstream.cc

@@ -573,6 +573,18 @@ void FieldStore::append_last_trailer_value(const char *data, size_t len) {
                                   trailers_, data, len);
 }
 
+void FieldStore::erase_content_length_and_transfer_encoding() {
+  for (auto &kv : headers_) {
+    switch (kv.token) {
+    case http2::HD_CONTENT_LENGTH:
+    case http2::HD_TRANSFER_ENCODING:
+      kv.name = StringRef{};
+      kv.token = -1;
+      break;
+    }
+  }
+}
+
 void Downstream::set_request_start_time(
     std::chrono::high_resolution_clock::time_point time) {
   request_start_time_ = std::move(time);

src/shrpx_downstream.h

@@ -119,6 +119,10 @@ public:
 
   bool trailer_key_prev() const { return trailer_key_prev_; }
 
+  // erase_content_length_and_transfer_encoding erases content-length
+  // and transfer-encoding header fields.
+  void erase_content_length_and_transfer_encoding();
+
   // content-length, -1 if it is unknown.
   int64_t content_length;
 

src/shrpx_http_downstream_connection.cc

@@ -935,18 +935,15 @@ int htp_hdrs_completecb(llhttp_t *htp) {
       return -1;
     }
     if (resp.fs.content_length == 0) {
-      auto cl = resp.fs.header(http2::HD_CONTENT_LENGTH);
+      resp.fs.erase_content_length_and_transfer_encoding();
-      assert(cl);
-      http2::erase_header(cl);
     } else if (resp.fs.content_length != -1) {
       return -1;
     }
   } else if (resp.http_status / 100 == 1 ||
              (resp.http_status / 100 == 2 && req.method == HTTP_CONNECT)) {
-    if (resp.fs.header(http2::HD_CONTENT_LENGTH) ||
+    // Server MUST NOT send Content-Length and Transfer-Encoding in
-        resp.fs.header(http2::HD_TRANSFER_ENCODING)) {
+    // these responses.
-      return -1;
+    resp.fs.erase_content_length_and_transfer_encoding();
-    }
   } else if (resp.fs.parse_content_length() != 0) {
     downstream->set_response_state(DownstreamState::MSG_BAD_HEADER);
     return -1;

tests/nghttp2_session_test.c

@@ -11762,6 +11762,8 @@ void test_nghttp2_http_ignore_content_length(void) {
   const nghttp2_nv conn_reqnv[] = {MAKE_NV(":authority", "localhost"),
                                    MAKE_NV(":method", "CONNECT"),
                                    MAKE_NV("content-length", "999999")};
+  const nghttp2_nv conn_cl_resnv[] = {MAKE_NV(":status", "200"),
+                                      MAKE_NV("content-length", "0")};
   nghttp2_stream *stream;
 
   mem = nghttp2_mem_default();
@@ -11791,6 +11793,24 @@ void test_nghttp2_http_ignore_content_length(void) {
 
   nghttp2_bufs_reset(&bufs);
 
+  /* Content-Length in 200 response to CONNECT is ignored */
+  stream = open_sent_stream2(session, 3, NGHTTP2_STREAM_OPENING);
+  stream->http_flags |= NGHTTP2_HTTP_FLAG_METH_CONNECT;
+
+  rv = pack_headers(&bufs, &deflater, 3, NGHTTP2_FLAG_END_HEADERS,
+                    conn_cl_resnv, ARRLEN(conn_cl_resnv), mem);
+  CU_ASSERT(0 == rv);
+
+  rv = nghttp2_session_mem_recv(session, bufs.head->buf.pos,
+                                nghttp2_buf_len(&bufs.head->buf));
+
+  CU_ASSERT((ssize_t)nghttp2_buf_len(&bufs.head->buf) == rv);
+
+  CU_ASSERT(NULL == nghttp2_session_get_next_ob_item(session));
+  CU_ASSERT(-1 == stream->content_length);
+
+  nghttp2_bufs_reset(&bufs);
+
   nghttp2_hd_deflate_free(&deflater);
   nghttp2_session_del(session);
 
@@ -11866,13 +11886,10 @@ void test_nghttp2_http_record_request_method(void) {
   CU_ASSERT((NGHTTP2_HTTP_FLAG_METH_CONNECT & stream->http_flags) > 0);
   CU_ASSERT(-1 == stream->content_length);
 
-  /* content-length is now allowed in 200 response to a CONNECT
+  /* content-length is ignored in 200 response to a CONNECT request */
-     request */
   item = nghttp2_session_get_next_ob_item(session);
 
-  CU_ASSERT(NULL != item);
+  CU_ASSERT(NULL == item);
-  CU_ASSERT(NGHTTP2_RST_STREAM == item->frame.hd.type);
-  CU_ASSERT(NGHTTP2_PROTOCOL_ERROR == item->frame.rst_stream.error_code);
 
   nghttp2_hd_deflate_free(&deflater);
   nghttp2_session_del(session);