From 489b4f307c8b3cd6152078ff89276bec268a08be Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 20 Feb 2015 00:58:20 +0900 Subject: [PATCH] nghttp, nghttpd, nghttpx: Remove validations libnghttp2 offers --- src/HttpServer.cc | 61 +--------------------------- src/HttpServer.h | 1 - src/nghttp.cc | 51 ++--------------------- src/shrpx_http2_session.cc | 57 ++------------------------ src/shrpx_http2_upstream.cc | 81 ++++--------------------------------- 5 files changed, 15 insertions(+), 236 deletions(-) diff --git a/src/HttpServer.cc b/src/HttpServer.cc index d3e5c55c..862de3c8 100644 --- a/src/HttpServer.cc +++ b/src/HttpServer.cc @@ -247,8 +247,7 @@ private: }; Stream::Stream(Http2Handler *handler, int32_t stream_id) - : handler(handler), body_left(0), upload_left(-1), stream_id(stream_id), - file(-1) { + : handler(handler), body_left(0), stream_id(stream_id), file(-1) { auto config = handler->get_config(); ev_timer_init(&rtimer, stream_timeout_cb, 0., config->stream_read_timeout); ev_timer_init(&wtimer, stream_timeout_cb, 0., config->stream_write_timeout); @@ -1021,44 +1020,8 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, return 0; } - if (!http2::check_nv(name, namelen, value, valuelen)) { - hd->submit_rst_stream(stream, NGHTTP2_PROTOCOL_ERROR); - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } - auto token = http2::lookup_token(name, namelen); - if (name[0] == ':') { - if ((!stream->headers.empty() && - stream->headers.back().name.c_str()[0] != ':') || - !http2::check_http2_request_pseudo_header(stream->hdidx, token)) { - - hd->submit_rst_stream(stream, NGHTTP2_PROTOCOL_ERROR); - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } - } - - if (!http2::http2_header_allowed(token)) { - hd->submit_rst_stream(stream, NGHTTP2_PROTOCOL_ERROR); - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } - - switch (token) { - case http2::HD_CONTENT_LENGTH: { - auto upload_left = util::parse_uint(value, valuelen); - if (upload_left != -1) { - stream->upload_left = upload_left; - } - break; - } - case http2::HD_TE: - if (!util::strieq("trailers", value, valuelen)) { - hd->submit_rst_stream(stream, NGHTTP2_PROTOCOL_ERROR); - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } - break; - } - http2::index_header(stream->hdidx, token, stream->headers.size()); http2::add_header(stream->headers, name, namelen, value, valuelen, flags & NGHTTP2_NV_FLAG_NO_INDEX, token); @@ -1104,10 +1067,6 @@ int hd_on_frame_recv_callback(nghttp2_session *session, if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { remove_stream_read_timeout(stream); - if (stream->upload_left > 0) { - hd->submit_rst_stream(stream, NGHTTP2_PROTOCOL_ERROR); - return 0; - } if (!hd->get_config()->early_response) { prepare_response(stream, hd); } @@ -1125,11 +1084,6 @@ int hd_on_frame_recv_callback(nghttp2_session *session, if (frame->headers.cat == NGHTTP2_HCAT_REQUEST) { - if (!http2::http2_mandatory_request_headers_presence(stream->hdidx)) { - hd->submit_rst_stream(stream, NGHTTP2_PROTOCOL_ERROR); - return 0; - } - auto expect100 = http2::get_header(stream->hdidx, http2::HD_EXPECT, stream->headers); @@ -1144,10 +1098,6 @@ int hd_on_frame_recv_callback(nghttp2_session *session, if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { remove_stream_read_timeout(stream); - if (stream->upload_left > 0) { - hd->submit_rst_stream(stream, NGHTTP2_PROTOCOL_ERROR); - return 0; - } if (!hd->get_config()->early_response) { prepare_response(stream, hd); } @@ -1244,15 +1194,6 @@ int on_data_chunk_recv_callback(nghttp2_session *session, uint8_t flags, // TODO Handle POST - if (stream->upload_left != -1) { - if (stream->upload_left < static_cast(len)) { - stream->upload_left = -1; - hd->submit_rst_stream(stream, NGHTTP2_PROTOCOL_ERROR); - return 0; - } - stream->upload_left -= len; - } - add_stream_read_timeout(stream); return 0; diff --git a/src/HttpServer.h b/src/HttpServer.h index 222daeb7..54b7abaf 100644 --- a/src/HttpServer.h +++ b/src/HttpServer.h @@ -81,7 +81,6 @@ struct Stream { ev_timer rtimer; ev_timer wtimer; int64_t body_left; - int64_t upload_left; int32_t stream_id; int file; http2::HeaderIndex hdidx; diff --git a/src/nghttp.cc b/src/nghttp.cc index 986bf59c..dcc69b52 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -1505,12 +1505,6 @@ int on_data_chunk_recv_callback(nghttp2_session *session, uint8_t flags, user_data); } - if (req->expect_final_response) { - nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, stream_id, - NGHTTP2_PROTOCOL_ERROR); - return 0; - } - req->response_len += len; if (req->inflater) { @@ -1667,12 +1661,6 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, flags, user_data); } - if (!http2::check_nv(name, namelen, value, valuelen)) { - nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, frame->hd.stream_id, - NGHTTP2_PROTOCOL_ERROR); - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } - switch (frame->hd.type) { case NGHTTP2_HEADERS: { auto req = static_cast( @@ -1682,23 +1670,14 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, break; } - if (frame->headers.cat != NGHTTP2_HCAT_RESPONSE && - frame->headers.cat != NGHTTP2_HCAT_PUSH_RESPONSE && - (frame->headers.cat != NGHTTP2_HCAT_HEADERS || - !req->expect_final_response)) { + /* ignore trailer header */ + if (frame->headers.cat == NGHTTP2_HCAT_HEADERS && + !req->expect_final_response) { break; } auto token = http2::lookup_token(name, namelen); - if (name[0] == ':') { - if (!req->response_pseudo_header_allowed(token)) { - nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, - frame->hd.stream_id, NGHTTP2_PROTOCOL_ERROR); - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } - } - http2::index_header(req->res_hdidx, token, req->res_nva.size()); http2::add_header(req->res_nva, name, namelen, value, valuelen, flags & NGHTTP2_NV_FLAG_NO_INDEX, token); @@ -1714,15 +1693,6 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, auto token = http2::lookup_token(name, namelen); - if (name[0] == ':') { - if (!req->push_request_pseudo_header_allowed(token)) { - nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, - frame->push_promise.promised_stream_id, - NGHTTP2_PROTOCOL_ERROR); - break; - } - } - http2::index_header(req->req_hdidx, token, req->req_nva.size()); http2::add_header(req->req_nva, name, namelen, value, valuelen, flags & NGHTTP2_NV_FLAG_NO_INDEX, token); @@ -1752,12 +1722,6 @@ int on_frame_recv_callback2(nghttp2_session *session, ; } - if (req->expect_final_response) { - nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, frame->hd.stream_id, - NGHTTP2_PROTOCOL_ERROR); - return 0; - } - if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { req->record_response_end_time(); } @@ -1795,11 +1759,6 @@ int on_frame_recv_callback2(nghttp2_session *session, } if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { - if (req->expect_final_response) { - nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, - frame->hd.stream_id, NGHTTP2_PROTOCOL_ERROR); - return 0; - } req->record_response_end_time(); } @@ -1826,9 +1785,7 @@ int on_frame_recv_callback2(nghttp2_session *session, authority = req->get_req_header(http2::HD_HOST); } - if (!scheme || !authority || !method || !path || scheme->value.empty() || - authority->value.empty() || method->value.empty() || - path->value.empty() || path->value[0] != '/') { + if (!scheme || !authority || !method || !path || path->value[0] != '/') { nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, frame->push_promise.promised_stream_id, NGHTTP2_PROTOCOL_ERROR); diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index f0af34b2..2d08e7f6 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -692,7 +692,6 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, const uint8_t *name, size_t namelen, const uint8_t *value, size_t valuelen, uint8_t flags, void *user_data) { - auto http2session = static_cast(user_data); auto sd = static_cast( nghttp2_session_get_stream_user_data(session, frame->hd.stream_id)); if (!sd || !sd->dconn) { @@ -716,40 +715,12 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, } return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; } - if (!http2::check_nv(name, namelen, value, valuelen)) { - return 0; - } auto token = http2::lookup_token(name, namelen); - if (name[0] == ':') { - if (!downstream->response_pseudo_header_allowed(token)) { - http2session->submit_rst_stream(frame->hd.stream_id, - NGHTTP2_PROTOCOL_ERROR); - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } - } - - if (!http2::http2_header_allowed(token)) { - http2session->submit_rst_stream(frame->hd.stream_id, - NGHTTP2_PROTOCOL_ERROR); - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } - if (token == http2::HD_CONTENT_LENGTH) { + // libnghttp2 guarantees this can be parsed auto len = util::parse_uint(value, valuelen); - if (len == -1) { - http2session->submit_rst_stream(frame->hd.stream_id, - NGHTTP2_PROTOCOL_ERROR); - downstream->set_response_state(Downstream::MSG_BAD_HEADER); - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } - if (downstream->get_response_content_length() != -1) { - http2session->submit_rst_stream(frame->hd.stream_id, - NGHTTP2_PROTOCOL_ERROR); - downstream->set_response_state(Downstream::MSG_BAD_HEADER); - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } downstream->set_response_content_length(len); } @@ -796,18 +767,8 @@ int on_response_headers(Http2Session *http2session, Downstream *downstream, downstream->set_expect_final_response(false); auto status = downstream->get_response_header(http2::HD__STATUS); - int status_code; - - if (!http2::non_empty_value(status) || - (status_code = http2::parse_http_status_code(status->value)) == -1) { - - http2session->submit_rst_stream(frame->hd.stream_id, - NGHTTP2_PROTOCOL_ERROR); - downstream->set_response_state(Downstream::MSG_RESET); - call_downstream_readcb(http2session, downstream); - - return 0; - } + // libnghttp2 guarantees this exists and can be parsed + auto status_code = http2::parse_http_status_code(status->value); downstream->set_response_http_status(status_code); downstream->set_response_major(2); @@ -912,12 +873,6 @@ int on_frame_recv_callback(nghttp2_session *session, const nghttp2_frame *frame, break; } - if (downstream->get_expect_final_response()) { - http2session->submit_rst_stream(frame->hd.stream_id, - NGHTTP2_PROTOCOL_ERROR); - break; - } - auto upstream = downstream->get_upstream(); rv = upstream->on_downstream_body(downstream, nullptr, 0, true); if (rv != 0) { @@ -977,12 +932,6 @@ int on_frame_recv_callback(nghttp2_session *session, const nghttp2_frame *frame, } if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { - if (downstream->get_expect_final_response()) { - http2session->submit_rst_stream(frame->hd.stream_id, - NGHTTP2_PROTOCOL_ERROR); - return 0; - } - downstream->disable_downstream_rtimer(); if (downstream->get_response_state() == Downstream::HEADER_COMPLETE) { diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 14e313e3..794beaec 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -180,49 +180,12 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, return 0; } - if (!http2::check_nv(name, namelen, value, valuelen)) { - upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } - auto token = http2::lookup_token(name, namelen); - if (name[0] == ':') { - if (!downstream->request_pseudo_header_allowed(token)) { - upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } - } - - if (!http2::http2_header_allowed(token)) { - upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } - - switch (token) { - case http2::HD_CONTENT_LENGTH: { + if (token == http2::HD_CONTENT_LENGTH) { + // libnghttp2 guarantees this can be parsed auto len = util::parse_uint(value, valuelen); - if (len == -1) { - if (upstream->error_reply(downstream, 400) != 0) { - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } - return 0; - } - if (downstream->get_request_content_length() != -1) { - if (upstream->error_reply(downstream, 400) != 0) { - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } - return 0; - } downstream->set_request_content_length(len); - break; - } - case http2::HD_TE: - if (!util::strieq("trailers", value, valuelen)) { - upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } - break; } downstream->add_request_header(name, namelen, value, valuelen, @@ -282,34 +245,19 @@ int on_request_headers(Http2Upstream *upstream, Downstream *downstream, http2::dump_nv(get_config()->http2_upstream_dump_request_header, nva); } - auto host = downstream->get_request_header(http2::HD_HOST); auto authority = downstream->get_request_header(http2::HD__AUTHORITY); auto path = downstream->get_request_header(http2::HD__PATH); auto method = downstream->get_request_header(http2::HD__METHOD); auto scheme = downstream->get_request_header(http2::HD__SCHEME); - bool is_connect = method && "CONNECT" == method->value; - bool having_host = http2::non_empty_value(host); + bool is_connect = "CONNECT" == method->value; bool having_authority = http2::non_empty_value(authority); - if (is_connect) { - // Here we strictly require :authority header field. - if (scheme || path || !having_authority) { - + // presence of mandatory header fields are guaranteed by libnghttp2. + if (!is_connect) { + // For HTTP/2 proxy, :authority is required. + if (get_config()->http2_proxy && !having_authority) { upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); - - return 0; - } - } else { - // For proxy, :authority is required. Otherwise, we can accept - // :authority or host for methods. - if (!http2::non_empty_value(method) || !http2::non_empty_value(scheme) || - (get_config()->http2_proxy && !having_authority) || - (!get_config()->http2_proxy && !having_authority && !having_host) || - !http2::non_empty_value(path)) { - - upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); - return 0; } } @@ -327,11 +275,6 @@ int on_request_headers(Http2Upstream *upstream, Downstream *downstream, downstream->set_request_state(Downstream::HEADER_COMPLETE); if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { - if (!downstream->validate_request_bodylen()) { - upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); - return 0; - } - downstream->disable_upstream_rtimer(); downstream->set_request_state(Downstream::MSG_COMPLETE); @@ -411,11 +354,6 @@ int on_frame_recv_callback(nghttp2_session *session, const nghttp2_frame *frame, if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { downstream->disable_upstream_rtimer(); - if (!downstream->validate_request_bodylen()) { - upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); - return 0; - } - downstream->end_upload_data(); downstream->set_request_state(Downstream::MSG_COMPLETE); } @@ -435,11 +373,6 @@ int on_frame_recv_callback(nghttp2_session *session, const nghttp2_frame *frame, } if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { - if (!downstream->validate_request_bodylen()) { - upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); - return 0; - } - downstream->disable_upstream_rtimer(); downstream->end_upload_data();