diff --git a/source/common/http/http1/BUILD b/source/common/http/http1/BUILD index d9b00a3171578..e57b43f0cd774 100644 --- a/source/common/http/http1/BUILD +++ b/source/common/http/http1/BUILD @@ -29,6 +29,7 @@ envoy_cc_library( "//source/common/buffer:buffer_lib", "//source/common/buffer:watermark_buffer_lib", "//source/common/common:assert_lib", + "//source/common/common:cleanup_lib", "//source/common/common:statusor_lib", "//source/common/common:utility_lib", "//source/common/http:codec_helper_lib", diff --git a/source/common/http/http1/codec_impl.cc b/source/common/http/http1/codec_impl.cc index d4aacd3a8ccc6..220f5711c2cb8 100644 --- a/source/common/http/http1/codec_impl.cc +++ b/source/common/http/http1/codec_impl.cc @@ -9,6 +9,7 @@ #include "envoy/http/header_map.h" #include "envoy/network/connection.h" +#include "common/common/cleanup.h" #include "common/common/enum_to_int.h" #include "common/common/utility.h" #include "common/http/exception.h" @@ -266,9 +267,10 @@ void ServerConnectionImpl::maybeAddSentinelBufferFragment(Buffer::WatermarkBuffe outbound_responses_++; } -void ServerConnectionImpl::doFloodProtectionChecks() const { +ConnectionImpl::HttpParserCode ServerConnectionImpl::doFloodProtectionChecks() { + ASSERT(dispatching_); if (!flood_protection_) { - return; + return HttpParserCode::Success; } // Before processing another request, make sure that we are below the response flood protection // threshold. @@ -276,8 +278,11 @@ void ServerConnectionImpl::doFloodProtectionChecks() const { ENVOY_CONN_LOG(trace, "error accepting request: too many pending responses queued", connection_); stats_.response_flood_.inc(); - throw FrameFloodException("Too many responses queued."); + ASSERT(codec_status_.ok()); + codec_status_ = Http::bufferFloodError("Too many responses queued."); + return HttpParserCode::Error; } + return HttpParserCode::Success; } void ConnectionImpl::flushOutput(bool end_encode) { @@ -391,8 +396,7 @@ void RequestEncoderImpl::encodeHeaders(const RequestHeaderMap& headers, bool end http_parser_settings ConnectionImpl::settings_{ [](http_parser* parser) -> int { - static_cast(parser->data)->onMessageBeginBase(); - return 0; + return enumToInt(static_cast(parser->data)->onMessageBeginBase()); }, [](http_parser* parser, const char* at, size_t length) -> int { static_cast(parser->data)->onUrl(at, length); @@ -400,23 +404,20 @@ http_parser_settings ConnectionImpl::settings_{ }, nullptr, // on_status [](http_parser* parser, const char* at, size_t length) -> int { - static_cast(parser->data)->onHeaderField(at, length); - return 0; + return enumToInt(static_cast(parser->data)->onHeaderField(at, length)); }, [](http_parser* parser, const char* at, size_t length) -> int { - static_cast(parser->data)->onHeaderValue(at, length); - return 0; + return enumToInt(static_cast(parser->data)->onHeaderValue(at, length)); }, [](http_parser* parser) -> int { - return static_cast(parser->data)->onHeadersCompleteBase(); + return enumToInt(static_cast(parser->data)->onHeadersCompleteBase()); }, [](http_parser* parser, const char* at, size_t length) -> int { static_cast(parser->data)->bufferBody(at, length); return 0; }, [](http_parser* parser) -> int { - static_cast(parser->data)->onMessageCompleteBase(); - return 0; + return enumToInt(static_cast(parser->data)->onMessageCompleteBase()); }, [](http_parser* parser) -> int { // A 0-byte chunk header is used to signal the end of the chunked body. @@ -444,19 +445,23 @@ ConnectionImpl::ConnectionImpl(Network::Connection& connection, Stats::Scope& st enable_trailers_(enable_trailers), reject_unsupported_transfer_encodings_(Runtime::runtimeFeatureEnabled( "envoy.reloadable_features.reject_unsupported_transfer_encodings")), - output_buffer_([&]() -> void { this->onBelowLowWatermark(); }, - [&]() -> void { this->onAboveHighWatermark(); }), + dispatching_(false), output_buffer_([&]() -> void { this->onBelowLowWatermark(); }, + [&]() -> void { this->onAboveHighWatermark(); }), max_headers_kb_(max_headers_kb), max_headers_count_(max_headers_count) { output_buffer_.setWatermarks(connection.bufferLimit()); http_parser_init(&parser_, type); parser_.data = this; } -void ConnectionImpl::completeLastHeader() { +ConnectionImpl::HttpParserCode ConnectionImpl::completeLastHeader() { + ASSERT(dispatching_); ENVOY_CONN_LOG(trace, "completed header: key={} value={}", connection_, current_header_field_.getStringView(), current_header_value_.getStringView()); - checkHeaderNameForUnderscores(); + if (!checkHeaderNameForUnderscores()) { + // This indicates that the request should be rejected due to header name with underscores. + return HttpParserCode::Error; + } auto& headers_or_trailers = headersOrTrailers(); if (!current_header_field_.empty()) { current_header_field_.inlineTransform([](char c) { return absl::ascii_tolower(c); }); @@ -474,12 +479,15 @@ void ConnectionImpl::completeLastHeader() { sendProtocolError(Http1ResponseCodeDetails::get().TooManyHeaders); const absl::string_view header_type = processing_trailers_ ? Http1HeaderTypes::get().Trailers : Http1HeaderTypes::get().Headers; - throw CodecProtocolException(absl::StrCat(header_type, " size exceeds limit")); + ASSERT(codec_status_.ok()); + codec_status_ = codecProtocolError(absl::StrCat(header_type, " size exceeds limit")); + return HttpParserCode::Error; } header_parsing_state_ = HeaderParsingState::Field; ASSERT(current_header_field_.empty()); ASSERT(current_header_value_.empty()); + return HttpParserCode::Success; } bool ConnectionImpl::maybeDirectDispatch(Buffer::Instance& data) { @@ -504,8 +512,13 @@ Http::Status ConnectionImpl::dispatch(Buffer::Instance& data) { Http::Status ConnectionImpl::innerDispatch(Buffer::Instance& data) { ENVOY_CONN_LOG(trace, "parsing {} bytes", connection_, data.length()); + // Make sure that dispatching_ is set to false after dispatching, even when + // ConnectionImpl::dispatch throws an exception. + Cleanup cleanup([this]() { dispatching_ = false; }); + ASSERT(!dispatching_); ASSERT(buffered_body_.length() == 0); + dispatching_ = true; if (maybeDirectDispatch(data)) { return Http::okStatus(); } @@ -516,7 +529,11 @@ Http::Status ConnectionImpl::innerDispatch(Buffer::Instance& data) { ssize_t total_parsed = 0; if (data.length() > 0) { for (const Buffer::RawSlice& slice : data.getRawSlices()) { - total_parsed += dispatchSlice(static_cast(slice.mem_), slice.len_); + auto statusor_parsed = dispatchSlice(static_cast(slice.mem_), slice.len_); + if (!statusor_parsed.ok()) { + return statusor_parsed.status(); + } + total_parsed += statusor_parsed.value(); if (HTTP_PARSER_ERRNO(&parser_) != HPE_OK) { // Parse errors trigger an exception in dispatchSlice so we are guaranteed to be paused at // this point. @@ -526,7 +543,10 @@ Http::Status ConnectionImpl::innerDispatch(Buffer::Instance& data) { } dispatchBufferedBody(); } else { - dispatchSlice(nullptr, 0); + auto result = dispatchSlice(nullptr, 0); + if (!result.ok()) { + return result.status(); + } } ASSERT(buffered_body_.length() == 0); @@ -539,39 +559,54 @@ Http::Status ConnectionImpl::innerDispatch(Buffer::Instance& data) { return Http::okStatus(); } -size_t ConnectionImpl::dispatchSlice(const char* slice, size_t len) { +Envoy::StatusOr ConnectionImpl::dispatchSlice(const char* slice, size_t len) { + ASSERT(codec_status_.ok() && dispatching_); ssize_t rc = http_parser_execute(&parser_, &settings_, slice, len); + if (!codec_status_.ok()) { + return codec_status_; + } if (HTTP_PARSER_ERRNO(&parser_) != HPE_OK && HTTP_PARSER_ERRNO(&parser_) != HPE_PAUSED) { sendProtocolError(Http1ResponseCodeDetails::get().HttpCodecError); - throw CodecProtocolException("http/1.1 protocol error: " + - std::string(http_errno_name(HTTP_PARSER_ERRNO(&parser_)))); + // Avoid overwriting the codec_status_ set in the callbacks. + ASSERT(codec_status_.ok()); + codec_status_ = codecProtocolError( + absl::StrCat("http/1.1 protocol error: ", http_errno_name(HTTP_PARSER_ERRNO(&parser_)))); + return codec_status_; } return rc; } -void ConnectionImpl::onHeaderField(const char* data, size_t length) { +ConnectionImpl::HttpParserCode ConnectionImpl::onHeaderField(const char* data, size_t length) { + ASSERT(dispatching_); // We previously already finished up the headers, these headers are // now trailers. if (header_parsing_state_ == HeaderParsingState::Done) { if (!enable_trailers_) { // Ignore trailers. - return; + return HttpParserCode::Success; } processing_trailers_ = true; header_parsing_state_ = HeaderParsingState::Field; } if (header_parsing_state_ == HeaderParsingState::Value) { - completeLastHeader(); + HttpParserCode exit_code = completeLastHeader(); + if (exit_code == HttpParserCode::Error) { + // If an error exit code is returned, there must be an error in the codec status. + ASSERT(!codec_status_.ok()); + return HttpParserCode::Error; + } } current_header_field_.append(data, length); + return HttpParserCode::Success; } -void ConnectionImpl::onHeaderValue(const char* data, size_t length) { +ConnectionImpl::HttpParserCode ConnectionImpl::onHeaderValue(const char* data, size_t length) { + ASSERT(dispatching_); if (header_parsing_state_ == HeaderParsingState::Done && !enable_trailers_) { // Ignore trailers. - return; + return HttpParserCode::Success; } if (processing_trailers_) { @@ -585,7 +620,10 @@ void ConnectionImpl::onHeaderValue(const char* data, size_t length) { ENVOY_CONN_LOG(debug, "invalid header value: {}", connection_, header_value); error_code_ = Http::Code::BadRequest; sendProtocolError(Http1ResponseCodeDetails::get().InvalidCharacters); - throw CodecProtocolException("http/1.1 protocol error: header value contains invalid chars"); + ASSERT(codec_status_.ok()); + codec_status_ = + codecProtocolError("http/1.1 protocol error: header value contains invalid chars"); + return HttpParserCode::Error; } } @@ -606,14 +644,23 @@ void ConnectionImpl::onHeaderValue(const char* data, size_t length) { processing_trailers_ ? Http1HeaderTypes::get().Trailers : Http1HeaderTypes::get().Headers; error_code_ = Http::Code::RequestHeaderFieldsTooLarge; sendProtocolError(Http1ResponseCodeDetails::get().HeadersTooLarge); - throw CodecProtocolException(absl::StrCat(header_type, " size exceeds limit")); + ASSERT(codec_status_.ok()); + codec_status_ = codecProtocolError(absl::StrCat(header_type, " size exceeds limit")); + return HttpParserCode::Error; } + return HttpParserCode::Success; } -int ConnectionImpl::onHeadersCompleteBase() { +ConnectionImpl::HttpParserCode ConnectionImpl::onHeadersCompleteBase() { ASSERT(!processing_trailers_); + ASSERT(dispatching_); ENVOY_CONN_LOG(trace, "onHeadersCompleteBase", connection_); - completeLastHeader(); + HttpParserCode exit_code = completeLastHeader(); + if (exit_code == HttpParserCode::Error) { + // If an error exit code is returned, there must be an error in the codec status. + ASSERT(!codec_status_.ok()); + return exit_code; + } if (!(parser_.http_major == 1 && parser_.http_minor == 1)) { // This is not necessarily true, but it's good enough since higher layers only care if this is @@ -660,15 +707,20 @@ int ConnectionImpl::onHeadersCompleteBase() { !absl::EqualsIgnoreCase(encoding, Headers::get().TransferEncodingValues.Chunked)) { error_code_ = Http::Code::NotImplemented; sendProtocolError(Http1ResponseCodeDetails::get().InvalidTransferEncoding); - throw CodecProtocolException("http/1.1 protocol error: unsupported transfer encoding"); + ASSERT(codec_status_.ok()); + codec_status_ = codecProtocolError("http/1.1 protocol error: unsupported transfer encoding"); + return HttpParserCode::Error; } } - int rc = onHeadersComplete(); + HttpParserCode rc = onHeadersComplete(); + if (rc == HttpParserCode::Error) { + return rc; + } header_parsing_state_ = HeaderParsingState::Done; // Returning 2 informs http_parser to not expect a body or further data on this connection. - return handling_upgrade_ ? 2 : rc; + return handling_upgrade_ ? HttpParserCode::NoBodyData : rc; } void ConnectionImpl::bufferBody(const char* data, size_t length) { @@ -677,6 +729,7 @@ void ConnectionImpl::bufferBody(const char* data, size_t length) { void ConnectionImpl::dispatchBufferedBody() { ASSERT(HTTP_PARSER_ERRNO(&parser_) == HPE_OK || HTTP_PARSER_ERRNO(&parser_) == HPE_PAUSED); + ASSERT(codec_status_.ok()); if (buffered_body_.length() > 0) { onBody(buffered_body_); buffered_body_.drain(buffered_body_.length()); @@ -691,7 +744,7 @@ void ConnectionImpl::onChunkHeader(bool is_final_chunk) { } } -void ConnectionImpl::onMessageCompleteBase() { +ConnectionImpl::HttpParserCode ConnectionImpl::onMessageCompleteBase() { ENVOY_CONN_LOG(trace, "message complete", connection_); dispatchBufferedBody(); @@ -702,19 +755,25 @@ void ConnectionImpl::onMessageCompleteBase() { ASSERT(!deferred_end_stream_headers_); ENVOY_CONN_LOG(trace, "Pausing parser due to upgrade.", connection_); http_parser_pause(&parser_, 1); - return; + return HttpParserCode::Success; } // If true, this indicates we were processing trailers and must // move the last header into current_header_map_ if (header_parsing_state_ == HeaderParsingState::Value) { - completeLastHeader(); + HttpParserCode exit_code = completeLastHeader(); + if (exit_code == HttpParserCode::Error) { + // If an error exit code is returned, there must be an error in the codec status. + ASSERT(!codec_status_.ok()); + return exit_code; + } } onMessageComplete(); + return HttpParserCode::Success; } -void ConnectionImpl::onMessageBeginBase() { +ConnectionImpl::HttpParserCode ConnectionImpl::onMessageBeginBase() { ENVOY_CONN_LOG(trace, "message begin", connection_); // Make sure that if HTTP/1.0 and HTTP/1.1 requests share a connection Envoy correctly sets // protocol for each request. Envoy defaults to 1.1 but sets the protocol to 1.0 where applicable @@ -723,7 +782,7 @@ void ConnectionImpl::onMessageBeginBase() { processing_trailers_ = false; header_parsing_state_ = HeaderParsingState::Field; allocHeaders(); - onMessageBegin(); + return onMessageBegin(); } void ConnectionImpl::onResetStreamBase(StreamResetReason reason) { @@ -763,7 +822,7 @@ void ServerConnectionImpl::onEncodeComplete() { } } -void ServerConnectionImpl::handlePath(RequestHeaderMap& headers, unsigned int method) { +bool ServerConnectionImpl::handlePath(RequestHeaderMap& headers, unsigned int method) { HeaderString path(Headers::get().Path); bool is_connect = (method == HTTP_CONNECT); @@ -774,7 +833,7 @@ void ServerConnectionImpl::handlePath(RequestHeaderMap& headers, unsigned int me (active_request.request_url_.getStringView()[0] == '/' || ((method == HTTP_OPTIONS) && active_request.request_url_.getStringView()[0] == '*'))) { headers.addViaMove(std::move(path), std::move(active_request.request_url_)); - return; + return true; } // If absolute_urls and/or connect are not going be handled, copy the url and return. @@ -783,13 +842,15 @@ void ServerConnectionImpl::handlePath(RequestHeaderMap& headers, unsigned int me // Absolute URLS in CONNECT requests will be rejected below by the URL class validation. if (!codec_settings_.allow_absolute_url_ && !is_connect) { headers.addViaMove(std::move(path), std::move(active_request.request_url_)); - return; + return true; } Utility::Url absolute_url; if (!absolute_url.initialize(active_request.request_url_.getStringView(), is_connect)) { sendProtocolError(Http1ResponseCodeDetails::get().InvalidUrl); - throw CodecProtocolException("http/1.1 protocol error: invalid url in request line"); + ASSERT(codec_status_.ok()); + codec_status_ = codecProtocolError("http/1.1 protocol error: invalid url in request line"); + return false; } // RFC7230#5.7 // When a proxy receives a request with an absolute-form of @@ -804,9 +865,10 @@ void ServerConnectionImpl::handlePath(RequestHeaderMap& headers, unsigned int me headers.setPath(absolute_url.pathAndQueryParams()); } active_request.request_url_.clear(); + return true; } -int ServerConnectionImpl::onHeadersComplete() { +ConnectionImpl::HttpParserCode ServerConnectionImpl::onHeadersComplete() { // Handle the case where response happens prior to request complete. It's up to upper layer code // to disconnect the connection but we shouldn't fire any more events since it doesn't make // sense. @@ -824,7 +886,9 @@ int ServerConnectionImpl::onHeadersComplete() { header_value); error_code_ = Http::Code::BadRequest; sendProtocolError(Http1ResponseCodeDetails::get().ConnectionHeaderSanitization); - throw CodecProtocolException("Invalid nominated headers in Connection."); + ASSERT(codec_status_.ok()); + codec_status_ = codecProtocolError("Invalid nominated headers in Connection."); + return HttpParserCode::Error; } } @@ -833,7 +897,11 @@ int ServerConnectionImpl::onHeadersComplete() { active_request.response_encoder_.setIsResponseToHeadRequest(parser_.method == HTTP_HEAD); active_request.response_encoder_.setIsResponseToConnectRequest(parser_.method == HTTP_CONNECT); - handlePath(*headers, parser_.method); + if (!handlePath(*headers, parser_.method)) { + // Reached a failure. + ASSERT(!codec_status_.ok()); + return HttpParserCode::Error; + } ASSERT(active_request.request_url_.empty()); headers->setMethod(method_string); @@ -842,8 +910,10 @@ int ServerConnectionImpl::onHeadersComplete() { auto details = HeaderUtility::requestHeadersValid(*headers); if (details.has_value()) { sendProtocolError(details.value().get()); - throw CodecProtocolException( + ASSERT(codec_status_.ok()); + codec_status_ = codecProtocolError( "http/1.1 protocol error: request headers failed spec compliance checks"); + return HttpParserCode::Error; } // Determine here whether we have a body or not. This uses the new RFC semantics where the @@ -866,20 +936,27 @@ int ServerConnectionImpl::onHeadersComplete() { } } - return 0; + return HttpParserCode::Success; } -void ServerConnectionImpl::onMessageBegin() { +ConnectionImpl::HttpParserCode ServerConnectionImpl::onMessageBegin() { if (!resetStreamCalled()) { ASSERT(!active_request_.has_value()); active_request_.emplace(*this, header_key_formatter_.get()); auto& active_request = active_request_.value(); + if (resetStreamCalled()) { + ASSERT(codec_status_.ok()); + codec_status_ = codecClientError("cannot create new streams after calling reset"); + return HttpParserCode::Error; + } active_request.request_decoder_ = &callbacks_.newStream(active_request.response_encoder_); // Check for pipelined request flood as we prepare to accept a new request. // Parse errors that happen prior to onMessageBegin result in stream termination, it is not // possible to overflow output buffers with early parse errors. - doFloodProtectionChecks(); + return doFloodProtectionChecks(); + } else { + return HttpParserCode::Success; } } @@ -965,7 +1042,7 @@ void ServerConnectionImpl::releaseOutboundResponse( delete fragment; } -void ServerConnectionImpl::checkHeaderNameForUnderscores() { +bool ServerConnectionImpl::checkHeaderNameForUnderscores() { if (headers_with_underscores_action_ != envoy::config::core::v3::HttpProtocolOptions::ALLOW && Http::HeaderUtility::headerNameContainsUnderscore(current_header_field_.getStringView())) { if (headers_with_underscores_action_ == @@ -981,9 +1058,13 @@ void ServerConnectionImpl::checkHeaderNameForUnderscores() { error_code_ = Http::Code::BadRequest; sendProtocolError(Http1ResponseCodeDetails::get().InvalidCharacters); stats_.requests_rejected_with_underscores_in_headers_.inc(); - throw CodecProtocolException("http/1.1 protocol error: header name contains underscores"); + ASSERT(codec_status_.ok()); + codec_status_ = + codecProtocolError("http/1.1 protocol error: header name contains underscores"); + return false; } } + return true; } ClientConnectionImpl::ClientConnectionImpl(Network::Connection& connection, Stats::Scope& stats, @@ -1005,10 +1086,6 @@ bool ClientConnectionImpl::cannotHaveBody() { } RequestEncoder& ClientConnectionImpl::newStream(ResponseDecoder& response_decoder) { - if (resetStreamCalled()) { - throw CodecClientException("cannot create new streams after calling reset"); - } - // If reads were disabled due to flow control, we expect reads to always be enabled again before // reusing this connection. This is done when the response is received. ASSERT(connection_.readEnabled()); @@ -1020,12 +1097,14 @@ RequestEncoder& ClientConnectionImpl::newStream(ResponseDecoder& response_decode return pending_response_.value().encoder_; } -int ClientConnectionImpl::onHeadersComplete() { +ConnectionImpl::HttpParserCode ClientConnectionImpl::onHeadersComplete() { // Handle the case where the client is closing a kept alive connection (by sending a 408 // with a 'Connection: close' header). In this case we just let response flush out followed // by the remote close. if (!pending_response_.has_value() && !resetStreamCalled()) { - throw PrematureResponseException(static_cast(parser_.status_code)); + ASSERT(codec_status_.ok()); + codec_status_ = prematureResponseError("", static_cast(parser_.status_code)); + return HttpParserCode::Error; } else if (pending_response_.has_value()) { ASSERT(!pending_response_done_); auto& headers = absl::get(headers_or_trailers_); @@ -1056,7 +1135,7 @@ int ClientConnectionImpl::onHeadersComplete() { // Here we deal with cases where the response cannot have a body, but http_parser does not deal // with it for us. - return cannotHaveBody() ? 1 : 0; + return cannotHaveBody() ? HttpParserCode::NoBody : HttpParserCode::Success; } bool ClientConnectionImpl::upgradeAllowed() const { diff --git a/source/common/http/http1/codec_impl.h b/source/common/http/http1/codec_impl.h index d3fa827e029ae..47898a34551f0 100644 --- a/source/common/http/http1/codec_impl.h +++ b/source/common/http/http1/codec_impl.h @@ -215,6 +215,10 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable dispatchSlice(const char* slice, size_t len); /** * Called by the http_parser when body data is received. @@ -297,9 +321,10 @@ class ConnectionImpl : public virtual Connection, protected Logger::Loggable active_request_; @@ -523,9 +553,9 @@ class ClientConnectionImpl : public ClientConnection, public ConnectionImpl { // ConnectionImpl void onEncodeComplete() override {} - void onMessageBegin() override {} + HttpParserCode onMessageBegin() override { return HttpParserCode::Success; } void onUrl(const char*, size_t) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } - int onHeadersComplete() override; + HttpParserCode onHeadersComplete() override; bool upgradeAllowed() const override; void onBody(Buffer::Instance& data) override; void onMessageComplete() override;