diff --git a/api/envoy/extensions/quic/server_preferred_address/v3/BUILD b/api/envoy/extensions/quic/server_preferred_address/v3/BUILD index 628f71321fba..09a37ad16b83 100644 --- a/api/envoy/extensions/quic/server_preferred_address/v3/BUILD +++ b/api/envoy/extensions/quic/server_preferred_address/v3/BUILD @@ -8,6 +8,5 @@ api_proto_package( deps = [ "//envoy/config/core/v3:pkg", "@com_github_cncf_xds//udpa/annotations:pkg", - "@com_github_cncf_xds//xds/annotations/v3:pkg", ], ) diff --git a/api/envoy/extensions/quic/server_preferred_address/v3/datasource.proto b/api/envoy/extensions/quic/server_preferred_address/v3/datasource.proto index 6baf39850763..5bea48663e66 100644 --- a/api/envoy/extensions/quic/server_preferred_address/v3/datasource.proto +++ b/api/envoy/extensions/quic/server_preferred_address/v3/datasource.proto @@ -4,8 +4,6 @@ package envoy.extensions.quic.server_preferred_address.v3; import "envoy/config/core/v3/base.proto"; -import "xds/annotations/v3/status.proto"; - import "udpa/annotations/status.proto"; import "validate/validate.proto"; @@ -20,10 +18,6 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // Configuration for DataSourceServerPreferredAddressConfig. message DataSourceServerPreferredAddressConfig { - // [#comment:TODO(danzh2010): discuss with API shepherds before removing WiP status.] - - option (xds.annotations.v3.message_status).work_in_progress = true; - // Addresses for server preferred address for a single address family (IPv4 or IPv6). message AddressFamilyConfig { // The server preferred address sent to clients. The data must contain an IP address string. diff --git a/api/envoy/extensions/quic/server_preferred_address/v3/fixed_server_preferred_address_config.proto b/api/envoy/extensions/quic/server_preferred_address/v3/fixed_server_preferred_address_config.proto index 20ec9a2a379e..43072fd50ef3 100644 --- a/api/envoy/extensions/quic/server_preferred_address/v3/fixed_server_preferred_address_config.proto +++ b/api/envoy/extensions/quic/server_preferred_address/v3/fixed_server_preferred_address_config.proto @@ -4,8 +4,6 @@ package envoy.extensions.quic.server_preferred_address.v3; import "envoy/config/core/v3/address.proto"; -import "xds/annotations/v3/status.proto"; - import "udpa/annotations/status.proto"; option java_package = "io.envoyproxy.envoy.extensions.quic.server_preferred_address.v3"; @@ -19,10 +17,6 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE; // Configuration for FixedServerPreferredAddressConfig. message FixedServerPreferredAddressConfig { - // [#comment:TODO(danzh2010): discuss with API shepherds before removing WiP status.] - - option (xds.annotations.v3.message_status).work_in_progress = true; - // Addresses for server preferred address for a single address family (IPv4 or IPv6). message AddressFamilyConfig { // The server preferred address sent to clients. diff --git a/bazel/external/BUILD b/bazel/external/BUILD index ac145dcdfd17..e1909b8de946 100644 --- a/bazel/external/BUILD +++ b/bazel/external/BUILD @@ -7,14 +7,10 @@ exports_files(["boringssl_fips.genrule_cmd"]) cc_library( name = "all_external", srcs = [":empty.cc"], - defines = ["OPENTRACING_STATIC"], - # TODO: external/io_opentracing_cpp/BUILD.bazel:19:1: Executing genrule - # @io_opentracing_cpp//:generate_version_h failed - needs porting tags = ["skip_on_windows"], deps = [ "@com_github_datadog_dd_trace_cpp//:dd_trace_cpp", "@com_google_googletest//:gtest", - "@io_opentracing_cpp//:opentracing", ], ) diff --git a/bazel/external/quiche.BUILD b/bazel/external/quiche.BUILD index 0f9339f2a1c3..0abec6540ec7 100644 --- a/bazel/external/quiche.BUILD +++ b/bazel/external/quiche.BUILD @@ -3102,6 +3102,7 @@ envoy_quic_cc_library( ":quic_core_http_spdy_utils_lib", ":quic_core_types_lib", ":quic_platform_base", + "@com_google_absl//absl/base:nullability", ], ) diff --git a/bazel/external/quiche_sequencer_fix.patch b/bazel/external/quiche_sequencer_fix.patch deleted file mode 100644 index b4203e92b6e3..000000000000 --- a/bazel/external/quiche_sequencer_fix.patch +++ /dev/null @@ -1,16 +0,0 @@ -# Fix https://github.com/envoyproxy/envoy-setec/issues/1496#issue-2251291349 - -diff --git a/quiche/quic/core/quic_stream_sequencer_buffer.cc b/quiche/quic/core/quic_stream_sequencer_buffer.cc -index d364d61bc..0966af4b0 100644 ---- a/quiche/quic/core/quic_stream_sequencer_buffer.cc -+++ b/quiche/quic/core/quic_stream_sequencer_buffer.cc -@@ -388,7 +388,8 @@ bool QuicStreamSequencerBuffer::PeekRegion(QuicStreamOffset offset, - - // Determine if entire block has been received. - size_t end_block_idx = GetBlockIndex(FirstMissingByte()); -- if (block_idx == end_block_idx) { -+ if (block_idx == end_block_idx && -+ block_offset < GetInBlockOffset(FirstMissingByte())) { - // Only read part of block before FirstMissingByte(). - iov->iov_len = GetInBlockOffset(FirstMissingByte()) - block_offset; - } else { diff --git a/bazel/foreign_cc/cares.patch b/bazel/foreign_cc/cares.patch new file mode 100644 index 000000000000..d666ef023f6e --- /dev/null +++ b/bazel/foreign_cc/cares.patch @@ -0,0 +1,58 @@ +From a070d7835d667b2fae5266fe1b790677dae47d25 Mon Sep 17 00:00:00 2001 +From: Brad House +Date: Thu, 12 Oct 2023 09:29:14 -0400 +Subject: [PATCH] Socket callbacks were passed SOCK_STREAM instead of + SOCK_DGRAM on udp + +A regression was introduced in 1.20.0 that would pass SOCK_STREAM on udp +connections due to code refactoring. If a client application validated this +data, it could cause issues as seen in gRPC. + +Fixes Issue: #571 +Fix By: Brad House (@bradh352) +--- + src/lib/ares_process.c | 10 ++++------ + 1 file changed, 4 insertions(+), 6 deletions(-) + +diff --git a/src/lib/ares_process.c b/src/lib/ares_process.c +index ca597db7ad..2f8e4de30d 100644 +--- a/src/lib/ares_process.c ++++ b/src/lib/ares_process.c +@@ -1065,6 +1065,7 @@ static ares_status_t open_socket(ares_channel channel, + unsigned short port; + struct server_connection *conn; + ares__llist_node_t *node; ++ int type = is_tcp?SOCK_STREAM:SOCK_DGRAM; + + if (is_tcp) { + port = aresx_sitous(server->addr.tcp_port? +@@ -1098,8 +1099,7 @@ static ares_status_t open_socket(ares_channel channel, + } + + /* Acquire a socket. */ +- s = ares__open_socket(channel, server->addr.family, +- is_tcp?SOCK_STREAM:SOCK_DGRAM, 0); ++ s = ares__open_socket(channel, server->addr.family, type, 0); + if (s == ARES_SOCKET_BAD) + return ARES_ECONNREFUSED; + +@@ -1129,8 +1129,7 @@ static ares_status_t open_socket(ares_channel channel, + #endif + + if (channel->sock_config_cb) { +- int err = channel->sock_config_cb(s, SOCK_STREAM, +- channel->sock_config_cb_data); ++ int err = channel->sock_config_cb(s, type, channel->sock_config_cb_data); + if (err < 0) { + ares__close_socket(channel, s); + return ARES_ECONNREFUSED; +@@ -1148,8 +1147,7 @@ static ares_status_t open_socket(ares_channel channel, + } + + if (channel->sock_create_cb) { +- int err = channel->sock_create_cb(s, SOCK_STREAM, +- channel->sock_create_cb_data); ++ int err = channel->sock_create_cb(s, type, channel->sock_create_cb_data); + if (err < 0) { + ares__close_socket(channel, s); + return ARES_ECONNREFUSED; diff --git a/bazel/repositories.bzl b/bazel/repositories.bzl index 9903adf38572..ae60f76f4d34 100644 --- a/bazel/repositories.bzl +++ b/bazel/repositories.bzl @@ -333,7 +333,6 @@ def envoy_dependencies(skip_targets = []): _com_googlesource_googleurl() _io_hyperscan() _io_vectorscan() - _io_opentracing_cpp() _io_opentelemetry_api_cpp() _net_colm_open_source_colm() _net_colm_open_source_ragel() @@ -444,6 +443,11 @@ def _com_github_c_ares_c_ares(): external_http_archive( name = "com_github_c_ares_c_ares", build_file_content = BUILD_ALL_CONTENT, + # Patch c-ares library aith commit + # https://github.com/c-ares/c-ares/commit/a070d7835d667b2fae5266fe1b790677dae47d25 + # This commit fixes an issue when the gRPC library attempts to resolve a domain name. + patches = ["@envoy//bazel/foreign_cc:cares.patch"], + patch_args = ["-p1"], ) native.bind( name = "ares", @@ -757,18 +761,6 @@ def _io_vectorscan(): patches = ["@envoy//bazel/foreign_cc:vectorscan.patch"], ) -def _io_opentracing_cpp(): - external_http_archive( - name = "io_opentracing_cpp", - patch_args = ["-p1"], - # Workaround for LSAN false positive in https://github.com/envoyproxy/envoy/issues/7647 - patches = ["@envoy//bazel:io_opentracing_cpp.patch"], - ) - native.bind( - name = "opentracing", - actual = "@io_opentracing_cpp//:opentracing", - ) - def _io_opentelemetry_api_cpp(): external_http_archive(name = "io_opentelemetry_cpp") native.bind( @@ -1133,10 +1125,6 @@ def _com_github_google_quiche(): external_http_archive( name = "com_github_google_quiche", patch_cmds = ["find quiche/ -type f -name \"*.bazel\" -delete"], - patches = [ - "@envoy//bazel/external:quiche_sequencer_fix.patch", - ], - patch_args = ["-p1"], build_file = "@envoy//bazel/external:quiche.BUILD", ) native.bind( diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 909182bf649d..8e93dd784593 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -563,24 +563,6 @@ REPOSITORY_LOCATIONS_SPEC = dict( license = "BSD-3-Clause", license_url = "https://github.com/VectorCamp/vectorscan/blob/vectorscan/{version}/LICENSE", ), - io_opentracing_cpp = dict( - project_name = "OpenTracing", - project_desc = "Vendor-neutral APIs and instrumentation for distributed tracing", - project_url = "https://opentracing.io", - version = "1.5.1", - sha256 = "015c4187f7a6426a2b5196f0ccd982aa87f010cf61f507ae3ce5c90523f92301", - strip_prefix = "opentracing-cpp-{version}", - urls = ["https://github.com/opentracing/opentracing-cpp/archive/v{version}.tar.gz"], - use_category = ["observability_ext"], - extensions = [ - "envoy.tracers.datadog", - "envoy.tracers.dynamic_ot", - ], - release_date = "2019-01-16", - cpe = "N/A", - license = "Apache-2.0", - license_url = "https://github.com/opentracing/opentracing-cpp/blob/v{version}/LICENSE", - ), io_opentelemetry_cpp = dict( project_name = "OpenTelemetry", project_desc = "Observability framework and toolkit designed to create and manage telemetry data such as traces, metrics, and logs.", @@ -1204,12 +1186,12 @@ REPOSITORY_LOCATIONS_SPEC = dict( project_name = "QUICHE", project_desc = "QUICHE (QUIC, HTTP/2, Etc) is Google‘s implementation of QUIC and related protocols", project_url = "https://github.com/google/quiche", - version = "f8ca4ffbe5eb5c099bd11ba3e90553fa282c8421", - sha256 = "7648ede3f32bc6367a629b245d268c9be47ba05e23b4345a54152dabeba387d9", + version = "f4ed5e0c74485fb302367b833b8974373fed9e4c", + sha256 = "05e40b18e78b76a14bfa02eca1d6ebcf4c2ea0333c5db9fbe04287f912db2c20", urls = ["https://github.com/google/quiche/archive/{version}.tar.gz"], strip_prefix = "quiche-{version}", use_category = ["controlplane", "dataplane_core"], - release_date = "2024-07-17", + release_date = "2024-07-26", cpe = "N/A", license = "BSD-3-Clause", license_url = "https://github.com/google/quiche/blob/{version}/LICENSE", diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 455edd5b5fb7..a23ee219a437 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -26,6 +26,9 @@ bug_fixes: - area: quic change: | Fixes access log formatter %CONNECTION_ID% for QUIC connections. +- area: c-ares + change: | + Applying a C-ares patch to fix DNS resoultion by the Google gRPC library. - area: ext_authz change: | Fixed fail-open behavior of the :ref:`failure_mode_allow config option @@ -77,6 +80,10 @@ removed_config_or_runtime: - area: stateful_session change: | Removed ``envoy.reloadable_features.stateful_session_encode_ttl_in_cookie`` runtime flag and legacy code paths. +- area: upstream flow control + change: | + Removed ``envoy.reloadable_features.upstream_wait_for_response_headers_before_disabling_read`` runtime flag + and legacy code paths. new_features: - area: tls @@ -108,5 +115,9 @@ new_features: change: | Added dynamic metadata matcher support :ref:`Dynamic metadata input ` and :ref:`Dynamic metadata input matcher `. +- area: access_log + change: | + Added new access log command operators ``%START_TIME_LOCAL%`` and ``%EMIT_TIME_LOCAL%``, + similar to ``%START_TIME%`` and ``%EMIT_TIME%``, but use local time zone. deprecated: diff --git a/docs/root/configuration/observability/access_log/usage.rst b/docs/root/configuration/observability/access_log/usage.rst index 6b9d0fadfcab..657c485a9b7d 100644 --- a/docs/root/configuration/observability/access_log/usage.rst +++ b/docs/root/configuration/observability/access_log/usage.rst @@ -170,6 +170,11 @@ The following command operators are supported: In typed JSON logs, START_TIME is always rendered as a string. +.. _config_access_log_format_start_time_local: + +%START_TIME_LOCAL% + Same as :ref:`START_TIME `, but use local time zone. + .. _config_access_log_format_emit_time: %EMIT_TIME% @@ -178,6 +183,11 @@ The following command operators are supported: EMIT_TIME can be customized using a `format string `_. See :ref:`START_TIME ` for additional format specifiers and examples. +.. _config_access_log_format_emit_time_local: + +%EMIT_TIME_LOCAL% + Same as :ref:`EMIT_TIME `, but use local time zone. + %REQUEST_HEADERS_BYTES% HTTP Uncompressed bytes of request headers. diff --git a/envoy/http/http_server_properties_cache.h b/envoy/http/http_server_properties_cache.h index c080a6ff6d4a..c980b178abe1 100644 --- a/envoy/http/http_server_properties_cache.h +++ b/envoy/http/http_server_properties_cache.h @@ -199,18 +199,5 @@ class HttpServerPropertiesCacheManager { using HttpServerPropertiesCacheManagerSharedPtr = std::shared_ptr; -/** - * Factory for getting an alternate protocols cache manager. - */ -class HttpServerPropertiesCacheManagerFactory { -public: - virtual ~HttpServerPropertiesCacheManagerFactory() = default; - - /** - * Get the alternate protocols cache manager. - */ - virtual HttpServerPropertiesCacheManagerSharedPtr get() PURE; -}; - } // namespace Http } // namespace Envoy diff --git a/envoy/server/BUILD b/envoy/server/BUILD index a412c4e164dd..0ea9b6a3b5de 100644 --- a/envoy/server/BUILD +++ b/envoy/server/BUILD @@ -181,6 +181,7 @@ envoy_cc_library( "//envoy/http:codes_interface", "//envoy/http:context_interface", "//envoy/http:filter_interface", + "//envoy/http:http_server_properties_cache_interface", "//envoy/init:manager_interface", "//envoy/local_info:local_info_interface", "//envoy/network:drain_decision_interface", diff --git a/envoy/server/factory_context.h b/envoy/server/factory_context.h index ff2fe0ab8253..6a4d77f3ad19 100644 --- a/envoy/server/factory_context.h +++ b/envoy/server/factory_context.h @@ -13,6 +13,7 @@ #include "envoy/http/codes.h" #include "envoy/http/context.h" #include "envoy/http/filter.h" +#include "envoy/http/http_server_properties_cache.h" #include "envoy/init/manager.h" #include "envoy/network/drain_decision.h" #include "envoy/network/filter.h" @@ -120,6 +121,11 @@ class CommonFactoryContext { */ virtual Upstream::ClusterManager& clusterManager() PURE; + /** + * @return const Http::HttpServerPropertiesCacheManager& instance for use by the entire server. + */ + virtual Http::HttpServerPropertiesCacheManager& httpServerPropertiesCacheManager() PURE; + /** * @return TimeSource& a reference to the time source. */ diff --git a/envoy/server/instance.h b/envoy/server/instance.h index 41f6448aec24..3b932858724f 100644 --- a/envoy/server/instance.h +++ b/envoy/server/instance.h @@ -13,6 +13,7 @@ #include "envoy/event/timer.h" #include "envoy/grpc/context.h" #include "envoy/http/context.h" +#include "envoy/http/http_server_properties_cache.h" #include "envoy/init/manager.h" #include "envoy/local_info/local_info.h" #include "envoy/network/listen_socket.h" @@ -71,6 +72,11 @@ class Instance { */ virtual const Upstream::ClusterManager& clusterManager() const PURE; + /** + * @return const Http::HttpServerPropertiesCacheManager& instance for use by the entire server. + */ + virtual Http::HttpServerPropertiesCacheManager& httpServerPropertiesCacheManager() PURE; + /** * @return Ssl::ContextManager& singleton for use by the entire server. */ diff --git a/mobile/examples/java/hello_world/MainActivity.java b/mobile/examples/java/hello_world/MainActivity.java index 78c0886ccee1..fcb9aa2bcf19 100644 --- a/mobile/examples/java/hello_world/MainActivity.java +++ b/mobile/examples/java/hello_world/MainActivity.java @@ -137,7 +137,7 @@ private void makeRequest() { return Unit.INSTANCE; }) .start(Executors.newSingleThreadExecutor()) - .sendHeaders(requestHeaders, true); + .sendHeaders(requestHeaders, /* endStream= */ true, /* idempotent= */ false); clear_text = !clear_text; } diff --git a/mobile/library/cc/stream.cc b/mobile/library/cc/stream.cc index edc7e22944b7..3bbb657fbea1 100644 --- a/mobile/library/cc/stream.cc +++ b/mobile/library/cc/stream.cc @@ -10,8 +10,8 @@ namespace Platform { Stream::Stream(InternalEngine* engine, envoy_stream_t handle) : engine_(engine), handle_(handle) {} -Stream& Stream::sendHeaders(Http::RequestHeaderMapPtr headers, bool end_stream) { - engine_->sendHeaders(handle_, std::move(headers), end_stream); +Stream& Stream::sendHeaders(Http::RequestHeaderMapPtr headers, bool end_stream, bool idempotent) { + engine_->sendHeaders(handle_, std::move(headers), end_stream, idempotent); return *this; } diff --git a/mobile/library/cc/stream.h b/mobile/library/cc/stream.h index 36e4c40e183c..b50da98d7a52 100644 --- a/mobile/library/cc/stream.h +++ b/mobile/library/cc/stream.h @@ -21,8 +21,11 @@ class Stream { * * @param headers the headers to send. * @param end_stream indicates whether to close the stream locally after sending this frame. + * @param idempotent indicates that the request is idempotent. When idempotent is set to true + * Envoy Mobile will retry on HTTP/3 post-handshake failures. By default, it is + * set to false. */ - Stream& sendHeaders(Http::RequestHeaderMapPtr headers, bool end_stream); + Stream& sendHeaders(Http::RequestHeaderMapPtr headers, bool end_stream, bool idempotent = false); /** * Send data over an open HTTP stream. This method can be invoked multiple times. diff --git a/mobile/library/common/http/client.cc b/mobile/library/common/http/client.cc index a83cd35b30f7..d0338595c6ff 100644 --- a/mobile/library/common/http/client.cc +++ b/mobile/library/common/http/client.cc @@ -11,7 +11,6 @@ #include "absl/strings/str_join.h" #include "absl/strings/string_view.h" #include "library/common/bridge/utility.h" -#include "library/common/http/header_utility.h" #include "library/common/stream_info/extra_stream_info.h" #include "library/common/system/system_helper.h" @@ -552,7 +551,8 @@ void Client::startStream(envoy_stream_t new_stream_handle, EnvoyStreamCallbacks& ENVOY_LOG(debug, "[S{}] start stream", new_stream_handle); } -void Client::sendHeaders(envoy_stream_t stream, RequestHeaderMapPtr headers, bool end_stream) { +void Client::sendHeaders(envoy_stream_t stream, RequestHeaderMapPtr headers, bool end_stream, + bool idempotent) { ASSERT(dispatcher_.isThreadSafe()); Client::DirectStreamSharedPtr direct_stream = getStream(stream, GetStreamFilters::AllowOnlyForOpenStreams); @@ -597,6 +597,12 @@ void Client::sendHeaders(envoy_stream_t stream, RequestHeaderMapPtr headers, boo // a request here: // https://github.com/envoyproxy/envoy/blob/c9e3b9d2c453c7fe56a0e3615f0c742ac0d5e768/source/common/router/config_impl.cc#L1091-L1096 headers->setReferenceForwardedProto(Headers::get().SchemeValues.Https); + // When the request is idempotent, it is safe to retry. + if (idempotent) { + // https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/router_filter#x-envoy-retry-on + headers->addCopy(Headers::get().EnvoyRetryOn, + Headers::get().EnvoyRetryOnValues.Http3PostConnectFailure); + } ENVOY_LOG(debug, "[S{}] request headers for stream (end_stream={}):\n{}", stream, end_stream, *headers); request_decoder->decodeHeaders(std::move(headers), end_stream); diff --git a/mobile/library/common/http/client.h b/mobile/library/common/http/client.h index 1c66baabb8c6..140e73e3205e 100644 --- a/mobile/library/common/http/client.h +++ b/mobile/library/common/http/client.h @@ -79,8 +79,12 @@ class Client : public Logger::Loggable { * @param stream the stream to send headers over. * @param headers the headers to send. * @param end_stream indicates whether to close the stream locally after sending this frame. + * @param idempotent indicates that the request is idempotent. When idempotent is set to true + * Envoy Mobile will retry on HTTP/3 post-handshake failures. By default, it is + * set to false. */ - void sendHeaders(envoy_stream_t stream, RequestHeaderMapPtr headers, bool end_stream); + void sendHeaders(envoy_stream_t stream, RequestHeaderMapPtr headers, bool end_stream, + bool idempotent = false); /** * Notify the stream that the caller is ready to receive more data from the response stream. Only diff --git a/mobile/library/common/internal_engine.cc b/mobile/library/common/internal_engine.cc index eb5084be17b5..587613480e30 100644 --- a/mobile/library/common/internal_engine.cc +++ b/mobile/library/common/internal_engine.cc @@ -58,10 +58,11 @@ envoy_status_t InternalEngine::startStream(envoy_stream_t stream, } envoy_status_t InternalEngine::sendHeaders(envoy_stream_t stream, Http::RequestHeaderMapPtr headers, - bool end_stream) { - return dispatcher_->post([this, stream, headers = std::move(headers), end_stream]() mutable { - http_client_->sendHeaders(stream, std::move(headers), end_stream); - }); + bool end_stream, bool idempotent) { + return dispatcher_->post( + [this, stream, headers = std::move(headers), end_stream, idempotent]() mutable { + http_client_->sendHeaders(stream, std::move(headers), end_stream, idempotent); + }); return ENVOY_SUCCESS; } diff --git a/mobile/library/common/internal_engine.h b/mobile/library/common/internal_engine.h index 327f5128cbaa..7a977d56f609 100644 --- a/mobile/library/common/internal_engine.h +++ b/mobile/library/common/internal_engine.h @@ -73,9 +73,12 @@ class InternalEngine : public Logger::Loggable { * @param stream the stream to send headers over. * @param headers the headers to send. * @param end_stream indicates whether to close the stream locally after sending this frame. + * @param idempotent indicates that the request is idempotent. When idempotent is set to true + * Envoy Mobile will retry on HTTP/3 post-handshake failures. By default, it is + * set to false. */ envoy_status_t sendHeaders(envoy_stream_t stream, Http::RequestHeaderMapPtr headers, - bool end_stream); + bool end_stream, bool idempotent = false); envoy_status_t readData(envoy_stream_t stream, size_t bytes_to_read); diff --git a/mobile/library/java/io/envoyproxy/envoymobile/engine/EnvoyHTTPStream.java b/mobile/library/java/io/envoyproxy/envoymobile/engine/EnvoyHTTPStream.java index c3c1d9481e7e..40253ee77ebf 100644 --- a/mobile/library/java/io/envoyproxy/envoymobile/engine/EnvoyHTTPStream.java +++ b/mobile/library/java/io/envoyproxy/envoymobile/engine/EnvoyHTTPStream.java @@ -38,11 +38,14 @@ public EnvoyHTTPStream(long engineHandle, long streamHandle, EnvoyHTTPCallbacks * Send headers over an open HTTP streamHandle. This method can be invoked once * and needs to be called before send_data. * - * @param headers, the headers to send. - * @param endStream, supplies whether this is headers only. + * @param headers the headers to send. + * @param endStream supplies whether this is headers only. + * @param idempotent indicates that the request is idempotent. When idempotent is set to true + * Envoy Mobile will retry on HTTP/3 post-handshake failures. */ - public void sendHeaders(Map> headers, boolean endStream) { - JniLibrary.sendHeaders(engineHandle, streamHandle, headers, endStream); + public void sendHeaders(Map> headers, boolean endStream, + boolean idempotent) { + JniLibrary.sendHeaders(engineHandle, streamHandle, headers, endStream, idempotent); } /** diff --git a/mobile/library/java/io/envoyproxy/envoymobile/engine/JniLibrary.java b/mobile/library/java/io/envoyproxy/envoymobile/engine/JniLibrary.java index fca73d8d2c0e..a4a2466145f5 100644 --- a/mobile/library/java/io/envoyproxy/envoymobile/engine/JniLibrary.java +++ b/mobile/library/java/io/envoyproxy/envoymobile/engine/JniLibrary.java @@ -73,14 +73,17 @@ protected static native int startStream(long engine, long stream, EnvoyHTTPCallb * Send headers over an open HTTP stream. This method can be invoked once and * needs to be called before send_data. * - * @param engine the stream's associated engine. - * @param stream the stream to send headers over. - * @param headers the headers to send. - * @param endStream supplies whether this is headers only. + * @param engine the stream's associated engine. + * @param stream the stream to send headers over. + * @param headers the headers to send. + * @param endStream supplies whether this is headers only. + * @param idempotent indicates that the request is idempotent. When idempotent is set to true + * Envoy Mobile will retry on HTTP/3 post-handshake failures. * @return the resulting status of the operation. */ protected static native int sendHeaders(long engine, long stream, - Map> headers, boolean endStream); + Map> headers, boolean endStream, + boolean idempotent); /** * Send data over an open HTTP stream. This method can be invoked multiple diff --git a/mobile/library/java/org/chromium/net/impl/CancelProofEnvoyStream.java b/mobile/library/java/org/chromium/net/impl/CancelProofEnvoyStream.java index b38cc6d580a7..e29c5a72d42f 100644 --- a/mobile/library/java/org/chromium/net/impl/CancelProofEnvoyStream.java +++ b/mobile/library/java/org/chromium/net/impl/CancelProofEnvoyStream.java @@ -61,7 +61,7 @@ void sendHeaders(Map> envoyRequestHeaders, boolean endStrea if (returnTrueIfCanceledOrIncreaseConcurrentlyRunningStreamOperations()) { return; // Already Cancelled - to late to send something. } - mStream.sendHeaders(envoyRequestHeaders, endStream); + mStream.sendHeaders(envoyRequestHeaders, endStream, /* idempotent= */ false); if (decreaseConcurrentlyRunningStreamOperationsAndReturnTrueIfAwaitingCancel()) { mStream.cancel(); // Cancel was called previously, so now this is honored. } diff --git a/mobile/library/java/org/chromium/net/impl/CronvoyUrlRequest.java b/mobile/library/java/org/chromium/net/impl/CronvoyUrlRequest.java index 5873c3897ba6..8d51147e4c00 100644 --- a/mobile/library/java/org/chromium/net/impl/CronvoyUrlRequest.java +++ b/mobile/library/java/org/chromium/net/impl/CronvoyUrlRequest.java @@ -180,6 +180,7 @@ public final class CronvoyUrlRequest extends CronvoyUrlRequestBase { private String mCurrentUrl; private volatile CronvoyUrlResponseInfoImpl mUrlResponseInfo; private String mPendingRedirectUrl; + private boolean mIdempotent; /** * @param executor The executor for orchestrating tasks between envoy-mobile callbacks @@ -188,7 +189,7 @@ public final class CronvoyUrlRequest extends CronvoyUrlRequestBase { Executor executor, String userAgent, boolean allowDirectExecutor, Collection connectionAnnotations, boolean trafficStatsTagSet, int trafficStatsTag, boolean trafficStatsUidSet, int trafficStatsUid, - RequestFinishedInfo.Listener requestFinishedListener) { + RequestFinishedInfo.Listener requestFinishedListener, boolean idempotent) { if (url == null) { throw new NullPointerException("URL is required"); } @@ -210,6 +211,7 @@ public final class CronvoyUrlRequest extends CronvoyUrlRequestBase { mCurrentUrl = url; mUserAgent = userAgent; mRequestAnnotations = connectionAnnotations; + mIdempotent = idempotent; } @Override @@ -538,7 +540,7 @@ private void fireOpenConnection() { mCronvoyCallbacks = new CronvoyHttpCallbacks(); mStream.set(mRequestContext.getEnvoyEngine().startStream(mCronvoyCallbacks, /* explicitFlowControl= */ true)); - mStream.get().sendHeaders(envoyRequestHeaders, mUploadDataStream == null); + mStream.get().sendHeaders(envoyRequestHeaders, mUploadDataStream == null, mIdempotent); if (mUploadDataStream != null && mUrlChain.size() == 1) { mUploadDataStream.initializeWithRequest(); } diff --git a/mobile/library/java/org/chromium/net/impl/CronvoyUrlRequestContext.java b/mobile/library/java/org/chromium/net/impl/CronvoyUrlRequestContext.java index bf6e5ae60643..3633e11edfeb 100644 --- a/mobile/library/java/org/chromium/net/impl/CronvoyUrlRequestContext.java +++ b/mobile/library/java/org/chromium/net/impl/CronvoyUrlRequestContext.java @@ -23,6 +23,7 @@ import java.util.concurrent.atomic.AtomicReference; import org.chromium.net.BidirectionalStream; import org.chromium.net.ExperimentalBidirectionalStream; +import org.chromium.net.ExperimentalUrlRequest; import org.chromium.net.NetworkQualityRttListener; import org.chromium.net.NetworkQualityThroughputListener; import org.chromium.net.RequestFinishedInfo; @@ -132,7 +133,8 @@ void setTaskToExecuteWhenInitializationIsCompleted(Runnable runnable) { checkHaveAdapter(); return new CronvoyUrlRequest(this, url, callback, executor, mUserAgent, allowDirectExecutor, requestAnnotations, trafficStatsTagSet, trafficStatsTag, - trafficStatsUidSet, trafficStatsUid, requestFinishedListener); + trafficStatsUidSet, trafficStatsUid, requestFinishedListener, + idempotency == ExperimentalUrlRequest.Builder.IDEMPOTENT); } } diff --git a/mobile/library/jni/jni_helper.cc b/mobile/library/jni/jni_helper.cc index 7b5e515dd518..1dd285e1116b 100644 --- a/mobile/library/jni/jni_helper.cc +++ b/mobile/library/jni/jni_helper.cc @@ -16,10 +16,19 @@ constexpr const char* THREAD_NAME = "EnvoyMain"; // Non-const variables. std::atomic java_vm_cache_; thread_local JNIEnv* jni_env_cache_ = nullptr; +// `jclass_cache_map` contains `jclass` references that are statically populated. This field is +// used by `FindClass` to find the `jclass` reference from a given class name. absl::flat_hash_map jclass_cache_map; // The `jclass_cache_set` is a superset of `jclass_cache_map`. It contains `jclass` objects that are // retrieve dynamically via `GetObjectClass`. -thread_local absl::flat_hash_set jclass_cache_set; +// +// The `jclass_cache_set` owns the `jclass` global refs, wrapped in `GlobalRefUniquePtr` to allow +// automatic `GlobalRef` destruction. The other fields, such as `jmethod_id_cache_map`, +// `static_jmethod_id_cache_map`, `jfield_id_cache_map`, and `static_jfield_id_cache_map` only +// borrow the `jclass` references. +// +// Note: all these fields are `thread_local` to avoid locking. +thread_local absl::flat_hash_set> jclass_cache_set; thread_local absl::flat_hash_map< std::tuple, jmethodID> @@ -43,13 +52,37 @@ thread_local absl::flat_hash_map< jclass addClassToCacheIfNotExist(JNIEnv* env, jclass clazz) { jclass java_class_global_ref = clazz; if (auto it = jclass_cache_set.find(clazz); it == jclass_cache_set.end()) { - jclass global_ref = reinterpret_cast(env->NewGlobalRef(clazz)); - jclass_cache_set.emplace(global_ref); + java_class_global_ref = reinterpret_cast(env->NewGlobalRef(clazz)); + jclass_cache_set.emplace(java_class_global_ref, GlobalRefDeleter()); } return java_class_global_ref; } } // namespace +void GlobalRefDeleter::operator()(jobject object) const { + if (object != nullptr) { + JniHelper::getThreadLocalEnv()->DeleteGlobalRef(object); + } +} + +void LocalRefDeleter::operator()(jobject object) const { + if (object != nullptr) { + JniHelper::getThreadLocalEnv()->DeleteLocalRef(object); + } +} + +void StringUtfDeleter::operator()(const char* c_str) const { + if (c_str != nullptr) { + JniHelper::getThreadLocalEnv()->ReleaseStringUTFChars(j_str_, c_str); + } +} + +void PrimitiveArrayCriticalDeleter::operator()(void* c_array) const { + if (c_array != nullptr) { + JniHelper::getThreadLocalEnv()->ReleasePrimitiveArrayCritical(array_, c_array, 0); + } +} + jint JniHelper::getVersion() { return JNI_VERSION; } void JniHelper::initialize(JavaVM* java_vm) { @@ -67,7 +100,7 @@ void JniHelper::finalize() { static_jfield_id_cache_map.clear(); jclass_cache_map.clear(); for (const auto& clazz : jclass_cache_set) { - env->DeleteGlobalRef(clazz); + env->DeleteGlobalRef(clazz.get()); } jclass_cache_set.clear(); } @@ -80,7 +113,7 @@ void JniHelper::addClassToCache(const char* class_name) { ASSERT(java_class != nullptr, absl::StrFormat("Unable to find class '%s'.", class_name)); jclass java_class_global_ref = reinterpret_cast(env->NewGlobalRef(java_class)); jclass_cache_map.emplace(class_name, java_class_global_ref); - jclass_cache_set.emplace(java_class_global_ref); + jclass_cache_set.emplace(java_class_global_ref, GlobalRefDeleter()); } JavaVM* JniHelper::getJavaVm() { return java_vm_cache_.load(std::memory_order_acquire); } @@ -90,18 +123,15 @@ void JniHelper::detachCurrentThread() { } JNIEnv* JniHelper::getThreadLocalEnv() { - if (jni_env_cache_ != nullptr) { - return jni_env_cache_; - } JavaVM* java_vm = getJavaVm(); ASSERT(java_vm != nullptr, "Unable to get JavaVM."); jint result = java_vm->GetEnv(reinterpret_cast(&jni_env_cache_), getVersion()); if (result == JNI_EDETACHED) { JavaVMAttachArgs args = {getVersion(), const_cast(THREAD_NAME), nullptr}; #if defined(__ANDROID__) - result = java_vm->AttachCurrentThread(&jni_env_cache_, &args); + result = java_vm->AttachCurrentThreadAsDaemon(&jni_env_cache_, &args); #else - result = java_vm->AttachCurrentThread(reinterpret_cast(&jni_env_cache_), &args); + result = java_vm->AttachCurrentThreadAsDaemon(reinterpret_cast(&jni_env_cache_), &args); #endif } ASSERT(result == JNI_OK, "Unable to get JNIEnv."); @@ -193,7 +223,7 @@ jclass JniHelper::findClass(const char* class_name) { } LocalRefUniquePtr JniHelper::getObjectClass(jobject object) { - return {env_->GetObjectClass(object), LocalRefDeleter(env_)}; + return {env_->GetObjectClass(object), LocalRefDeleter()}; } void JniHelper::throwNew(const char* java_class_name, const char* message) { @@ -207,34 +237,33 @@ void JniHelper::throwNew(const char* java_class_name, const char* message) { jboolean JniHelper::exceptionCheck() { return env_->ExceptionCheck(); } LocalRefUniquePtr JniHelper::exceptionOccurred() { - return {env_->ExceptionOccurred(), LocalRefDeleter(env_)}; + return {env_->ExceptionOccurred(), LocalRefDeleter()}; } void JniHelper::exceptionCleared() { env_->ExceptionClear(); } GlobalRefUniquePtr JniHelper::newGlobalRef(jobject object) { - GlobalRefUniquePtr result(env_->NewGlobalRef(object), GlobalRefDeleter(env_)); + GlobalRefUniquePtr result(env_->NewGlobalRef(object), GlobalRefDeleter()); return result; } LocalRefUniquePtr JniHelper::newObject(jclass clazz, jmethodID method_id, ...) { va_list args; va_start(args, method_id); - LocalRefUniquePtr result(env_->NewObjectV(clazz, method_id, args), - LocalRefDeleter(env_)); + LocalRefUniquePtr result(env_->NewObjectV(clazz, method_id, args), LocalRefDeleter()); rethrowException(); va_end(args); return result; } LocalRefUniquePtr JniHelper::newStringUtf(const char* str) { - LocalRefUniquePtr result(env_->NewStringUTF(str), LocalRefDeleter(env_)); + LocalRefUniquePtr result(env_->NewStringUTF(str), LocalRefDeleter()); rethrowException(); return result; } StringUtfUniquePtr JniHelper::getStringUtfChars(jstring str, jboolean* is_copy) { - StringUtfUniquePtr result(env_->GetStringUTFChars(str, is_copy), StringUtfDeleter(env_, str)); + StringUtfUniquePtr result(env_->GetStringUTFChars(str, is_copy), StringUtfDeleter(str)); rethrowException(); return result; } @@ -243,8 +272,7 @@ jsize JniHelper::getArrayLength(jarray array) { return env_->GetArrayLength(arra #define DEFINE_NEW_ARRAY(JAVA_TYPE, JNI_TYPE) \ LocalRefUniquePtr JniHelper::new##JAVA_TYPE##Array(jsize length) { \ - LocalRefUniquePtr result(env_->New##JAVA_TYPE##Array(length), \ - LocalRefDeleter(env_)); \ + LocalRefUniquePtr result(env_->New##JAVA_TYPE##Array(length), LocalRefDeleter()); \ rethrowException(); \ return result; \ } @@ -261,7 +289,7 @@ DEFINE_NEW_ARRAY(Boolean, jbooleanArray) LocalRefUniquePtr JniHelper::newObjectArray(jsize length, jclass element_class, jobject initial_element) { LocalRefUniquePtr result( - env_->NewObjectArray(length, element_class, initial_element), LocalRefDeleter(env_)); + env_->NewObjectArray(length, element_class, initial_element), LocalRefDeleter()); return result; } @@ -362,7 +390,7 @@ void JniHelper::callStaticVoidMethod(jclass clazz, jmethodID method_id, ...) { LocalRefUniquePtr JniHelper::newDirectByteBuffer(void* address, jlong capacity) { LocalRefUniquePtr result(env_->NewDirectByteBuffer(address, capacity), - LocalRefDeleter(env_)); + LocalRefDeleter()); rethrowException(); return result; } diff --git a/mobile/library/jni/jni_helper.h b/mobile/library/jni/jni_helper.h index d21b33804a6b..cb3b07f20b84 100644 --- a/mobile/library/jni/jni_helper.h +++ b/mobile/library/jni/jni_helper.h @@ -10,16 +10,14 @@ namespace JNI { /** A custom deleter to delete JNI global ref. */ class GlobalRefDeleter { public: - explicit GlobalRefDeleter(JNIEnv* env) : env_(env) {} + explicit GlobalRefDeleter() = default; - void operator()(jobject object) const { - if (object != nullptr) { - env_->DeleteGlobalRef(object); - } - } + GlobalRefDeleter(const GlobalRefDeleter&) = default; -private: - JNIEnv* const env_; + // This is to allow move semantics in `GlobalRefUniquePtr`. + GlobalRefDeleter& operator=(const GlobalRefDeleter&) = default; + + void operator()(jobject object) const; }; /** A unique pointer for JNI global ref. */ @@ -29,21 +27,14 @@ using GlobalRefUniquePtr = std::unique_ptr::type /** A custom deleter to delete JNI local ref. */ class LocalRefDeleter { public: - explicit LocalRefDeleter(JNIEnv* env) : env_(env) {} + explicit LocalRefDeleter() = default; LocalRefDeleter(const LocalRefDeleter&) = default; // This is to allow move semantics in `LocalRefUniquePtr`. - LocalRefDeleter& operator=(const LocalRefDeleter&) { return *this; } - - void operator()(jobject object) const { - if (object != nullptr) { - env_->DeleteLocalRef(object); - } - } + LocalRefDeleter& operator=(const LocalRefDeleter&) = default; -private: - JNIEnv* const env_; + void operator()(jobject object) const; }; /** A unique pointer for JNI local ref. */ @@ -53,16 +44,11 @@ using LocalRefUniquePtr = std::unique_ptr::type, /** A custom deleter for UTF strings. */ class StringUtfDeleter { public: - StringUtfDeleter(JNIEnv* env, jstring j_str) : env_(env), j_str_(j_str) {} + explicit StringUtfDeleter(jstring j_str) : j_str_(j_str) {} - void operator()(const char* c_str) const { - if (c_str != nullptr) { - env_->ReleaseStringUTFChars(j_str_, c_str); - } - } + void operator()(const char* c_str) const; private: - JNIEnv* const env_; jstring j_str_; }; @@ -111,16 +97,11 @@ using ArrayElementsUniquePtr = std::unique_ptr< /** A custom deleter for JNI primitive array critical. */ class PrimitiveArrayCriticalDeleter { public: - PrimitiveArrayCriticalDeleter(JNIEnv* env, jarray array) : env_(env), array_(array) {} + explicit PrimitiveArrayCriticalDeleter(jarray array) : array_(array) {} - void operator()(void* c_array) const { - if (c_array != nullptr) { - env_->ReleasePrimitiveArrayCritical(array_, c_array, 0); - } - } + void operator()(void* c_array) const; private: - JNIEnv* const env_; jarray array_; }; @@ -221,7 +202,7 @@ class JniHelper { template [[nodiscard]] LocalRefUniquePtr getObjectField(jobject object, jfieldID field_id) { LocalRefUniquePtr result(static_cast(env_->GetObjectField(object, field_id)), - LocalRefDeleter(env_)); + LocalRefDeleter()); return result; } @@ -360,7 +341,7 @@ class JniHelper { template [[nodiscard]] LocalRefUniquePtr getObjectArrayElement(jobjectArray array, jsize index) { LocalRefUniquePtr result(static_cast(env_->GetObjectArrayElement(array, index)), - LocalRefDeleter(env_)); + LocalRefDeleter()); rethrowException(); return result; } @@ -382,7 +363,7 @@ class JniHelper { jboolean* is_copy) { PrimitiveArrayCriticalUniquePtr result( static_cast(env_->GetPrimitiveArrayCritical(array, is_copy)), - PrimitiveArrayCriticalDeleter(env_, array)); + PrimitiveArrayCriticalDeleter(array)); return result; } @@ -429,7 +410,7 @@ class JniHelper { va_list args; va_start(args, method_id); LocalRefUniquePtr result(static_cast(env_->CallObjectMethodV(object, method_id, args)), - LocalRefDeleter(env_)); + LocalRefDeleter()); va_end(args); rethrowException(); return result; @@ -461,8 +442,7 @@ class JniHelper { va_list args; va_start(args, method_id); LocalRefUniquePtr result( - static_cast(env_->CallStaticObjectMethodV(clazz, method_id, args)), - LocalRefDeleter(env_)); + static_cast(env_->CallStaticObjectMethodV(clazz, method_id, args)), LocalRefDeleter()); va_end(args); rethrowException(); return result; diff --git a/mobile/library/jni/jni_impl.cc b/mobile/library/jni/jni_impl.cc index 25eced8b83b9..847182a8c241 100644 --- a/mobile/library/jni/jni_impl.cc +++ b/mobile/library/jni/jni_impl.cc @@ -565,8 +565,8 @@ jvm_http_filter_on_resume(const char* method, envoy_headers* headers, envoy_data headers_length = static_cast(headers->length); passHeaders("passHeader", *headers, j_context); } - Envoy::JNI::LocalRefUniquePtr j_in_data = Envoy::JNI::LocalRefUniquePtr( - nullptr, Envoy::JNI::LocalRefDeleter(jni_helper.getEnv())); + Envoy::JNI::LocalRefUniquePtr j_in_data = + Envoy::JNI::LocalRefUniquePtr(nullptr, Envoy::JNI::LocalRefDeleter()); if (data) { j_in_data = Envoy::JNI::envoyDataToJavaByteBuffer(jni_helper, *data); } @@ -1017,12 +1017,13 @@ extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibra extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibrary_sendHeaders( JNIEnv* env, jclass, jlong engine_handle, jlong stream_handle, jobject headers, - jboolean end_stream) { + jboolean end_stream, jboolean idempotent) { Envoy::JNI::JniHelper jni_helper(env); auto cpp_headers = Envoy::Http::Utility::createRequestHeaderMapPtr(); Envoy::JNI::javaHeadersToCppHeaders(jni_helper, headers, *cpp_headers); return reinterpret_cast(engine_handle) - ->sendHeaders(static_cast(stream_handle), std::move(cpp_headers), end_stream); + ->sendHeaders(static_cast(stream_handle), std::move(cpp_headers), end_stream, + idempotent); } extern "C" JNIEXPORT jint JNICALL Java_io_envoyproxy_envoymobile_engine_JniLibrary_sendTrailers( diff --git a/mobile/library/kotlin/io/envoyproxy/envoymobile/Stream.kt b/mobile/library/kotlin/io/envoyproxy/envoymobile/Stream.kt index 056df5a52d9a..d7df21c35563 100644 --- a/mobile/library/kotlin/io/envoyproxy/envoymobile/Stream.kt +++ b/mobile/library/kotlin/io/envoyproxy/envoymobile/Stream.kt @@ -17,10 +17,16 @@ open class Stream( * * @param headers Headers to send over the stream. * @param endStream Whether this is a headers-only request. + * @param idempotent indicates that the request is idempotent. When idempotent is set to true + * Envoy Mobile will retry on HTTP/3 post-handshake failures. By default, it is set to false. * @return This stream, for chaining syntax. */ - open fun sendHeaders(headers: RequestHeaders, endStream: Boolean): Stream { - underlyingStream.sendHeaders(headers.caseSensitiveHeaders(), endStream) + open fun sendHeaders( + headers: RequestHeaders, + endStream: Boolean, + idempotent: Boolean = false + ): Stream { + underlyingStream.sendHeaders(headers.caseSensitiveHeaders(), endStream, idempotent) return this } diff --git a/mobile/test/common/http/client_test.cc b/mobile/test/common/http/client_test.cc index e6f2d29f8a70..bc7d4a61c7e7 100644 --- a/mobile/test/common/http/client_test.cc +++ b/mobile/test/common/http/client_test.cc @@ -212,6 +212,34 @@ TEST_P(ClientTest, BasicStreamHeaders) { ASSERT_EQ(callbacks_called.on_complete_calls_, 1); } +TEST_P(ClientTest, BasicStreamHeadersIdempotent) { + // Create a stream, and set up request_decoder_ and response_encoder_ + StreamCallbacksCalled callbacks_called; + createStream(createDefaultStreamCallbacks(callbacks_called)); + + // Send request headers. + EXPECT_CALL(dispatcher_, pushTrackedObject(_)); + EXPECT_CALL(dispatcher_, popTrackedObject(_)); + TestRequestHeaderMapImpl expected_headers; + HttpTestUtility::addDefaultHeaders(expected_headers); + expected_headers.addCopy("x-envoy-mobile-cluster", "base_clear"); + expected_headers.addCopy("x-forwarded-proto", "https"); + expected_headers.addCopy("x-envoy-retry-on", "http3-post-connect-failure"); + EXPECT_CALL(*request_decoder_, decodeHeaders_(HeaderMapEqual(&expected_headers), true)); + http_client_.sendHeaders(stream_, createDefaultRequestHeaders(), /* end_stream= */ true, + /* idempotent= */ true); + + // Encode response headers. + EXPECT_CALL(dispatcher_, pushTrackedObject(_)); + EXPECT_CALL(dispatcher_, popTrackedObject(_)); + EXPECT_CALL(dispatcher_, deferredDelete_(_)); + TestResponseHeaderMapImpl response_headers{{":status", "200"}}; + response_encoder_->encodeHeaders(response_headers, true); + ASSERT_EQ(callbacks_called.on_headers_calls_, 1); + // Ensure that the callbacks on the EnvoyStreamCallbacks were called. + ASSERT_EQ(callbacks_called.on_complete_calls_, 1); +} + TEST_P(ClientTest, BasicStreamData) { StreamCallbacksCalled callbacks_called; callbacks_called.end_stream_with_headers_ = false; diff --git a/mobile/test/java/integration/AndroidEngineExplicitFlowTest.java b/mobile/test/java/integration/AndroidEngineExplicitFlowTest.java index a01ed6ed9fa8..e6e19862933a 100644 --- a/mobile/test/java/integration/AndroidEngineExplicitFlowTest.java +++ b/mobile/test/java/integration/AndroidEngineExplicitFlowTest.java @@ -585,7 +585,8 @@ private Response sendRequest(RequestScenario requestScenario) throws Exception { .start(requestScenario.useDirectExecutor ? Runnable::run : Executors.newSingleThreadExecutor()); streamRef.set(stream); // Set before sending headers to avoid race conditions. - stream.sendHeaders(requestScenario.getHeaders(), !requestScenario.hasBody()); + stream.sendHeaders(requestScenario.getHeaders(), !requestScenario.hasBody(), + /* idempotent= */ false); if (requestScenario.hasBody()) { // The first "send" is assumes that the window is available - API contract. onSendWindowAvailable(requestScenario, streamRef.get(), chunkIterator, response.get()); diff --git a/mobile/test/java/integration/AndroidEngineFlowTest.java b/mobile/test/java/integration/AndroidEngineFlowTest.java index f28eba1c14b3..469191664a56 100644 --- a/mobile/test/java/integration/AndroidEngineFlowTest.java +++ b/mobile/test/java/integration/AndroidEngineFlowTest.java @@ -305,7 +305,8 @@ private Response sendRequest(RequestScenario requestScenario) throws Exception { return null; }) .start(Executors.newSingleThreadExecutor()) - .sendHeaders(requestScenario.getHeaders(), !requestScenario.hasBody()); + .sendHeaders(requestScenario.getHeaders(), !requestScenario.hasBody(), + /* idempotent= */ false); if (requestScenario.cancelBeforeSendingRequestBody) { stream.cancel(); } else { diff --git a/mobile/test/java/integration/AndroidEngineSocketTagTest.java b/mobile/test/java/integration/AndroidEngineSocketTagTest.java index f90ec51dbc4f..c61b7442e7fb 100644 --- a/mobile/test/java/integration/AndroidEngineSocketTagTest.java +++ b/mobile/test/java/integration/AndroidEngineSocketTagTest.java @@ -142,7 +142,8 @@ private Response sendRequest(RequestScenario requestScenario) throws Exception { .start(requestScenario.useDirectExecutor ? Runnable::run : Executors.newSingleThreadExecutor()); streamRef.set(stream); // Set before sending headers to avoid race conditions. - stream.sendHeaders(requestScenario.getHeaders(), !requestScenario.hasBody()); + stream.sendHeaders(requestScenario.getHeaders(), !requestScenario.hasBody(), + /* idempotent= */ false); latch.await(); response.get().throwAssertionErrorIfAny(); return response.get(); diff --git a/mobile/test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServerTest.java b/mobile/test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServerTest.java index 68a74dcb91fb..654fd86e3d94 100644 --- a/mobile/test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServerTest.java +++ b/mobile/test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServerTest.java @@ -122,7 +122,8 @@ private Response sendRequest(RequestScenario requestScenario) throws Exception { return null; }) .start(Executors.newSingleThreadExecutor()) - .sendHeaders(requestScenario.getHeaders(), !requestScenario.hasBody()); + .sendHeaders(requestScenario.getHeaders(), !requestScenario.hasBody(), + /* idempotent= */ false); requestScenario.getBodyChunks().forEach(stream::sendData); requestScenario.getClosingBodyChunk().ifPresent(stream::close); latch.await(); diff --git a/mobile/test/java/org/chromium/net/impl/CancelProofEnvoyStreamTest.java b/mobile/test/java/org/chromium/net/impl/CancelProofEnvoyStreamTest.java index 64d18abd59e2..e39bbd6b8241 100644 --- a/mobile/test/java/org/chromium/net/impl/CancelProofEnvoyStreamTest.java +++ b/mobile/test/java/org/chromium/net/impl/CancelProofEnvoyStreamTest.java @@ -358,7 +358,8 @@ private class MockedStream extends EnvoyHTTPStream { private MockedStream() { super(0, 0, null, false); } @Override - public void sendHeaders(Map> headers, boolean endStream) { + public void sendHeaders(Map> headers, boolean endStream, + boolean idempotent) { mSendHeadersInvocationCount.incrementAndGet(); mStartLatch.countDown(); mSendHeadersBlock.block(); diff --git a/mobile/test/java/org/chromium/net/testing/AndroidEnvoyExplicitH2FlowTest.java b/mobile/test/java/org/chromium/net/testing/AndroidEnvoyExplicitH2FlowTest.java index cf5d6b2a774f..6dc019d445a3 100644 --- a/mobile/test/java/org/chromium/net/testing/AndroidEnvoyExplicitH2FlowTest.java +++ b/mobile/test/java/org/chromium/net/testing/AndroidEnvoyExplicitH2FlowTest.java @@ -105,7 +105,7 @@ public void continuousWrite_withCancelOnResponseHeaders() throws Exception { return null; }) .start(Runnable::run) // direct executor - all the logic runs on the EM Network Thread. - .sendHeaders(requestHeaders, false)); + .sendHeaders(requestHeaders, /* endStream= */ false, /* idempotent= */ false)); ByteBuffer bf = ByteBuffer.allocateDirect(1); bf.put((byte)'a'); stream.get().sendData(bf); diff --git a/mobile/test/java/org/chromium/net/testing/Http2TestServerTest.java b/mobile/test/java/org/chromium/net/testing/Http2TestServerTest.java index cbcd6fd1af14..80acf38fab33 100644 --- a/mobile/test/java/org/chromium/net/testing/Http2TestServerTest.java +++ b/mobile/test/java/org/chromium/net/testing/Http2TestServerTest.java @@ -132,7 +132,7 @@ public void testGetRequestWithPlatformCertValidatorFail() throws Exception { }) .setOnCancel((ignored) -> { throw new AssertionError("Unexpected OnCancel called."); }) .start(Executors.newSingleThreadExecutor()) - .sendHeaders(requestScenario.getHeaders(), false); + .sendHeaders(requestScenario.getHeaders(), /* endStream= */ false, /* idempotent= */ false); latch.await(); assertNotNull(response.get().getEnvoyError()); @@ -169,7 +169,7 @@ public void testSubjectAltNameErrorWithPlatformCertValidator() throws Exception }) .setOnCancel((ignored) -> { throw new AssertionError("Unexpected OnCancel called."); }) .start(Executors.newSingleThreadExecutor()) - .sendHeaders(requestScenario.getHeaders(), false); + .sendHeaders(requestScenario.getHeaders(), /* endStream= */ false, /* idempotent= */ false); latch.await(); assertNotNull(response.get().getEnvoyError()); @@ -220,7 +220,8 @@ private Response sendRequest(RequestScenario requestScenario) throws Exception { return null; }) .start(Executors.newSingleThreadExecutor()) - .sendHeaders(requestScenario.getHeaders(), /* hasRequestBody= */ false); + .sendHeaders(requestScenario.getHeaders(), /* hasRequestBody= */ false, + /* idempotent= */ false); latch.await(); response.get().throwAssertionErrorIfAny(); diff --git a/mobile/test/kotlin/io/envoyproxy/envoymobile/mocks/MockEnvoyHTTPStream.kt b/mobile/test/kotlin/io/envoyproxy/envoymobile/mocks/MockEnvoyHTTPStream.kt index 7e9a8ac6e6f6..4fdb68f5121f 100644 --- a/mobile/test/kotlin/io/envoyproxy/envoymobile/mocks/MockEnvoyHTTPStream.kt +++ b/mobile/test/kotlin/io/envoyproxy/envoymobile/mocks/MockEnvoyHTTPStream.kt @@ -11,7 +11,11 @@ import java.nio.ByteBuffer */ class MockEnvoyHTTPStream(val callbacks: EnvoyHTTPCallbacks, val explicitFlowControl: Boolean) : EnvoyHTTPStream(0, 0, callbacks, explicitFlowControl) { - override fun sendHeaders(headers: MutableMap>?, endStream: Boolean) {} + override fun sendHeaders( + headers: MutableMap>?, + endStream: Boolean, + idempotent: Boolean + ) {} override fun sendData(data: ByteBuffer?, endStream: Boolean) {} diff --git a/mobile/test/kotlin/io/envoyproxy/envoymobile/mocks/MockStream.kt b/mobile/test/kotlin/io/envoyproxy/envoymobile/mocks/MockStream.kt index 838ce7bb3ebf..a3dd5fbab43e 100644 --- a/mobile/test/kotlin/io/envoyproxy/envoymobile/mocks/MockStream.kt +++ b/mobile/test/kotlin/io/envoyproxy/envoymobile/mocks/MockStream.kt @@ -24,7 +24,9 @@ class MockStream(underlyingStream: MockEnvoyHTTPStream) : EnvoyFinalStreamIntel(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, false, 0, 0, 0, 0) /** Closure that will be called when request headers are sent. */ - var onRequestHeaders: ((headers: RequestHeaders, endStream: Boolean) -> Unit)? = null + var onRequestHeaders: + ((headers: RequestHeaders, endStream: Boolean, idempotent: Boolean) -> Unit)? = + null /** Closure that will be called when request data is sent. */ var onRequestData: ((data: ByteBuffer, endStream: Boolean) -> Unit)? = null /** Closure that will be called when request trailers are sent. */ @@ -32,8 +34,12 @@ class MockStream(underlyingStream: MockEnvoyHTTPStream) : /** Closure that will be called when the stream is canceled by the client. */ var onCancel: (() -> Unit)? = null - override fun sendHeaders(headers: RequestHeaders, endStream: Boolean): Stream { - onRequestHeaders?.invoke(headers, endStream) + override fun sendHeaders( + headers: RequestHeaders, + endStream: Boolean, + idempotent: Boolean + ): Stream { + onRequestHeaders?.invoke(headers, endStream, idempotent) return this } diff --git a/source/common/common/utility.cc b/source/common/common/utility.cc index de988db79f42..463683960a20 100644 --- a/source/common/common/utility.cc +++ b/source/common/common/utility.cc @@ -92,7 +92,8 @@ std::string DateFormatter::fromTime(const SystemTime& time) const { // A map is used to keep different formatted format strings at a given second. absl::node_hash_map formatted; }; - static thread_local CachedTime cached_time; + static thread_local CachedTime cached_time_utc; + static thread_local CachedTime cached_time_local; const std::chrono::nanoseconds epoch_time_ns = std::chrono::duration_cast(time.time_since_epoch()); @@ -100,6 +101,7 @@ std::string DateFormatter::fromTime(const SystemTime& time) const { const std::chrono::seconds epoch_time_seconds = std::chrono::duration_cast(epoch_time_ns); + CachedTime& cached_time = local_time_ ? cached_time_local : cached_time_utc; const auto& item = cached_time.formatted.find(raw_format_string_); if (item == cached_time.formatted.end() || item->second.epoch_time_seconds != epoch_time_seconds) { @@ -203,7 +205,8 @@ DateFormatter::fromTimeAndPrepareSpecifierOffsets(time_t time, SpecifierOffsets& specifier_offsets.reserve(specifiers_.size()); for (const auto& specifier : specifiers_) { std::string current_format = - absl::FormatTime(specifier.segment_, absl::FromTimeT(time), absl::UTCTimeZone()); + absl::FormatTime(specifier.segment_, absl::FromTimeT(time), + local_time_ ? absl::LocalTimeZone() : absl::UTCTimeZone()); absl::StrAppend(&formatted_time, current_format, specifier.second_ ? seconds_str : std::string(specifier.width_, '?')); @@ -501,16 +504,22 @@ void StringUtil::escapeToOstream(std::ostream& os, absl::string_view view) { } } -const std::string& getDefaultDateFormat() { +const std::string& getDefaultDateFormat(bool local_time) { + if (local_time) { + CONSTRUCT_ON_FIRST_USE(std::string, "%Y-%m-%dT%H:%M:%E3S%z"); + } CONSTRUCT_ON_FIRST_USE(std::string, "%Y-%m-%dT%H:%M:%E3SZ"); } -std::string AccessLogDateTimeFormatter::fromTime(const SystemTime& system_time) { +std::string AccessLogDateTimeFormatter::fromTime(const SystemTime& system_time, bool local_time) { struct CachedTime { std::chrono::seconds epoch_time_seconds; std::string formatted_time; }; - static thread_local CachedTime cached_time; + static thread_local CachedTime cached_time_utc; + static thread_local CachedTime cached_time_local; + + CachedTime& cached_time = local_time ? cached_time_local : cached_time_utc; const std::chrono::milliseconds epoch_time_ms = std::chrono::duration_cast(system_time.time_since_epoch()); @@ -519,14 +528,15 @@ std::string AccessLogDateTimeFormatter::fromTime(const SystemTime& system_time) std::chrono::duration_cast(epoch_time_ms); if (cached_time.formatted_time.empty() || cached_time.epoch_time_seconds != epoch_time_seconds) { - cached_time.formatted_time = absl::FormatTime( - getDefaultDateFormat(), absl::FromChrono(system_time), absl::UTCTimeZone()); + cached_time.formatted_time = + absl::FormatTime(getDefaultDateFormat(local_time), absl::FromChrono(system_time), + local_time ? absl::LocalTimeZone() : absl::UTCTimeZone()); cached_time.epoch_time_seconds = epoch_time_seconds; } else { - // Overwrite the digits in the ".000Z" at the end of the string with the + // Overwrite the digits in the ".000Z" or ".000%z" at the end of the string with the // millisecond count from the input time. - ASSERT(cached_time.formatted_time.length() == 24); - size_t offset = cached_time.formatted_time.length() - 4; + ASSERT(cached_time.formatted_time.length() >= 23); + size_t offset = 20; uint32_t msec = epoch_time_ms.count() % 1000; cached_time.formatted_time[offset++] = ('0' + (msec / 100)); msec %= 100; diff --git a/source/common/common/utility.h b/source/common/common/utility.h index 5e1562eda35f..872904b6a47a 100644 --- a/source/common/common/utility.h +++ b/source/common/common/utility.h @@ -44,12 +44,14 @@ const std::string errorDetails(int error_code); */ class DateFormatter { public: - DateFormatter(const std::string& format_string) : raw_format_string_(format_string) { + DateFormatter(const std::string& format_string, bool local_time = false) + : raw_format_string_(format_string), local_time_(local_time) { parse(format_string); } /** - * @return std::string representing the GMT/UTC time based on the input time. + * @return std::string representing the GMT/UTC time based on the input time, + * or local zone time when local_time_ is true. */ std::string fromTime(const SystemTime& time) const; @@ -104,6 +106,9 @@ class DateFormatter { // This is the format string as supplied in configuration, e.g. "foo %3f bar". const std::string raw_format_string_; + + // Whether use local time zone. + const bool local_time_; }; /** @@ -111,7 +116,7 @@ class DateFormatter { */ class AccessLogDateTimeFormatter { public: - static std::string fromTime(const SystemTime& time); + static std::string fromTime(const SystemTime& time, bool local_time = false); }; /** diff --git a/source/common/formatter/stream_info_formatter.cc b/source/common/formatter/stream_info_formatter.cc index 2c183986d136..3e40fa15b4c1 100644 --- a/source/common/formatter/stream_info_formatter.cc +++ b/source/common/formatter/stream_info_formatter.cc @@ -550,8 +550,10 @@ UpstreamPeerCertVEndFormatter::UpstreamPeerCertVEndFormatter(const std::string& : absl::optional(); })) {} -SystemTimeFormatter::SystemTimeFormatter(const std::string& format, TimeFieldExtractorPtr f) - : date_formatter_(format), time_field_extractor_(std::move(f)) { +SystemTimeFormatter::SystemTimeFormatter(const std::string& format, TimeFieldExtractorPtr f, + bool local_time) + : date_formatter_(format, local_time), time_field_extractor_(std::move(f)), + local_time_(local_time) { // Validate the input specifier here. The formatted string may be destined for a header, and // should not contain invalid characters {NUL, LR, CF}. if (std::regex_search(format, getSystemTimeFormatNewlinePattern())) { @@ -566,7 +568,7 @@ SystemTimeFormatter::format(const StreamInfo::StreamInfo& stream_info) const { return absl::nullopt; } if (date_formatter_.formatString().empty()) { - return AccessLogDateTimeFormatter::fromTime(time_field.value()); + return AccessLogDateTimeFormatter::fromTime(time_field.value(), local_time_); } return date_formatter_.fromTime(time_field.value()); } @@ -1658,6 +1660,17 @@ const StreamInfoFormatterProviderLookupTable& getKnownStreamInfoFormatterProvide return stream_info.startTime(); })); }}}, + {"START_TIME_LOCAL", + {CommandSyntaxChecker::PARAMS_OPTIONAL, + [](const std::string& format, absl::optional) { + return std::make_unique( + format, + std::make_unique( + [](const StreamInfo::StreamInfo& stream_info) -> absl::optional { + return stream_info.startTime(); + }), + true); + }}}, {"EMIT_TIME", {CommandSyntaxChecker::PARAMS_OPTIONAL, [](const std::string& format, absl::optional) { @@ -1668,6 +1681,17 @@ const StreamInfoFormatterProviderLookupTable& getKnownStreamInfoFormatterProvide return stream_info.timeSource().systemTime(); })); }}}, + {"EMIT_TIME_LOCAL", + {CommandSyntaxChecker::PARAMS_OPTIONAL, + [](const std::string& format, absl::optional) { + return std::make_unique( + format, + std::make_unique( + [](const StreamInfo::StreamInfo& stream_info) -> absl::optional { + return stream_info.timeSource().systemTime(); + }), + true); + }}}, {"DYNAMIC_METADATA", {CommandSyntaxChecker::PARAMS_REQUIRED, [](const std::string& format, absl::optional max_length) { diff --git a/source/common/formatter/stream_info_formatter.h b/source/common/formatter/stream_info_formatter.h index 89b37fe959ab..8de144eb0d4f 100644 --- a/source/common/formatter/stream_info_formatter.h +++ b/source/common/formatter/stream_info_formatter.h @@ -169,7 +169,7 @@ class SystemTimeFormatter : public StreamInfoFormatterProvider { std::function(const StreamInfo::StreamInfo& stream_info)>; using TimeFieldExtractorPtr = std::unique_ptr; - SystemTimeFormatter(const std::string& format, TimeFieldExtractorPtr f); + SystemTimeFormatter(const std::string& format, TimeFieldExtractorPtr f, bool local_time = false); // StreamInfoFormatterProvider absl::optional format(const StreamInfo::StreamInfo&) const override; @@ -178,6 +178,8 @@ class SystemTimeFormatter : public StreamInfoFormatterProvider { private: const Envoy::DateFormatter date_formatter_; const TimeFieldExtractorPtr time_field_extractor_; + // Whether use local time zone. + const bool local_time_; }; /** diff --git a/source/common/http/http2/metadata_encoder.h b/source/common/http/http2/metadata_encoder.h index 21bfa8f76a01..1dfb9499eb75 100644 --- a/source/common/http/http2/metadata_encoder.h +++ b/source/common/http/http2/metadata_encoder.h @@ -12,7 +12,7 @@ #include "source/common/common/logger.h" #include "quiche/http2/adapter/data_source.h" -#include "quiche/spdy/core/hpack/hpack_encoder.h" +#include "quiche/http2/hpack/hpack_encoder.h" namespace Envoy { namespace Http { diff --git a/source/common/http/http_server_properties_cache_impl.cc b/source/common/http/http_server_properties_cache_impl.cc index 06410d796e92..b694cc512f66 100644 --- a/source/common/http/http_server_properties_cache_impl.cc +++ b/source/common/http/http_server_properties_cache_impl.cc @@ -5,7 +5,7 @@ #include "source/common/common/logger.h" #include "source/common/http/http3_status_tracker_impl.h" -#include "quiche/spdy/core/spdy_alt_svc_wire_format.h" +#include "quiche/http2/core/spdy_alt_svc_wire_format.h" #include "re2/re2.h" namespace Envoy { diff --git a/source/common/http/http_server_properties_cache_manager_impl.cc b/source/common/http/http_server_properties_cache_manager_impl.cc index a640fcd3be6a..b9beaabcf024 100644 --- a/source/common/http/http_server_properties_cache_manager_impl.cc +++ b/source/common/http/http_server_properties_cache_manager_impl.cc @@ -13,11 +13,10 @@ namespace Envoy { namespace Http { -SINGLETON_MANAGER_REGISTRATION(alternate_protocols_cache_manager); - HttpServerPropertiesCacheManagerImpl::HttpServerPropertiesCacheManagerImpl( - AlternateProtocolsData& data, ThreadLocal::SlotAllocator& tls) - : data_(data), slot_(tls) { + Server::Configuration::ServerFactoryContext& context, + ProtobufMessage::ValidationVisitor& validation_visitor, ThreadLocal::SlotAllocator& tls) + : data_(context, validation_visitor), slot_(tls) { slot_.set([](Event::Dispatcher& /*dispatcher*/) { return std::make_shared(); }); } @@ -74,11 +73,5 @@ HttpServerPropertiesCacheSharedPtr HttpServerPropertiesCacheManagerImpl::getCach return new_cache; } -HttpServerPropertiesCacheManagerSharedPtr HttpServerPropertiesCacheManagerFactoryImpl::get() { - return singleton_manager_.getTyped( - SINGLETON_MANAGER_REGISTERED_NAME(alternate_protocols_cache_manager), - [this] { return std::make_shared(data_, tls_); }); -} - } // namespace Http } // namespace Envoy diff --git a/source/common/http/http_server_properties_cache_manager_impl.h b/source/common/http/http_server_properties_cache_manager_impl.h index 0cfe8411a9e5..8ab68df28e3f 100644 --- a/source/common/http/http_server_properties_cache_manager_impl.h +++ b/source/common/http/http_server_properties_cache_manager_impl.h @@ -27,7 +27,8 @@ struct AlternateProtocolsData { class HttpServerPropertiesCacheManagerImpl : public HttpServerPropertiesCacheManager, public Singleton::Instance { public: - HttpServerPropertiesCacheManagerImpl(AlternateProtocolsData& data, + HttpServerPropertiesCacheManagerImpl(Server::Configuration::ServerFactoryContext& context, + ProtobufMessage::ValidationVisitor& validation_visitor, ThreadLocal::SlotAllocator& tls); // HttpServerPropertiesCacheManager @@ -52,26 +53,11 @@ class HttpServerPropertiesCacheManagerImpl : public HttpServerPropertiesCacheMan absl::flat_hash_map caches_; }; - AlternateProtocolsData& data_; + AlternateProtocolsData data_; // Thread local state for the cache. ThreadLocal::TypedSlot slot_; }; -class HttpServerPropertiesCacheManagerFactoryImpl : public HttpServerPropertiesCacheManagerFactory { -public: - HttpServerPropertiesCacheManagerFactoryImpl(Singleton::Manager& singleton_manager, - ThreadLocal::SlotAllocator& tls, - AlternateProtocolsData data) - : singleton_manager_(singleton_manager), tls_(tls), data_(data) {} - - HttpServerPropertiesCacheManagerSharedPtr get() override; - -private: - Singleton::Manager& singleton_manager_; - ThreadLocal::SlotAllocator& tls_; - AlternateProtocolsData data_; -}; - } // namespace Http } // namespace Envoy diff --git a/source/common/quic/envoy_quic_client_stream.cc b/source/common/quic/envoy_quic_client_stream.cc index e81683eddced..3e329da66541 100644 --- a/source/common/quic/envoy_quic_client_stream.cc +++ b/source/common/quic/envoy_quic_client_stream.cc @@ -11,9 +11,9 @@ #include "source/common/quic/envoy_quic_utils.h" #include "source/common/runtime/runtime_features.h" +#include "quiche/common/http/http_header_block.h" #include "quiche/quic/core/http/quic_header_list.h" #include "quiche/quic/core/quic_session.h" -#include "quiche/spdy/core/http2_header_block.h" namespace Envoy { namespace Quic { @@ -55,7 +55,7 @@ Http::Status EnvoyQuicClientStream::encodeHeaders(const Http::RequestHeaderMap& local_end_stream_ = end_stream; SendBufferMonitor::ScopedWatermarkBufferUpdater updater(this, this); - spdy::Http2HeaderBlock spdy_headers; + quiche::HttpHeaderBlock spdy_headers; #ifndef ENVOY_ENABLE_UHV // Extended CONNECT to H/1 upgrade transformation has moved to UHV if (Runtime::runtimeFeatureEnabled("envoy.reloadable_features.use_http3_header_normalisation") && diff --git a/source/common/quic/envoy_quic_server_stream.cc b/source/common/quic/envoy_quic_server_stream.cc index de9cb028f09a..812025913c59 100644 --- a/source/common/quic/envoy_quic_server_stream.cc +++ b/source/common/quic/envoy_quic_server_stream.cc @@ -15,9 +15,9 @@ #include "source/common/quic/quic_stats_gatherer.h" #include "source/common/runtime/runtime_features.h" +#include "quiche/common/http/http_header_block.h" #include "quiche/quic/core/http/quic_header_list.h" #include "quiche/quic/core/quic_session.h" -#include "quiche/spdy/core/http2_header_block.h" #include "quiche_platform_impl/quiche_mem_slice_impl.h" namespace Envoy { diff --git a/source/common/quic/envoy_quic_utils.cc b/source/common/quic/envoy_quic_utils.cc index 137d5f2b80c2..3b01794a68e6 100644 --- a/source/common/quic/envoy_quic_utils.cc +++ b/source/common/quic/envoy_quic_utils.cc @@ -56,8 +56,8 @@ quic::QuicSocketAddress envoyIpAddressToQuicSocketAddress(const Network::Address return quic::QuicSocketAddress(ss); } -spdy::Http2HeaderBlock envoyHeadersToHttp2HeaderBlock(const Http::HeaderMap& headers) { - spdy::Http2HeaderBlock header_block; +quiche::HttpHeaderBlock envoyHeadersToHttp2HeaderBlock(const Http::HeaderMap& headers) { + quiche::HttpHeaderBlock header_block; headers.iterate([&header_block](const Http::HeaderEntry& header) -> Http::HeaderMap::Iterate { // The key-value pairs are copied. header_block.AppendValueOrAddHeader(header.key().getStringView(), diff --git a/source/common/quic/envoy_quic_utils.h b/source/common/quic/envoy_quic_utils.h index 4a4efa399bcc..d9a494e32aa4 100644 --- a/source/common/quic/envoy_quic_utils.h +++ b/source/common/quic/envoy_quic_utils.h @@ -12,13 +12,13 @@ #include "source/common/quic/quic_io_handle_wrapper.h" #include "openssl/ssl.h" +#include "quiche/common/http/http_header_block.h" #include "quiche/quic/core/http/quic_header_list.h" #include "quiche/quic/core/quic_config.h" #include "quiche/quic/core/quic_error_codes.h" #include "quiche/quic/core/quic_types.h" #include "quiche/quic/platform/api/quic_ip_address.h" #include "quiche/quic/platform/api/quic_socket_address.h" -#include "quiche/spdy/core/http2_header_block.h" namespace Envoy { namespace Quic { @@ -101,9 +101,10 @@ quicHeadersToEnvoyHeaders(const quic::QuicHeaderList& header_list, HeaderValidat template std::unique_ptr -http2HeaderBlockToEnvoyTrailers(const spdy::Http2HeaderBlock& header_block, uint32_t max_headers_kb, - uint32_t max_headers_allowed, HeaderValidator& validator, - absl::string_view& details, quic::QuicRstStreamErrorCode& rst) { +http2HeaderBlockToEnvoyTrailers(const quiche::HttpHeaderBlock& header_block, + uint32_t max_headers_kb, uint32_t max_headers_allowed, + HeaderValidator& validator, absl::string_view& details, + quic::QuicRstStreamErrorCode& rst) { auto headers = T::create(max_headers_kb, max_headers_allowed); if (header_block.size() > max_headers_allowed) { details = Http3ResponseCodeDetailValues::too_many_trailers; @@ -138,7 +139,7 @@ http2HeaderBlockToEnvoyTrailers(const spdy::Http2HeaderBlock& header_block, uint return headers; } -spdy::Http2HeaderBlock envoyHeadersToHttp2HeaderBlock(const Http::HeaderMap& headers); +quiche::HttpHeaderBlock envoyHeadersToHttp2HeaderBlock(const Http::HeaderMap& headers); // Called when Envoy wants to reset the underlying QUIC stream. quic::QuicRstStreamErrorCode envoyResetReasonToQuicRstError(Http::StreamResetReason reason); diff --git a/source/common/router/upstream_request.cc b/source/common/router/upstream_request.cc index 949340b233f8..18fb79901bec 100644 --- a/source/common/router/upstream_request.cc +++ b/source/common/router/upstream_request.cc @@ -94,8 +94,6 @@ UpstreamRequest::UpstreamRequest(RouterFilterInterface& parent, record_timeout_budget_(parent_.cluster()->timeoutBudgetStats().has_value()), cleaned_up_(false), had_upstream_(false), stream_options_({can_send_early_data, can_use_http3}), grpc_rq_success_deferred_(false), - upstream_wait_for_response_headers_before_disabling_read_(Runtime::runtimeFeatureEnabled( - "envoy.reloadable_features.upstream_wait_for_response_headers_before_disabling_read")), enable_half_close_(enable_half_close) { if (auto tracing_config = parent_.callbacks()->tracingConfig(); tracing_config.has_value()) { if (tracing_config->spawnUpstreamSpan() || parent_.config().start_child_span_) { @@ -717,17 +715,6 @@ void UpstreamRequest::clearRequestEncoder() { } void UpstreamRequest::readDisableOrDefer(bool disable) { - if (!upstream_wait_for_response_headers_before_disabling_read_) { - if (disable) { - parent_.cluster()->trafficStats()->upstream_flow_control_paused_reading_total_.inc(); - upstream_->readDisable(true); - } else { - parent_.cluster()->trafficStats()->upstream_flow_control_resumed_reading_total_.inc(); - upstream_->readDisable(false); - } - return; - } - if (disable) { // See comments on deferred_read_disabling_count_ for when we do and don't defer. if (parent_.downstreamResponseStarted()) { diff --git a/source/common/router/upstream_request.h b/source/common/router/upstream_request.h index eb32a8ec8431..c6af01ffe4f9 100644 --- a/source/common/router/upstream_request.h +++ b/source/common/router/upstream_request.h @@ -256,7 +256,6 @@ class UpstreamRequest : public Logger::Loggable, bool had_upstream_ : 1; Http::ConnectionPool::Instance::StreamOptions stream_options_; bool grpc_rq_success_deferred_ : 1; - bool upstream_wait_for_response_headers_before_disabling_read_ : 1; bool enable_half_close_ : 1; }; diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 9dba22ab7e4f..55cf4e0e57d7 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -92,7 +92,6 @@ RUNTIME_GUARD(envoy_reloadable_features_udp_socket_apply_aggregated_read_limit); RUNTIME_GUARD(envoy_reloadable_features_uhv_allow_malformed_url_encoding); RUNTIME_GUARD(envoy_reloadable_features_upstream_allow_connect_with_2xx); RUNTIME_GUARD(envoy_reloadable_features_upstream_remote_address_use_connection); -RUNTIME_GUARD(envoy_reloadable_features_upstream_wait_for_response_headers_before_disabling_read); RUNTIME_GUARD(envoy_reloadable_features_use_http3_header_normalisation); RUNTIME_GUARD(envoy_reloadable_features_use_typed_metadata_in_proxy_protocol_listener); RUNTIME_GUARD(envoy_reloadable_features_validate_connect); diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 8d2af96ed777..d62f54f9ec29 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -2215,8 +2215,8 @@ Http::ConnectionPool::InstancePtr ProdClusterManagerFactory::allocateConnPool( Http::HttpServerPropertiesCacheSharedPtr alternate_protocols_cache; if (alternate_protocol_options.has_value()) { // If there is configuration for an alternate protocols cache, always create one. - alternate_protocols_cache = alternate_protocols_cache_manager_->getCache( - alternate_protocol_options.value(), dispatcher); + alternate_protocols_cache = + alternate_protocols_cache_manager_.getCache(alternate_protocol_options.value(), dispatcher); } else if (!alternate_protocol_options.has_value() && (protocols.size() == 2 || (protocols.size() == 1 && protocols[0] == Http::Protocol::Http2))) { @@ -2227,7 +2227,7 @@ Http::ConnectionPool::InstancePtr ProdClusterManagerFactory::allocateConnPool( envoy::config::core::v3::AlternateProtocolsCacheOptions default_options; default_options.set_name(host->cluster().name()); alternate_protocols_cache = - alternate_protocols_cache_manager_->getCache(default_options, dispatcher); + alternate_protocols_cache_manager_.getCache(default_options, dispatcher); } absl::optional origin = @@ -2259,7 +2259,7 @@ Http::ConnectionPool::InstancePtr ProdClusterManagerFactory::allocateConnPool( envoy::config::core::v3::AlternateProtocolsCacheOptions default_options; default_options.set_name(host->cluster().name()); alternate_protocols_cache = - alternate_protocols_cache_manager_->getCache(default_options, dispatcher); + alternate_protocols_cache_manager_.getCache(default_options, dispatcher); } ASSERT(contains(protocols, {Http::Protocol::Http11, Http::Protocol::Http2})); diff --git a/source/common/upstream/cluster_manager_impl.h b/source/common/upstream/cluster_manager_impl.h index 1a78fcf72fed..62f2efd2c62a 100644 --- a/source/common/upstream/cluster_manager_impl.h +++ b/source/common/upstream/cluster_manager_impl.h @@ -61,9 +61,7 @@ class ProdClusterManagerFactory : public ClusterManagerFactory { : context_(context), stats_(stats), tls_(tls), http_context_(http_context), dns_resolver_fn_(dns_resolver_fn), ssl_context_manager_(ssl_context_manager), secret_manager_(secret_manager), quic_stat_names_(quic_stat_names), - alternate_protocols_cache_manager_factory_(context.singletonManager(), tls, - {context, context.messageValidationVisitor()}), - alternate_protocols_cache_manager_(alternate_protocols_cache_manager_factory_.get()), + alternate_protocols_cache_manager_(context.httpServerPropertiesCacheManager()), server_(server) {} // Upstream::ClusterManagerFactory @@ -104,8 +102,7 @@ class ProdClusterManagerFactory : public ClusterManagerFactory { Ssl::ContextManager& ssl_context_manager_; Secret::SecretManager& secret_manager_; Quic::QuicStatNames& quic_stat_names_; - Http::HttpServerPropertiesCacheManagerFactoryImpl alternate_protocols_cache_manager_factory_; - Http::HttpServerPropertiesCacheManagerSharedPtr alternate_protocols_cache_manager_; + Http::HttpServerPropertiesCacheManager& alternate_protocols_cache_manager_; Server::Instance& server_; }; diff --git a/source/extensions/filters/http/alternate_protocols_cache/config.cc b/source/extensions/filters/http/alternate_protocols_cache/config.cc index ae290b2e959b..30e69d24321a 100644 --- a/source/extensions/filters/http/alternate_protocols_cache/config.cc +++ b/source/extensions/filters/http/alternate_protocols_cache/config.cc @@ -18,12 +18,9 @@ Http::FilterFactoryCb AlternateProtocolsCacheFilterFactory::createFilterFactoryF auto& server_context = context.serverFactoryContext(); - Http::HttpServerPropertiesCacheManagerFactoryImpl alternate_protocol_cache_manager_factory( - server_context.singletonManager(), server_context.threadLocal(), - {context.serverFactoryContext(), context.messageValidationVisitor()}); - FilterConfigSharedPtr filter_config( - std::make_shared(proto_config, alternate_protocol_cache_manager_factory, - server_context.mainThreadDispatcher().timeSource())); + FilterConfigSharedPtr filter_config(std::make_shared( + proto_config, context.serverFactoryContext().httpServerPropertiesCacheManager(), + server_context.mainThreadDispatcher().timeSource())); return [filter_config](Http::FilterChainFactoryCallbacks& callbacks) -> void { callbacks.addStreamEncoderFilter( diff --git a/source/extensions/filters/http/alternate_protocols_cache/filter.cc b/source/extensions/filters/http/alternate_protocols_cache/filter.cc index 426aab2db922..d64d3cc0a103 100644 --- a/source/extensions/filters/http/alternate_protocols_cache/filter.cc +++ b/source/extensions/filters/http/alternate_protocols_cache/filter.cc @@ -19,10 +19,8 @@ using CustomClusterType = envoy::config::cluster::v3::Cluster::CustomClusterType FilterConfig::FilterConfig( const envoy::extensions::filters::http::alternate_protocols_cache::v3::FilterConfig& config, - Http::HttpServerPropertiesCacheManagerFactory& alternate_protocol_cache_manager_factory, - TimeSource& time_source) - : alternate_protocol_cache_manager_(alternate_protocol_cache_manager_factory.get()), - time_source_(time_source) { + Http::HttpServerPropertiesCacheManager& cache_manager, TimeSource& time_source) + : alternate_protocol_cache_manager_(cache_manager), time_source_(time_source) { if (config.has_alternate_protocols_cache_options()) { ENVOY_LOG_MISC(warn, "Using deprecated and ignored alternate_protocols_cache_options in " "alternate_protocols_cache config."); @@ -42,7 +40,7 @@ Http::FilterHeadersStatus Filter::encodeHeaders(Http::ResponseHeaderMap& headers Http::HttpServerPropertiesCacheSharedPtr cache; auto info = encoder_callbacks_->streamInfo().upstreamClusterInfo(); if (info && (*info)->alternateProtocolsCacheOptions()) { - cache = config_->alternateProtocolCacheManager()->getCache( + cache = config_->alternateProtocolCacheManager().getCache( *((*info)->alternateProtocolsCacheOptions()), dispatcher_); } if (!cache) { diff --git a/source/extensions/filters/http/alternate_protocols_cache/filter.h b/source/extensions/filters/http/alternate_protocols_cache/filter.h index 57cb67a09b2b..5932cf9d891f 100644 --- a/source/extensions/filters/http/alternate_protocols_cache/filter.h +++ b/source/extensions/filters/http/alternate_protocols_cache/filter.h @@ -16,19 +16,17 @@ namespace AlternateProtocolsCache { */ class FilterConfig { public: - FilterConfig( - const envoy::extensions::filters::http::alternate_protocols_cache::v3::FilterConfig& - proto_config, - Http::HttpServerPropertiesCacheManagerFactory& alternate_protocol_cache_manager_factory, - TimeSource& time_source); + FilterConfig(const envoy::extensions::filters::http::alternate_protocols_cache::v3::FilterConfig& + proto_config, + Http::HttpServerPropertiesCacheManager& cache_manager, TimeSource& time_source); TimeSource& timeSource() { return time_source_; } - const Http::HttpServerPropertiesCacheManagerSharedPtr alternateProtocolCacheManager() { + Http::HttpServerPropertiesCacheManager& alternateProtocolCacheManager() { return alternate_protocol_cache_manager_; } private: - const Http::HttpServerPropertiesCacheManagerSharedPtr alternate_protocol_cache_manager_; + Http::HttpServerPropertiesCacheManager& alternate_protocol_cache_manager_; TimeSource& time_source_; }; diff --git a/source/server/config_validation/server.cc b/source/server/config_validation/server.cc index d84ea0cb4fcb..b19238c8226b 100644 --- a/source/server/config_validation/server.cc +++ b/source/server/config_validation/server.cc @@ -139,6 +139,11 @@ void ValidationInstance::initialize(const Options& options, ssl_context_manager_ = std::make_unique(server_contexts_); + http_server_properties_cache_manager_ = + std::make_unique( + serverFactoryContext(), messageValidationContext().staticValidationVisitor(), + thread_local_); + cluster_manager_factory_ = std::make_unique( server_contexts_, stats(), threadLocal(), http_context_, [this]() -> Network::DnsResolverSharedPtr { return this->dnsResolver(); }, diff --git a/source/server/config_validation/server.h b/source/server/config_validation/server.h index ce01c3e62310..09949ab87c66 100644 --- a/source/server/config_validation/server.h +++ b/source/server/config_validation/server.h @@ -80,6 +80,9 @@ class ValidationInstance final : Logger::Loggable, const Upstream::ClusterManager& clusterManager() const override { return *config_.clusterManager(); } + Http::HttpServerPropertiesCacheManager& httpServerPropertiesCacheManager() override { + return *http_server_properties_cache_manager_; + } Ssl::ContextManager& sslContextManager() override { return *ssl_context_manager_; } Event::Dispatcher& dispatcher() override { return *dispatcher_; } Network::DnsResolverSharedPtr dnsResolver() override; @@ -179,6 +182,7 @@ class ValidationInstance final : Logger::Loggable, Configuration::MainImpl config_; LocalInfo::LocalInfoPtr local_info_; AccessLog::AccessLogManagerImpl access_log_manager_; + std::unique_ptr http_server_properties_cache_manager_; std::unique_ptr cluster_manager_factory_; std::unique_ptr listener_manager_; std::unique_ptr overload_manager_; diff --git a/source/server/server.cc b/source/server/server.cc index 4bef9c40d887..2b43921d973a 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -740,6 +740,11 @@ absl::Status InstanceBase::initializeOrThrow(Network::Address::InstanceConstShar ssl_context_manager_ = std::make_unique(server_contexts_); + http_server_properties_cache_manager_ = + std::make_unique( + serverFactoryContext(), messageValidationContext().staticValidationVisitor(), + thread_local_); + cluster_manager_factory_ = std::make_unique( serverFactoryContext(), stats_store_, thread_local_, http_context_, [this]() -> Network::DnsResolverSharedPtr { return this->getOrCreateDnsResolver(); }, diff --git a/source/server/server.h b/source/server/server.h index 7911eda0d67e..41dcd8363264 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -178,6 +178,9 @@ class ServerFactoryContextImpl : public Configuration::ServerFactoryContext, // Configuration::ServerFactoryContext Upstream::ClusterManager& clusterManager() override { return server_.clusterManager(); } + Http::HttpServerPropertiesCacheManager& httpServerPropertiesCacheManager() override { + return server_.httpServerPropertiesCacheManager(); + } Event::Dispatcher& mainThreadDispatcher() override { return server_.dispatcher(); } const Server::Options& options() override { return server_.options(); } const LocalInfo::LocalInfo& localInfo() const override { return server_.localInfo(); } @@ -264,6 +267,9 @@ class InstanceBase : Logger::Loggable, Api::Api& api() override { return *api_; } Upstream::ClusterManager& clusterManager() override; const Upstream::ClusterManager& clusterManager() const override; + Http::HttpServerPropertiesCacheManager& httpServerPropertiesCacheManager() override { + return *http_server_properties_cache_manager_; + } Ssl::ContextManager& sslContextManager() override { return *ssl_context_manager_; } Event::Dispatcher& dispatcher() override { return *dispatcher_; } Network::DnsResolverSharedPtr dnsResolver() override { return dns_resolver_; } @@ -408,6 +414,7 @@ class InstanceBase : Logger::Loggable, std::unique_ptr overload_manager_; std::unique_ptr null_overload_manager_; std::vector bootstrap_extensions_; + std::unique_ptr http_server_properties_cache_manager_; Envoy::MutexTracer* mutex_tracer_; Grpc::ContextImpl grpc_context_; Http::ContextImpl http_context_; diff --git a/test/common/common/utility_test.cc b/test/common/common/utility_test.cc index dad3a15a0ece..3e9550a546d6 100644 --- a/test/common/common/utility_test.cc +++ b/test/common/common/utility_test.cc @@ -603,6 +603,25 @@ TEST(AccessLogDateTimeFormatter, fromTime) { EXPECT_EQ("2018-04-03T23:06:08.999Z", AccessLogDateTimeFormatter::fromTime(time4)); } +TEST(AccessLogDateTimeFormatter, fromTimeLocalTimeZone) { + SystemTime time1(std::chrono::seconds(1522796769)); + EXPECT_EQ( + absl::FormatTime("%Y-%m-%dT%H:%M:%E3S%z", absl::FromChrono(time1), absl::LocalTimeZone()), + AccessLogDateTimeFormatter::fromTime(time1, true)); + SystemTime time2(std::chrono::milliseconds(1522796769123)); + EXPECT_EQ( + absl::FormatTime("%Y-%m-%dT%H:%M:%E3S%z", absl::FromChrono(time2), absl::LocalTimeZone()), + AccessLogDateTimeFormatter::fromTime(time2, true)); + SystemTime time3(std::chrono::milliseconds(1522796769999)); + EXPECT_EQ( + absl::FormatTime("%Y-%m-%dT%H:%M:%E3S%z", absl::FromChrono(time3), absl::LocalTimeZone()), + AccessLogDateTimeFormatter::fromTime(time3, true)); + SystemTime time4(std::chrono::milliseconds(1522796768999)); + EXPECT_EQ( + absl::FormatTime("%Y-%m-%dT%H:%M:%E3S%z", absl::FromChrono(time4), absl::LocalTimeZone()), + AccessLogDateTimeFormatter::fromTime(time4, true)); +} + TEST(Primes, isPrime) { EXPECT_FALSE(Primes::isPrime(0)); EXPECT_TRUE(Primes::isPrime(67)); @@ -1026,6 +1045,25 @@ TEST(DateFormatter, FromTime) { EXPECT_EQ("aaa00", DateFormatter(std::string(3, 'a') + "%H").fromTime(time2)); } +TEST(DateFormatter, FromTimeLocalTimeZone) { + const SystemTime time1(std::chrono::seconds(1522796769)); + EXPECT_EQ( + absl::FormatTime("%Y-%m-%dT%H:%M:%S.000Z", absl::FromChrono(time1), absl::LocalTimeZone()), + DateFormatter("%Y-%m-%dT%H:%M:%S.000Z", true).fromTime(time1)); + EXPECT_EQ(absl::FormatTime("aaa%H", absl::FromChrono(time1), absl::LocalTimeZone()), + DateFormatter(std::string(3, 'a') + "%H", true).fromTime(time1)); + const SystemTime time2(std::chrono::seconds(0)); + EXPECT_EQ( + absl::FormatTime("%Y-%m-%dT%H:%M:%S.000Z", absl::FromChrono(time2), absl::LocalTimeZone()), + DateFormatter("%Y-%m-%dT%H:%M:%S.000Z", true).fromTime(time2)); + EXPECT_EQ(absl::FormatTime("aaa%H", absl::FromChrono(time2), absl::LocalTimeZone()), + DateFormatter(std::string(3, 'a') + "%H", true).fromTime(time2)); + SystemTime time3(std::chrono::milliseconds(1522796769999)); + EXPECT_EQ( + absl::FormatTime("%Y-%m-%dT%H:%M:%E3S%z", absl::FromChrono(time3), absl::LocalTimeZone()), + DateFormatter("%Y-%m-%dT%H:%M:%S.%3f%z", true).fromTime(time3)); +} + // Check the time complexity. Make sure DateFormatter can finish parsing long messy string without // crashing/freezing. This should pass in 0-2 seconds if O(n). Finish in 30-120 seconds if O(n^2) TEST(DateFormatter, ParseLongString) { diff --git a/test/common/formatter/substitution_formatter_fuzz_test.dict b/test/common/formatter/substitution_formatter_fuzz_test.dict index 637faeacff65..d5a69c589b95 100644 --- a/test/common/formatter/substitution_formatter_fuzz_test.dict +++ b/test/common/formatter/substitution_formatter_fuzz_test.dict @@ -1,6 +1,7 @@ # format strings "[%START_TIME%] \"%REQ(:METHOD)% %REQ(X-ENVOY-ORIGINAL-PATH?:PATH)% %PROTOCOL%\" %RESPONSE_CODE% %RESPONSE_FLAGS% %BYTES_RECEIVED% %BYTES_SENT% %DURATION% %RESP(X-ENVOY-UPSTREAM-SERVICE-TIME)% \"%REQ(X-FORWARDED-FOR)%\" \"%REQ(USER-AGENT)%\" \"%REQ(X-REQUEST-ID)%\" \"%REQ(:AUTHORITY)%\" \"%UPSTREAM_HOST%\"\x0A" "%START_TIME%" +"%START_TIME_LOCAL%" "%BYTES_RECEIVED%" "%PROTOCOL%" "%RESPONSE_CODE%" diff --git a/test/common/formatter/substitution_formatter_test.cc b/test/common/formatter/substitution_formatter_test.cc index 7fd8e2c491b0..a9613ce63bf9 100644 --- a/test/common/formatter/substitution_formatter_test.cc +++ b/test/common/formatter/substitution_formatter_test.cc @@ -4185,16 +4185,21 @@ TEST(SubstitutionFormatterTest, StructFormatterStartTimeTest) { absl::node_hash_map expected_json_map = { {"simple_date", "2018/03/28"}, {"test_time", fmt::format("{}", expected_time_in_epoch)}, + {"test_time_local", fmt::format("{}", expected_time_in_epoch)}, {"bad_format", "bad_format"}, {"default", "2018-03-28T23:35:58.000Z"}, + {"default_local", + absl::FormatTime("%Y-%m-%dT%H:%M:%E3S%z", absl::FromChrono(time), absl::LocalTimeZone())}, {"all_zeroes", "000000000.0.00.000"}}; ProtobufWkt::Struct key_mapping; TestUtility::loadFromYaml(R"EOF( simple_date: '%START_TIME(%Y/%m/%d)%' test_time: '%START_TIME(%s)%' + test_time_local: '%START_TIME_LOCAL(%s)%' bad_format: '%START_TIME(bad_format)%' default: '%START_TIME%' + default_local: '%START_TIME_LOCAL%' all_zeroes: '%START_TIME(%f.%1f.%2f.%3f)%' )EOF", key_mapping); diff --git a/test/common/http/http2/http2_frame.cc b/test/common/http/http2/http2_frame.cc index fe4229fd3aa5..349cd35ade2a 100644 --- a/test/common/http/http2/http2_frame.cc +++ b/test/common/http/http2/http2_frame.cc @@ -6,7 +6,7 @@ #include "source/common/common/hex.h" -#include "quiche/spdy/core/hpack/hpack_encoder.h" +#include "quiche/http2/hpack/hpack_encoder.h" namespace { diff --git a/test/common/http/http_server_properties_cache_manager_test.cc b/test/common/http/http_server_properties_cache_manager_test.cc index 0d2fe3d44fb9..e29d8ac47349 100644 --- a/test/common/http/http_server_properties_cache_manager_test.cc +++ b/test/common/http/http_server_properties_cache_manager_test.cc @@ -23,17 +23,13 @@ class HttpServerPropertiesCacheManagerTest : public testing::Test, options2_.mutable_max_entries()->set_value(max_entries2_); } void initialize() { - AlternateProtocolsData data(context_.server_factory_context_, - context_.messageValidationVisitor()); - factory_ = std::make_unique( - singleton_manager_, tls_, data); - manager_ = factory_->get(); + manager_ = std::make_unique( + context_.server_factory_context_, context_.messageValidationVisitor(), tls_); } Singleton::ManagerImpl singleton_manager_; NiceMock context_; testing::NiceMock tls_; - std::unique_ptr factory_; HttpServerPropertiesCacheManagerSharedPtr manager_; const std::string name1_ = "name1"; const std::string name2_ = "name2"; @@ -45,13 +41,6 @@ class HttpServerPropertiesCacheManagerTest : public testing::Test, envoy::config::core::v3::AlternateProtocolsCacheOptions options2_; }; -TEST_F(HttpServerPropertiesCacheManagerTest, FactoryGet) { - initialize(); - - EXPECT_NE(nullptr, manager_); - EXPECT_EQ(manager_, factory_->get()); -} - TEST_F(HttpServerPropertiesCacheManagerTest, GetCache) { initialize(); HttpServerPropertiesCacheSharedPtr cache = manager_->getCache(options1_, dispatcher_); diff --git a/test/extensions/filters/http/alternate_protocols_cache/filter_test.cc b/test/extensions/filters/http/alternate_protocols_cache/filter_test.cc index dd778c414035..f1dd2c03835d 100644 --- a/test/extensions/filters/http/alternate_protocols_cache/filter_test.cc +++ b/test/extensions/filters/http/alternate_protocols_cache/filter_test.cc @@ -27,23 +27,19 @@ class FilterTest : public testing::Test, public Event::TestUsingSimulatedTime { } void initialize(bool populate_config) { - EXPECT_CALL(alternate_protocols_cache_manager_factory_, get()) - .WillOnce(Return(alternate_protocols_cache_manager_)); - envoy::extensions::filters::http::alternate_protocols_cache::v3::FilterConfig proto_config; if (populate_config) { EXPECT_CALL(*alternate_protocols_cache_manager_, getCache(_, _)) .Times(testing::AnyNumber()) .WillOnce(Return(alternate_protocols_cache_)); } - filter_config_ = std::make_shared( - proto_config, alternate_protocols_cache_manager_factory_, simTime()); + filter_config_ = std::make_shared(proto_config, + *alternate_protocols_cache_manager_, simTime()); filter_ = std::make_unique(filter_config_, dispatcher_); filter_->setEncoderFilterCallbacks(callbacks_); } Event::MockDispatcher dispatcher_; - Http::MockHttpServerPropertiesCacheManagerFactory alternate_protocols_cache_manager_factory_; std::shared_ptr alternate_protocols_cache_manager_; std::shared_ptr alternate_protocols_cache_; FilterConfigSharedPtr filter_config_; diff --git a/test/extensions/key_value/file_based/alternate_protocols_cache_impl_test.cc b/test/extensions/key_value/file_based/alternate_protocols_cache_impl_test.cc index 0787fe7c4ea6..a803c9f4e72e 100644 --- a/test/extensions/key_value/file_based/alternate_protocols_cache_impl_test.cc +++ b/test/extensions/key_value/file_based/alternate_protocols_cache_impl_test.cc @@ -20,16 +20,12 @@ class HttpServerPropertiesCacheManagerTest : public testing::Test, options_.mutable_max_entries()->set_value(10); } void initialize() { - Http::AlternateProtocolsData data{context_.server_factory_context_, - context_.messageValidationVisitor()}; - factory_ = std::make_unique( - singleton_manager_, tls_, data); - manager_ = factory_->get(); + manager_ = std::make_unique( + context_.server_factory_context_, context_.messageValidationVisitor(), tls_); } Singleton::ManagerImpl singleton_manager_; NiceMock context_; testing::NiceMock tls_; - std::unique_ptr factory_; Http::HttpServerPropertiesCacheManagerSharedPtr manager_; envoy::config::core::v3::AlternateProtocolsCacheOptions options_; testing::NiceMock dispatcher_; diff --git a/test/extensions/tracers/common/ot/opentracing_driver_impl_test.cc b/test/extensions/tracers/common/ot/opentracing_driver_impl_test.cc deleted file mode 100644 index 352dfd8f2c14..000000000000 --- a/test/extensions/tracers/common/ot/opentracing_driver_impl_test.cc +++ /dev/null @@ -1,379 +0,0 @@ -#include -#include -#include - -#include "source/extensions/tracers/common/ot/opentracing_driver_impl.h" - -#include "test/mocks/http/mocks.h" -#include "test/mocks/stats/mocks.h" -#include "test/mocks/stream_info/mocks.h" -#include "test/mocks/tracing/mocks.h" - -#include "gmock/gmock.h" -#include "gtest/gtest.h" -#include "opentracing/mocktracer/in_memory_recorder.h" -#include "opentracing/mocktracer/tracer.h" - -namespace Envoy { -namespace Extensions { -namespace Tracers { -namespace Common { -namespace Ot { -namespace { - -class NullSpanContext : public opentracing::SpanContext { -public: - void ForeachBaggageItem( - std::function) const override { - // not implemented - } - - virtual std::unique_ptr clone() const noexcept { - return std::make_unique(*this); - } -}; - -class NullSpan : public opentracing::Span { -public: - explicit NullSpan(const opentracing::Tracer& tracer) : tracer_(tracer) {} - - void FinishWithOptions(const opentracing::FinishSpanOptions&) noexcept override { - // not implemented - } - - void SetOperationName(opentracing::string_view /*name*/) noexcept override { - // not implemented - } - - void SetTag(opentracing::string_view /*key*/, - const opentracing::Value& /*value*/) noexcept override { - // not implemented - } - - void SetBaggageItem(opentracing::string_view /*key*/, - opentracing::string_view /*value*/) noexcept override { - // not implemented - } - - std::string BaggageItem(opentracing::string_view /*key*/) const noexcept override { - return ""; // not implemented - } - - void Log(std::initializer_list< - std::pair> /*fields*/) noexcept override { - // not implemented - } - - const opentracing::SpanContext& context() const noexcept override { - static NullSpanContext context; - return context; - } - - const opentracing::Tracer& tracer() const noexcept override { return tracer_; } - - const opentracing::Tracer& tracer_; -}; - -class ContextIteratingTracer : public opentracing::Tracer { -public: - explicit ContextIteratingTracer( - std::vector>& extracted_headers_destination) - : extracted_headers_(&extracted_headers_destination) {} - - std::unique_ptr - StartSpanWithOptions(opentracing::string_view /*operation_name*/, - const opentracing::StartSpanOptions&) const noexcept override { - return std::make_unique(*this); - } - - opentracing::expected Inject(const opentracing::SpanContext&, - std::ostream& /*writer*/) const override { - return {}; // not implemented - } - - opentracing::expected Inject(const opentracing::SpanContext&, - const opentracing::TextMapWriter&) const override { - return {}; // not implemented - } - - opentracing::expected Inject(const opentracing::SpanContext&, - const opentracing::HTTPHeadersWriter&) const override { - return {}; // not implemented - } - - opentracing::expected - Inject(const opentracing::SpanContext& sc, - const opentracing::CustomCarrierWriter& writer) const override { - return opentracing::Tracer::Inject(sc, writer); - } - - opentracing::expected> - Extract(std::istream& /*reader*/) const override { - return std::unique_ptr(); // not implemented - } - - opentracing::expected> - Extract(const opentracing::TextMapReader&) const override { - return std::unique_ptr(); // not implemented - } - - opentracing::expected> - Extract(const opentracing::HTTPHeadersReader& reader) const override { - reader.ForeachKey([this](opentracing::string_view key, - opentracing::string_view value) -> opentracing::expected { - extracted_headers_->emplace_back(key, value); - return {}; - }); - - return std::unique_ptr(); - } - - opentracing::expected> - Extract(const opentracing::CustomCarrierReader& reader) const override { - return opentracing::Tracer::Extract(reader); - } - - std::vector>* extracted_headers_; -}; - -class TestDriver : public OpenTracingDriver { -public: - TestDriver(OpenTracingDriver::PropagationMode propagation_mode, - const opentracing::mocktracer::PropagationOptions& propagation_options, - Stats::Scope& scope) - : OpenTracingDriver{scope}, propagation_mode_{propagation_mode} { - opentracing::mocktracer::MockTracerOptions options; - auto recorder = new opentracing::mocktracer::InMemoryRecorder{}; - recorder_ = recorder; - options.recorder.reset(recorder); - options.propagation_options = propagation_options; - tracer_ = std::make_shared(std::move(options)); - } - - TestDriver(const std::shared_ptr& tracer, Stats::Scope& scope) - : OpenTracingDriver{scope}, - propagation_mode_{PropagationMode::TracerNative}, recorder_{nullptr}, tracer_{tracer} {} - - const opentracing::mocktracer::InMemoryRecorder& recorder() const { return *recorder_; } - - // OpenTracingDriver - opentracing::Tracer& tracer() override { return *tracer_; } - - PropagationMode propagationMode() const override { return propagation_mode_; } - -private: - const OpenTracingDriver::PropagationMode propagation_mode_; - const opentracing::mocktracer::InMemoryRecorder* recorder_; - std::shared_ptr tracer_; -}; - -class OpenTracingDriverTest : public testing::Test { -public: - void - setupValidDriver(OpenTracingDriver::PropagationMode propagation_mode = - OpenTracingDriver::PropagationMode::SingleHeader, - const opentracing::mocktracer::PropagationOptions& propagation_options = {}) { - driver_ = - std::make_unique(propagation_mode, propagation_options, *stats_.rootScope()); - } - - void setupValidDriver(std::vector>& headers_destination) { - auto tracer = std::make_shared(headers_destination); - driver_ = std::make_unique(tracer, *stats_.rootScope()); - } - - const std::string operation_name_{"test"}; - Tracing::TestTraceContextImpl request_headers_{ - {":path", "/"}, {":method", "GET"}, {"x-request-id", "foo"}}; - NiceMock stream_info_; - - std::unique_ptr driver_; - Stats::TestUtil::TestStore stats_; - - NiceMock config_; -}; - -TEST_F(OpenTracingDriverTest, FlushSpanWithTag) { - setupValidDriver(); - - Tracing::SpanPtr first_span = driver_->startSpan( - config_, request_headers_, stream_info_, operation_name_, {Tracing::Reason::Sampling, true}); - first_span->setTag("abc", "123"); - first_span->finishSpan(); - - const std::map expected_tags = { - {"abc", std::string{"123"}}, - {opentracing::ext::span_kind, std::string{opentracing::ext::span_kind_rpc_server}}}; - - EXPECT_EQ(1, driver_->recorder().spans().size()); - EXPECT_EQ(expected_tags, driver_->recorder().top().tags); -} - -TEST_F(OpenTracingDriverTest, FlushSpanWithLog) { - setupValidDriver(); - - Tracing::SpanPtr first_span = driver_->startSpan( - config_, request_headers_, stream_info_, operation_name_, {Tracing::Reason::Sampling, true}); - const auto timestamp = - SystemTime{std::chrono::duration_cast(std::chrono::hours{123})}; - first_span->log(timestamp, "abc"); - first_span->finishSpan(); - - const std::vector expected_logs = { - {timestamp, {{"event", std::string{"abc"}}}}}; - - EXPECT_EQ(1, driver_->recorder().spans().size()); - EXPECT_EQ(expected_logs, driver_->recorder().top().logs); -} - -TEST_F(OpenTracingDriverTest, FlushSpanWithBaggage) { - setupValidDriver(); - - Tracing::SpanPtr first_span = driver_->startSpan( - config_, request_headers_, stream_info_, operation_name_, {Tracing::Reason::Sampling, true}); - first_span->setBaggage("abc", "123"); - first_span->finishSpan(); - - const std::map expected_baggage = {{"abc", "123"}}; - - EXPECT_EQ(1, driver_->recorder().spans().size()); - EXPECT_EQ(expected_baggage, driver_->recorder().top().span_context.baggage); -} - -TEST_F(OpenTracingDriverTest, TagSamplingFalseByDecision) { - setupValidDriver(OpenTracingDriver::PropagationMode::TracerNative, {}); - - Tracing::SpanPtr first_span = driver_->startSpan( - config_, request_headers_, stream_info_, operation_name_, {Tracing::Reason::Sampling, false}); - first_span->finishSpan(); - - const std::map expected_tags = { - {opentracing::ext::sampling_priority, 0}, - {opentracing::ext::span_kind, std::string{opentracing::ext::span_kind_rpc_server}}}; - - EXPECT_EQ(1, driver_->recorder().spans().size()); - EXPECT_EQ(expected_tags, driver_->recorder().top().tags); -} - -TEST_F(OpenTracingDriverTest, TagSamplingFalseByFlag) { - setupValidDriver(OpenTracingDriver::PropagationMode::TracerNative, {}); - - Tracing::SpanPtr first_span = driver_->startSpan( - config_, request_headers_, stream_info_, operation_name_, {Tracing::Reason::Sampling, true}); - first_span->setSampled(false); - first_span->finishSpan(); - - const std::map expected_tags = { - {opentracing::ext::sampling_priority, 0}, - {opentracing::ext::span_kind, std::string{opentracing::ext::span_kind_rpc_server}}}; - - EXPECT_EQ(1, driver_->recorder().spans().size()); - EXPECT_EQ(expected_tags, driver_->recorder().top().tags); -} - -TEST_F(OpenTracingDriverTest, TagSpanKindClient) { - setupValidDriver(OpenTracingDriver::PropagationMode::TracerNative, {}); - - ON_CALL(config_, operationName()).WillByDefault(testing::Return(Tracing::OperationName::Egress)); - - Tracing::SpanPtr first_span = driver_->startSpan( - config_, request_headers_, stream_info_, operation_name_, {Tracing::Reason::Sampling, true}); - first_span->finishSpan(); - - const std::map expected_tags = { - {opentracing::ext::span_kind, std::string{opentracing::ext::span_kind_rpc_client}}}; - - EXPECT_EQ(1, driver_->recorder().spans().size()); - EXPECT_EQ(expected_tags, driver_->recorder().top().tags); -} - -TEST_F(OpenTracingDriverTest, TagSpanKindServer) { - setupValidDriver(OpenTracingDriver::PropagationMode::TracerNative, {}); - - ON_CALL(config_, operationName()).WillByDefault(testing::Return(Tracing::OperationName::Ingress)); - - Tracing::SpanPtr first_span = driver_->startSpan( - config_, request_headers_, stream_info_, operation_name_, {Tracing::Reason::Sampling, true}); - first_span->finishSpan(); - - const std::map expected_tags = { - {opentracing::ext::span_kind, std::string{opentracing::ext::span_kind_rpc_server}}}; - - EXPECT_EQ(1, driver_->recorder().spans().size()); - EXPECT_EQ(expected_tags, driver_->recorder().top().tags); -} - -TEST_F(OpenTracingDriverTest, InjectFailure) { - for (OpenTracingDriver::PropagationMode propagation_mode : - {OpenTracingDriver::PropagationMode::SingleHeader, - OpenTracingDriver::PropagationMode::TracerNative}) { - opentracing::mocktracer::PropagationOptions propagation_options; - propagation_options.inject_error_code = std::make_error_code(std::errc::bad_message); - setupValidDriver(propagation_mode, propagation_options); - - Tracing::SpanPtr span = driver_->startSpan(config_, request_headers_, stream_info_, - operation_name_, {Tracing::Reason::Sampling, true}); - - const auto span_context_injection_error_count = - stats_.counter("tracing.opentracing.span_context_injection_error").value(); - EXPECT_FALSE( - request_headers_.context_map_.contains(Http::CustomHeaders::get().OtSpanContext.get())); - span->injectContext(request_headers_, Tracing::UpstreamContext()); - - EXPECT_EQ(span_context_injection_error_count + 1, - stats_.counter("tracing.opentracing.span_context_injection_error").value()); - } -} - -TEST_F(OpenTracingDriverTest, ExtractWithUnindexedHeader) { - opentracing::mocktracer::PropagationOptions propagation_options; - propagation_options.propagation_key = "unindexed-header"; - setupValidDriver(OpenTracingDriver::PropagationMode::TracerNative, propagation_options); - - Tracing::SpanPtr first_span = driver_->startSpan( - config_, request_headers_, stream_info_, operation_name_, {Tracing::Reason::Sampling, true}); - first_span->injectContext(request_headers_, Tracing::UpstreamContext()); - - Tracing::SpanPtr second_span = driver_->startSpan( - config_, request_headers_, stream_info_, operation_name_, {Tracing::Reason::Sampling, true}); - second_span->finishSpan(); - first_span->finishSpan(); - - auto spans = driver_->recorder().spans(); - EXPECT_EQ(spans.at(1).span_context.span_id, spans.at(0).references.at(0).span_id); -} - -TEST_F(OpenTracingDriverTest, GetTraceId) { - setupValidDriver(); - - Tracing::SpanPtr first_span = driver_->startSpan( - config_, request_headers_, stream_info_, operation_name_, {Tracing::Reason::Sampling, true}); - first_span->setTag("abc", "123"); - first_span->finishSpan(); - - // This method is unimplemented and a noop. - ASSERT_EQ(first_span->getTraceId(), ""); - // This method is unimplemented and a noop. - ASSERT_EQ(first_span->getSpanId(), ""); -} - -TEST_F(OpenTracingDriverTest, ExtractUsingForeach) { - std::vector> extracted_headers; - setupValidDriver(extracted_headers); - - // Starting a new span, given the `request_headers_`, will visit the headers - // using "for each." We can immediately discard the span. - driver_->startSpan(config_, request_headers_, stream_info_, operation_name_, - {Tracing::Reason::Sampling, true}); - - for (const auto& [key, value] : extracted_headers) { - EXPECT_EQ(value, request_headers_.get(key)); - } -} - -} // namespace -} // namespace Ot -} // namespace Common -} // namespace Tracers -} // namespace Extensions -} // namespace Envoy diff --git a/test/integration/multiplexed_integration_test.cc b/test/integration/multiplexed_integration_test.cc index 5fce3c17d577..8ba0d7738750 100644 --- a/test/integration/multiplexed_integration_test.cc +++ b/test/integration/multiplexed_integration_test.cc @@ -2927,8 +2927,6 @@ TEST_P(MultiplexedIntegrationTest, PerTryTimeoutWhileDownstreamStopsReading) { if (downstreamProtocol() != Http::CodecType::HTTP2) { return; } - config_helper_.addRuntimeOverride( - "envoy.reloadable_features.upstream_wait_for_response_headers_before_disabling_read", "true"); config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void { RELEASE_ASSERT(bootstrap.mutable_static_resources()->listeners_size() >= 1, ""); diff --git a/test/mocks/http/http_server_properties_cache.cc b/test/mocks/http/http_server_properties_cache.cc index 11140e638a90..b178d39d0fb1 100644 --- a/test/mocks/http/http_server_properties_cache.cc +++ b/test/mocks/http/http_server_properties_cache.cc @@ -7,8 +7,5 @@ MockHttpServerPropertiesCache::~MockHttpServerPropertiesCache() = default; MockHttpServerPropertiesCacheManager::~MockHttpServerPropertiesCacheManager() = default; -MockHttpServerPropertiesCacheManagerFactory::~MockHttpServerPropertiesCacheManagerFactory() = - default; - } // namespace Http } // namespace Envoy diff --git a/test/mocks/http/http_server_properties_cache.h b/test/mocks/http/http_server_properties_cache.h index ba1515e69b0e..2eced57f9f87 100644 --- a/test/mocks/http/http_server_properties_cache.h +++ b/test/mocks/http/http_server_properties_cache.h @@ -32,12 +32,5 @@ class MockHttpServerPropertiesCacheManager : public HttpServerPropertiesCacheMan Event::Dispatcher& dispatcher)); }; -class MockHttpServerPropertiesCacheManagerFactory : public HttpServerPropertiesCacheManagerFactory { -public: - ~MockHttpServerPropertiesCacheManagerFactory() override; - - MOCK_METHOD(HttpServerPropertiesCacheManagerSharedPtr, get, ()); -}; - } // namespace Http } // namespace Envoy diff --git a/test/mocks/server/BUILD b/test/mocks/server/BUILD index 7fe3818ab7fa..1c88e71a5e1a 100644 --- a/test/mocks/server/BUILD +++ b/test/mocks/server/BUILD @@ -220,6 +220,7 @@ envoy_cc_mock( "//test/mocks/access_log:access_log_mocks", "//test/mocks/api:api_mocks", "//test/mocks/http:http_mocks", + "//test/mocks/http:http_server_properties_cache_mocks", "//test/mocks/init:init_mocks", "//test/mocks/local_info:local_info_mocks", "//test/mocks/network:network_mocks", diff --git a/test/mocks/server/instance.cc b/test/mocks/server/instance.cc index 545f5b0b89f2..8d3b8e3afac9 100644 --- a/test/mocks/server/instance.cc +++ b/test/mocks/server/instance.cc @@ -31,6 +31,8 @@ MockInstance::MockInstance() ON_CALL(*this, api()).WillByDefault(ReturnRef(api_)); ON_CALL(*this, admin()).WillByDefault(Return(OptRef{admin_})); ON_CALL(*this, clusterManager()).WillByDefault(ReturnRef(cluster_manager_)); + ON_CALL(*this, httpServerPropertiesCacheManager()) + .WillByDefault(ReturnRef(http_server_properties_cache_manager_)); ON_CALL(*this, sslContextManager()).WillByDefault(ReturnRef(ssl_context_manager_)); ON_CALL(*this, accessLogManager()).WillByDefault(ReturnRef(access_log_manager_)); ON_CALL(*this, runtime()).WillByDefault(ReturnRef(runtime_loader_)); diff --git a/test/mocks/server/instance.h b/test/mocks/server/instance.h index 801ce70dc88f..6d52ec82a3ab 100644 --- a/test/mocks/server/instance.h +++ b/test/mocks/server/instance.h @@ -2,6 +2,7 @@ #include "envoy/server/instance.h" +#include "test/mocks/http/http_server_properties_cache.h" #include "test/mocks/server/server_factory_context.h" #include "test/mocks/server/transport_socket_factory_context.h" @@ -22,6 +23,7 @@ class MockInstance : public Instance { MOCK_METHOD(Api::Api&, api, ()); MOCK_METHOD(Upstream::ClusterManager&, clusterManager, ()); MOCK_METHOD(const Upstream::ClusterManager&, clusterManager, (), (const)); + MOCK_METHOD(Http::HttpServerPropertiesCacheManager&, httpServerPropertiesCacheManager, ()); MOCK_METHOD(Ssl::ContextManager&, sslContextManager, ()); MOCK_METHOD(Event::Dispatcher&, dispatcher, ()); MOCK_METHOD(Network::DnsResolverSharedPtr, dnsResolver, ()); @@ -79,6 +81,8 @@ class MockInstance : public Instance { Event::GlobalTimeSystem time_system_; std::unique_ptr secret_manager_; testing::NiceMock cluster_manager_; + testing::NiceMock + http_server_properties_cache_manager_; Thread::MutexBasicLockable access_log_lock_; testing::NiceMock runtime_loader_; testing::NiceMock dispatcher_; diff --git a/test/mocks/server/server_factory_context.cc b/test/mocks/server/server_factory_context.cc index 34b58962c158..80fe4b390425 100644 --- a/test/mocks/server/server_factory_context.cc +++ b/test/mocks/server/server_factory_context.cc @@ -11,6 +11,8 @@ MockServerFactoryContext::MockServerFactoryContext() : singleton_manager_(new Singleton::ManagerImpl()), http_context_(store_.symbolTable()), grpc_context_(store_.symbolTable()), router_context_(store_.symbolTable()) { ON_CALL(*this, clusterManager()).WillByDefault(ReturnRef(cluster_manager_)); + ON_CALL(*this, httpServerPropertiesCacheManager()) + .WillByDefault(ReturnRef(http_server_properties_cache_manager_)); ON_CALL(*this, mainThreadDispatcher()).WillByDefault(ReturnRef(dispatcher_)); ON_CALL(*this, drainDecision()).WillByDefault(ReturnRef(drain_manager_)); ON_CALL(*this, localInfo()).WillByDefault(ReturnRef(local_info_)); diff --git a/test/mocks/server/server_factory_context.h b/test/mocks/server/server_factory_context.h index 61d48d940c07..fc4c69deed00 100644 --- a/test/mocks/server/server_factory_context.h +++ b/test/mocks/server/server_factory_context.h @@ -12,6 +12,7 @@ #include "test/mocks/access_log/mocks.h" #include "test/mocks/api/mocks.h" #include "test/mocks/event/mocks.h" +#include "test/mocks/http/http_server_properties_cache.h" #include "test/mocks/http/mocks.h" #include "test/mocks/init/mocks.h" #include "test/mocks/local_info/mocks.h" @@ -56,6 +57,7 @@ class MockServerFactoryContext : public virtual ServerFactoryContext { ~MockServerFactoryContext() override; MOCK_METHOD(Upstream::ClusterManager&, clusterManager, ()); + MOCK_METHOD(Http::HttpServerPropertiesCacheManager&, httpServerPropertiesCacheManager, ()); MOCK_METHOD(Event::Dispatcher&, mainThreadDispatcher, ()); MOCK_METHOD(const Server::Options&, options, ()); MOCK_METHOD(const Network::DrainDecision&, drainDecision, ()); @@ -99,6 +101,8 @@ class MockServerFactoryContext : public virtual ServerFactoryContext { testing::NiceMock access_log_manager_; testing::NiceMock init_manager_; testing::NiceMock lifecycle_notifier_; + testing::NiceMock + http_server_properties_cache_manager_; Singleton::ManagerPtr singleton_manager_; testing::NiceMock admin_; @@ -137,6 +141,7 @@ class StatelessMockServerFactoryContext : public virtual ServerFactoryContext { ~StatelessMockServerFactoryContext() override = default; MOCK_METHOD(Upstream::ClusterManager&, clusterManager, ()); + MOCK_METHOD(Http::HttpServerPropertiesCacheManager&, httpServerPropertiesCacheManager, ()); MOCK_METHOD(Event::Dispatcher&, mainThreadDispatcher, ()); MOCK_METHOD(const Server::Options&, options, ()); MOCK_METHOD(const Network::DrainDecision&, drainDecision, ());