diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index fe319d2c30ba6..305a09868566a 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -15,6 +15,7 @@ Minor Behavior Changes * bandwidth_limit: added :ref:`bandwidth limit stats ` *request_enforced* and *response_enforced*. * config: the log message for "gRPC config stream closed" now uses the most recent error message, and reports seconds instead of milliseconds for how long the most recent status has been received. * dns: now respecting the returned DNS TTL for resolved hosts, rather than always relying on the hard-coded :ref:`dns_refresh_rate. ` This behavior can be temporarily reverted by setting the runtime guard ``envoy.reloadable_features.use_dns_ttl`` to false. +* http: envoy will now proxy 102 and 103 headers from upstream, though as with 100s only the first 1xx response headers will be sent. This behavioral change by can temporarily reverted by setting runtime guard ``envoy.reloadable_features.proxy_102_103`` to false. * http: usage of the experimental matching API is no longer guarded behind a feature flag, as the corresponding protobuf fields have been marked as WIP. * json: switching from rapidjson to nlohmann/json. This behavioral change can be temporarily reverted by setting runtime guard ``envoy.reloadable_features.remove_legacy_json`` to false. * listener: destroy per network filter chain stats when a network filter chain is removed during the listener in place update. diff --git a/envoy/http/codec.h b/envoy/http/codec.h index 9dec51e385ff5..7445fb20eec52 100644 --- a/envoy/http/codec.h +++ b/envoy/http/codec.h @@ -141,7 +141,7 @@ class ResponseEncoder : public virtual StreamEncoder { public: /** * Encode supported 1xx headers. - * Currently only 100-Continue headers are supported. + * Currently 100-Continue, 102-Processing, and 103-Early-Data headers are supported. * @param headers supplies the 1xx header map to encode. */ virtual void encode1xxHeaders(const ResponseHeaderMap& headers) PURE; @@ -237,7 +237,7 @@ class ResponseDecoder : public virtual StreamDecoder { public: /** * Called with decoded 1xx headers. - * Currently only 100-Continue headers are supported. + * Currently 100-Continue, 102-Processing, and 103-Early-Data headers are supported. * @param headers supplies the decoded 1xx headers map. */ virtual void decode1xxHeaders(ResponseHeaderMapPtr&& headers) PURE; diff --git a/source/common/http/header_utility.cc b/source/common/http/header_utility.cc index c278b30bd19f3..a7f630b041b4c 100644 --- a/source/common/http/header_utility.cc +++ b/source/common/http/header_utility.cc @@ -202,6 +202,11 @@ bool HeaderUtility::authorityIsValid(const absl::string_view header_value) { } bool HeaderUtility::isSpecial1xx(const ResponseHeaderMap& response_headers) { + if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.proxy_102_103") && + (response_headers.Status()->value() == "102" || + response_headers.Status()->value() == "103")) { + return true; + } return response_headers.Status()->value() == "100"; } diff --git a/source/common/http/http1/codec_impl.cc b/source/common/http/http1/codec_impl.cc index 2bf8b4cc1dc92..0468bdbcb86fd 100644 --- a/source/common/http/http1/codec_impl.cc +++ b/source/common/http/http1/codec_impl.cc @@ -181,8 +181,8 @@ void StreamEncoderImpl::encodeHeadersBase(const RequestOrResponseHeaderMap& head if (saw_content_length || disable_chunk_encoding_) { chunk_encoding_ = false; } else { - if (status && *status == 100) { - // Make sure we don't serialize chunk information with 100-Continue headers. + if (status && (*status == 100 || *status == 102 || *status == 103)) { + // Make sure we don't serialize chunk information with 1xx headers. chunk_encoding_ = false; } else if (status && *status == 304 && connection_.noChunkedEncodingHeaderFor304()) { // For 304 response, since it should never have a body, we should not need to chunk_encode at diff --git a/source/common/http/http2/codec_impl.cc b/source/common/http/http2/codec_impl.cc index d663aca876d5f..694cbb7bc27ca 100644 --- a/source/common/http/http2/codec_impl.cc +++ b/source/common/http/http2/codec_impl.cc @@ -400,7 +400,7 @@ void ConnectionImpl::ClientStreamImpl::decodeHeaders() { received_noninformational_headers_ = !CodeUtility::is1xx(status) || status == enumToInt(Http::Code::SwitchingProtocols); - if (status == enumToInt(Http::Code::Continue)) { + if (HeaderUtility::isSpecial1xx(*headers)) { ASSERT(!remote_end_stream_); response_decoder_.decode1xxHeaders(std::move(headers)); } else { diff --git a/source/common/router/router.cc b/source/common/router/router.cc index f58d36f56eacc..59336369ab11d 100644 --- a/source/common/router/router.cc +++ b/source/common/router/router.cc @@ -1227,8 +1227,9 @@ void Filter::handleNon5xxResponseHeaders(absl::optional response_decoder; + Http::RequestEncoder& request_encoder = codec_->newStream(response_decoder); + TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/"}, {":authority", "host"}}; + EXPECT_TRUE(request_encoder.encodeHeaders(headers, true).ok()); + + EXPECT_CALL(response_decoder, decode1xxHeaders_(_)); + EXPECT_CALL(response_decoder, decodeData(_, _)).Times(0); + Buffer::OwnedImpl initial_response("HTTP/1.1 102 Processing\r\n\r\n"); + auto status = codec_->dispatch(initial_response); + EXPECT_TRUE(status.ok()); + + EXPECT_CALL(response_decoder, decodeHeaders_(_, false)); + EXPECT_CALL(response_decoder, decodeData(_, _)).Times(0); + Buffer::OwnedImpl response("HTTP/1.1 200 OK\r\n\r\n"); + status = codec_->dispatch(response); + EXPECT_TRUE(status.ok()); +} + +// 103 response followed by 200 results in a [decode1xxHeaders, decodeHeaders] sequence. +TEST_F(Http1ClientConnectionImplTest, EarlyHintHeaders) { + initialize(); + + NiceMock response_decoder; + Http::RequestEncoder& request_encoder = codec_->newStream(response_decoder); + TestRequestHeaderMapImpl headers{{":method", "GET"}, {":path", "/"}, {":authority", "host"}}; + EXPECT_TRUE(request_encoder.encodeHeaders(headers, true).ok()); + + EXPECT_CALL(response_decoder, decode1xxHeaders_(_)); + EXPECT_CALL(response_decoder, decodeData(_, _)).Times(0); + Buffer::OwnedImpl initial_response("HTTP/1.1 103 Early Hints\r\n\r\n"); + auto status = codec_->dispatch(initial_response); + EXPECT_TRUE(status.ok()); + + EXPECT_CALL(response_decoder, decodeHeaders_(_, false)); + EXPECT_CALL(response_decoder, decodeData(_, _)).Times(0); + Buffer::OwnedImpl response("HTTP/1.1 200 OK\r\n\r\n"); + status = codec_->dispatch(response); + EXPECT_TRUE(status.ok()); +} + // Multiple 100 responses are passed to the response encoder (who is responsible for coalescing). TEST_F(Http1ClientConnectionImplTest, MultipleContinueHeaders) { initialize(); @@ -2536,9 +2580,7 @@ TEST_F(Http1ClientConnectionImplTest, MultipleContinueHeaders) { EXPECT_TRUE(status.ok()); } -// 101/102 headers etc. are passed to the response encoder (who is responsibly for deciding to -// upgrade, ignore, etc.). -TEST_F(Http1ClientConnectionImplTest, 1xxNonContinueHeaders) { +TEST_F(Http1ClientConnectionImplTest, Unsupported1xxHeader) { initialize(); NiceMock response_decoder; @@ -2547,7 +2589,7 @@ TEST_F(Http1ClientConnectionImplTest, 1xxNonContinueHeaders) { EXPECT_TRUE(request_encoder.encodeHeaders(headers, true).ok()); EXPECT_CALL(response_decoder, decodeHeaders_(_, false)); - Buffer::OwnedImpl response("HTTP/1.1 102 Processing\r\n\r\n"); + Buffer::OwnedImpl response("HTTP/1.1 199 Unknown\r\n\r\n"); auto status = codec_->dispatch(response); EXPECT_TRUE(status.ok()); } diff --git a/test/common/http/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index 9855f487fefe3..2cc18d87b36e4 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -527,9 +527,9 @@ TEST_P(Http2CodecImplTest, MultipleContinueHeaders) { response_encoder_->encodeHeaders(response_headers, true); }; -// 101/102 headers etc. are passed to the response encoder (who is responsibly for deciding to +// 104 headers etc. are passed to the response encoder (who is responsibly for deciding to // upgrade, ignore, etc.). -TEST_P(Http2CodecImplTest, 1xxNonContinueHeaders) { +TEST_P(Http2CodecImplTest, Unsupported1xxHeader) { initialize(); TestRequestHeaderMapImpl request_headers; @@ -537,7 +537,7 @@ TEST_P(Http2CodecImplTest, 1xxNonContinueHeaders) { EXPECT_CALL(request_decoder_, decodeHeaders_(_, true)); EXPECT_TRUE(request_encoder_->encodeHeaders(request_headers, true).ok()); - TestResponseHeaderMapImpl other_headers{{":status", "102"}}; + TestResponseHeaderMapImpl other_headers{{":status", "104"}}; EXPECT_CALL(response_decoder_, decodeHeaders_(_, false)); response_encoder_->encodeHeaders(other_headers, false); }; diff --git a/test/common/quic/envoy_quic_client_stream_test.cc b/test/common/quic/envoy_quic_client_stream_test.cc index a2b8ac4da7d8c..7bead0f2972df 100644 --- a/test/common/quic/envoy_quic_client_stream_test.cc +++ b/test/common/quic/envoy_quic_client_stream_test.cc @@ -206,14 +206,14 @@ TEST_F(EnvoyQuicClientStreamTest, PostRequestAnd1xx) { })); EXPECT_CALL(stream_decoder_, decodeHeaders_(_, /*end_stream=*/false)) .WillOnce(Invoke([](const Http::ResponseHeaderMapPtr& headers, bool) { - EXPECT_EQ("103", headers->getStatusValue()); + EXPECT_EQ("199", headers->getStatusValue()); EXPECT_EQ("1", headers->get(Http::LowerCaseString("i"))[0]->value().getStringView()); })); size_t offset = 0; size_t i = 0; // Receive several 10x headers, only the first 100 Continue header should be // delivered. - for (const std::string& status : {"100", "103", "100"}) { + for (const std::string& status : {"100", "199", "100"}) { spdy::SpdyHeaderBlock continue_header; continue_header[":status"] = status; continue_header["i"] = absl::StrCat("", i++); diff --git a/test/integration/http_integration.cc b/test/integration/http_integration.cc index 814744f5ea4d4..0a387125542d2 100644 --- a/test/integration/http_integration.cc +++ b/test/integration/http_integration.cc @@ -971,7 +971,7 @@ void HttpIntegrationTest::testEnvoyHandling1xx(bool additional_continue_from_ups auto response = std::move(encoder_decoder.second); ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); // The continue headers should arrive immediately. - response->waitForContinueHeaders(); + response->waitFor1xxHeaders(); ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); // Send the rest of the request. @@ -994,7 +994,7 @@ void HttpIntegrationTest::testEnvoyHandling1xx(bool additional_continue_from_ups } if (disconnect_after_100) { - response->waitForContinueHeaders(); + response->waitFor1xxHeaders(); codec_client_->close(); EXPECT_THAT(waitForAccessLog(access_log_name_), HasSubstr("100")); ASSERT_TRUE(fake_upstream_connection_->close()); @@ -1020,7 +1020,8 @@ void HttpIntegrationTest::testEnvoyHandling1xx(bool additional_continue_from_ups void HttpIntegrationTest::testEnvoyProxying1xx(bool continue_before_upstream_complete, bool with_encoder_filter, - bool with_multiple_1xx_headers) { + bool with_multiple_1xx_headers, + absl::string_view initial_code) { if (with_encoder_filter) { // Add a filter to make sure 100s play well with them. config_helper_.prependFilter("name: passthrough-filter"); @@ -1044,37 +1045,39 @@ void HttpIntegrationTest::testEnvoyProxying1xx(bool continue_before_upstream_com ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); + // This case tests sending on 100-Continue headers before the client has sent all the + // request data. if (continue_before_upstream_complete) { + upstream_request_->encode1xxHeaders( + Http::TestResponseHeaderMapImpl{{":status", initial_code.data()}}); if (with_multiple_1xx_headers) { upstream_request_->encode1xxHeaders(Http::TestResponseHeaderMapImpl{{":status", "100"}}); upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "102"}}, false); upstream_request_->encode1xxHeaders(Http::TestResponseHeaderMapImpl{{":status", "100"}}); } - // This case tests sending on 100-Continue headers before the client has sent all the - // request data. - upstream_request_->encode1xxHeaders(Http::TestResponseHeaderMapImpl{{":status", "100"}}); - response->waitForContinueHeaders(); + response->waitFor1xxHeaders(); } // Send all of the request data and wait for it to be received upstream. codec_client_->sendData(*request_encoder_, 10, true); ASSERT_TRUE(upstream_request_->waitForEndStream(*dispatcher_)); + // This case tests forwarding 100-Continue after the client has sent all data. if (!continue_before_upstream_complete) { + upstream_request_->encode1xxHeaders( + Http::TestResponseHeaderMapImpl{{":status", initial_code.data()}}); if (with_multiple_1xx_headers) { upstream_request_->encode1xxHeaders(Http::TestResponseHeaderMapImpl{{":status", "100"}}); upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "102"}}, false); upstream_request_->encode1xxHeaders(Http::TestResponseHeaderMapImpl{{":status", "100"}}); } - // This case tests forwarding 100-Continue after the client has sent all data. - upstream_request_->encode1xxHeaders(Http::TestResponseHeaderMapImpl{{":status", "100"}}); - response->waitForContinueHeaders(); + response->waitFor1xxHeaders(); } // Now send the rest of the response. upstream_request_->encodeHeaders(default_response_headers_, true); ASSERT_TRUE(response->waitForEndStream()); EXPECT_TRUE(response->complete()); ASSERT(response->informationalHeaders() != nullptr); - EXPECT_EQ("100", response->informationalHeaders()->getStatusValue()); + EXPECT_EQ(initial_code, response->informationalHeaders()->getStatusValue()); EXPECT_EQ("200", response->headers().getStatusValue()); } diff --git a/test/integration/http_integration.h b/test/integration/http_integration.h index 7bdc2b749e2e6..c7fd491026ab2 100644 --- a/test/integration/http_integration.h +++ b/test/integration/http_integration.h @@ -252,7 +252,8 @@ class HttpIntegrationTest : public BaseIntegrationTest { const std::string& via = "", bool disconnect_after_100 = false); void testEnvoyProxying1xx(bool continue_before_upstream_complete = false, bool with_encoder_filter = false, - bool with_multiple_1xx_headers = false); + bool with_multiple_1xx_headers = false, + absl::string_view initial_code = "100"); void simultaneousRequest(uint32_t request1_bytes, uint32_t request2_bytes, uint32_t response1_bytes, uint32_t response2_bytes); diff --git a/test/integration/integration_stream_decoder.cc b/test/integration/integration_stream_decoder.cc index 8bf5307a268bb..775641b8d0c73 100644 --- a/test/integration/integration_stream_decoder.cc +++ b/test/integration/integration_stream_decoder.cc @@ -24,7 +24,7 @@ namespace Envoy { IntegrationStreamDecoder::IntegrationStreamDecoder(Event::Dispatcher& dispatcher) : dispatcher_(dispatcher) {} -void IntegrationStreamDecoder::waitForContinueHeaders() { +void IntegrationStreamDecoder::waitFor1xxHeaders() { if (!continue_headers_.get()) { waiting_for_continue_headers_ = true; dispatcher_.run(Event::Dispatcher::RunType::Block); diff --git a/test/integration/integration_stream_decoder.h b/test/integration/integration_stream_decoder.h index dbdacf6930dbf..e4e5b3c85b207 100644 --- a/test/integration/integration_stream_decoder.h +++ b/test/integration/integration_stream_decoder.h @@ -35,7 +35,7 @@ class IntegrationStreamDecoder : public Http::ResponseDecoder, public Http::Stre const Http::MetadataMap& metadataMap() { return *metadata_map_; } uint64_t keyCount(std::string key) { return duplicated_metadata_key_count_[key]; } uint32_t metadataMapsDecodedCount() const { return metadata_maps_decoded_count_; } - void waitForContinueHeaders(); + void waitFor1xxHeaders(); void waitForHeaders(); // This function waits until body_ has at least size bytes in it (it might have more). clearBody() // can be used if the previous body data is not relevant and the test wants to wait for a specific diff --git a/test/integration/multiplexed_integration_test.cc b/test/integration/multiplexed_integration_test.cc index df5622fff917c..726d7c5e331a3 100644 --- a/test/integration/multiplexed_integration_test.cc +++ b/test/integration/multiplexed_integration_test.cc @@ -507,7 +507,7 @@ TEST_P(Http2MetadataIntegrationTest, TestResponseMetadata) { waitForNextUpstreamRequest(); upstream_request_->encode1xxHeaders(Http::TestResponseHeaderMapImpl{{":status", "100"}}); - response->waitForContinueHeaders(); + response->waitFor1xxHeaders(); upstream_request_->encodeHeaders(default_response_headers_, false); upstream_request_->encodeData(100, true); diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 72c79fea27df5..c43ac822ee2bc 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -1341,6 +1341,14 @@ TEST_P(ProtocolIntegrationTest, EnvoyProxyingLateMultiple1xx) { testEnvoyProxying1xx(false, false, true); } +TEST_P(ProtocolIntegrationTest, EnvoyProxying102) { + testEnvoyProxying1xx(false, false, false, "102"); +} + +TEST_P(ProtocolIntegrationTest, EnvoyProxying103) { + testEnvoyProxying1xx(false, false, false, "103"); +} + TEST_P(ProtocolIntegrationTest, TwoRequests) { testTwoRequests(); } TEST_P(ProtocolIntegrationTest, TwoRequestsWithForcedBackup) { testTwoRequests(true); }