From 5645cad57720a9954b1bcf58397b1da06945c8ed Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 3 Dec 2016 14:57:16 +0900 Subject: [PATCH 1/2] Accept and ignore content-length: 0 in 204 response for now --- lib/nghttp2_http.c | 16 +++++++++++++++- lib/nghttp2_int.h | 8 +++++++- tests/nghttp2_session_test.c | 16 ++++++++++++---- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/lib/nghttp2_http.c b/lib/nghttp2_http.c index 23db1cbc..877f5e35 100644 --- a/lib/nghttp2_http.c +++ b/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; diff --git a/lib/nghttp2_int.h b/lib/nghttp2_int.h index 9ea8c820..30cf7274 100644 --- a/lib/nghttp2_int.h +++ b/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 */ diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 064adbf9..696ec394 100644 --- a/tests/nghttp2_session_test.c +++ b/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 */ - check_nghttp2_http_recv_headers_fail(session, &deflater, 19, - NGHTTP2_STREAM_OPENING, cl204_resnv, - ARRLEN(cl204_resnv)); + /* response header has 0 content-length with 204 status code */ + check_nghttp2_http_recv_headers_ok(session, &deflater, 19, + NGHTTP2_STREAM_OPENING, 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); From b6a9cf9ffaef37ce0a5400cd88e58ba1b393c27a Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 3 Dec 2016 14:57:48 +0900 Subject: [PATCH 2/2] nghttpx: Accept and ignore content-length: 0 in 204 response for now --- src/http2.cc | 5 +++++ src/http2.h | 3 +++ src/shrpx_http_downstream_connection.cc | 28 ++++++++++++++++++++----- src/shrpx_mruby_module_response.cc | 2 +- 4 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/http2.cc b/src/http2.cc index 38d74a3e..04dfb7ad 100644 --- a/src/http2.cc +++ b/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, diff --git a/src/http2.h b/src/http2.h index 5ebe11f1..9859930b 100644 --- a/src/http2.h +++ b/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 diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc index 2519d109..16435a84 100644 --- a/src/shrpx_http_downstream_connection.cc +++ b/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 || - (resp.http_status == 200 && req.method == HTTP_CONNECT)) { + if (resp.http_status == 204) { + 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; } - } - - if (resp.fs.parse_content_length() != 0) { + } else if (resp.fs.parse_content_length() != 0) { downstream->set_response_state(Downstream::MSG_BAD_HEADER); return -1; } diff --git a/src/shrpx_mruby_module_response.cc b/src/shrpx_mruby_module_response.cc index 145def0e..7e263804 100644 --- a/src/shrpx_mruby_module_response.cc +++ b/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;