Merge pull request #736 from nghttp2/accept-204-content-length-0

Accept content-length: 0 in 204 response

lib/nghttp2_http.c

@@ -250,7 +250,21 @@ static int http_response_on_header(nghttp2_stream *stream, nghttp2_hd_nv *nv,
     break;
   }
   case NGHTTP2_TOKEN_CONTENT_LENGTH: {
-    if (stream->status_code == 204 || stream->status_code / 100 == 1 ||
+    if (stream->status_code == 204) {
+      /* content-length header field in 204 response is prohibited by
+         RFC 7230.  But some widely used servers send content-length:
+         0.  Until they get fixed, we ignore it. */
+      if (stream->content_length != -1) {
+        /* Found multiple content-length field */
+        return NGHTTP2_ERR_HTTP_HEADER;
+      }
+      if (!lstrieq("0", nv->value->base, nv->value->len)) {
+        return NGHTTP2_ERR_HTTP_HEADER;
+      }
+      stream->content_length = 0;
+      return NGHTTP2_ERR_REMOVE_HTTP_HEADER;
+    }
+    if (stream->status_code / 100 == 1 ||
         (stream->status_code == 200 &&
          (stream->http_flags & NGHTTP2_HTTP_FLAG_METH_CONNECT))) {
       return NGHTTP2_ERR_HTTP_HEADER;

lib/nghttp2_int.h

@@ -46,7 +46,13 @@ typedef enum {
    * Invalid HTTP header field was received but it can be treated as
    * if it was not received because of compatibility reasons.
    */
-  NGHTTP2_ERR_IGN_HTTP_HEADER = -105
+  NGHTTP2_ERR_IGN_HTTP_HEADER = -105,
+  /*
+   * Invalid HTTP header field was received, and it is ignored.
+   * Unlike NGHTTP2_ERR_IGN_HTTP_HEADER, this does not invoke
+   * nghttp2_on_invalid_header_callback.
+   */
+  NGHTTP2_ERR_REMOVE_HTTP_HEADER = -106
 } nghttp2_internal_error;
 
 #endif /* NGHTTP2_INT_H */

src/http2.cc

@@ -477,6 +477,11 @@ void dump_nv(FILE *out, const HeaderRefs &nva) {
   fflush(out);
 }
 
+void erase_header(HeaderRef *hd) {
+  hd->name = StringRef{};
+  hd->token = -1;
+}
+
 StringRef rewrite_location_uri(BlockAllocator &balloc, const StringRef &uri,
                                const http_parser_url &u,
                                const StringRef &match_host,

src/http2.h

@@ -227,6 +227,9 @@ void dump_nv(FILE *out, const Headers &nva);
 
 void dump_nv(FILE *out, const HeaderRefs &nva);
 
+// Ereases header in |hd|.
+void erase_header(HeaderRef *hd);
+
 // Rewrites redirection URI which usually appears in location header
 // field. The |uri| is the URI in the location header field. The |u|
 // stores the result of parsed |uri|. The |request_authority| is the

src/shrpx_http_downstream_connection.cc

@@ -720,15 +720,33 @@ int htp_hdrs_completecb(http_parser *htp) {
   // 204.  Also server MUST NOT send Transfer-Encoding with a status
   // code 200 to a CONNECT request.  Same holds true with
   // Content-Length.
-  if (resp.http_status == 204 || resp.http_status / 100 == 1 ||
+  if (resp.http_status == 204) {
-      (resp.http_status == 200 && req.method == HTTP_CONNECT)) {
+    if (resp.fs.header(http2::HD_TRANSFER_ENCODING)) {
+      return -1;
+    }
+    // Some server send content-length: 0 for 204.  Until they get
+    // fixed, we accept, but ignore it.
+
+    // Calling parse_content_length() detects duplicated
+    // content-length header fields.
+    if (resp.fs.parse_content_length() != 0) {
+      return -1;
+    }
+    if (resp.fs.content_length != 0) {
+      return -1;
+    }
+    if (resp.fs.content_length == 0) {
+      auto cl = resp.fs.header(http2::HD_CONTENT_LENGTH);
+      assert(cl);
+      http2::erase_header(cl);
+    }
+  } else if (resp.http_status / 100 == 1 ||
+             (resp.http_status == 200 && req.method == HTTP_CONNECT)) {
     if (resp.fs.header(http2::HD_CONTENT_LENGTH) ||
         resp.fs.header(http2::HD_TRANSFER_ENCODING)) {
       return -1;
     }
-  }
+  } else if (resp.fs.parse_content_length() != 0) {
-
-  if (resp.fs.parse_content_length() != 0) {
     downstream->set_response_state(Downstream::MSG_BAD_HEADER);
     return -1;
   }

src/shrpx_mruby_module_response.cc

@@ -233,7 +233,7 @@ mrb_value response_return(mrb_state *mrb, mrb_value self) {
       (resp.http_status == 200 && req.method == HTTP_CONNECT)) {
     if (cl) {
       // Delete content-length here
-      cl->name = StringRef{};
+      http2::erase_header(cl);
     }
 
     resp.fs.content_length = -1;

tests/nghttp2_session_test.c

@@ -10204,6 +10204,8 @@ void test_nghttp2_http_mandatory_headers(void) {
                                     MAKE_NV("content-length", "0")};
   const nghttp2_nv cl204_resnv[] = {MAKE_NV(":status", "204"),
                                     MAKE_NV("content-length", "0")};
+  const nghttp2_nv clnonzero204_resnv[] = {MAKE_NV(":status", "204"),
+                                           MAKE_NV("content-length", "100")};
 
   /* test case for request */
   const nghttp2_nv nopath_reqnv[] = {MAKE_NV(":scheme", "https"),
@@ -10307,10 +10309,16 @@ void test_nghttp2_http_mandatory_headers(void) {
                                        NGHTTP2_STREAM_OPENING, cl1xx_resnv,
                                        ARRLEN(cl1xx_resnv));
 
-  /* response header has content-length with 204 status code */
+  /* response header has 0 content-length with 204 status code */
-  check_nghttp2_http_recv_headers_fail(session, &deflater, 19,
+  check_nghttp2_http_recv_headers_ok(session, &deflater, 19,
-                                       NGHTTP2_STREAM_OPENING, cl204_resnv,
+                                     NGHTTP2_STREAM_OPENING, cl204_resnv,
-                                       ARRLEN(cl204_resnv));
+                                     ARRLEN(cl204_resnv));
+
+  /* response header has nonzero content-length with 204 status
+     code */
+  check_nghttp2_http_recv_headers_fail(
+      session, &deflater, 21, NGHTTP2_STREAM_OPENING, clnonzero204_resnv,
+      ARRLEN(clnonzero204_resnv));
 
   nghttp2_hd_deflate_free(&deflater);