From 58bd05cf8b714a2f2e31f88edc8235f2df16c3d5 Mon Sep 17 00:00:00 2001 From: Brian Avery Date: Mon, 21 Sep 2020 16:53:11 -0400 Subject: [PATCH] Revert "[1.7] Fix CVE-2020-25017 (#47)" (#48) This reverts commit c33686315c123541b58e7bd8787c9eba78e65d42. --- VERSION | 2 +- docs/root/version_history/current.rst | 183 +++++++++++++++++- docs/root/version_history/v1.15.0.rst | 166 ---------------- docs/root/version_history/v1.15.1.rst | 27 --- docs/root/version_history/version_history.rst | 2 - include/envoy/http/header_map.h | 25 --- source/common/http/BUILD | 1 - source/common/http/header_map_impl.cc | 52 +++-- source/common/http/header_map_impl.h | 6 +- source/common/http/header_utility.cc | 49 +---- source/common/http/header_utility.h | 29 --- source/common/runtime/runtime_features.cc | 2 - .../extensions/filters/common/expr/context.cc | 13 -- .../extensions/filters/common/expr/context.h | 23 +-- .../filters/common/expr/evaluator.cc | 18 +- .../filters/common/expr/evaluator.h | 4 +- .../filters/http/header_to_metadata/BUILD | 1 - .../header_to_metadata_filter.cc | 9 +- .../extensions/filters/http/jwt_authn/BUILD | 1 - .../filters/http/jwt_authn/extractor.cc | 8 +- source/extensions/filters/http/lua/BUILD | 1 - .../extensions/filters/http/lua/wrappers.cc | 9 +- test/common/http/BUILD | 2 - test/common/http/header_map_impl_test.cc | 58 +----- test/common/http/header_utility_test.cc | 72 ------- test/extensions/common/wasm/BUILD | 4 +- .../filters/common/expr/context_test.cc | 38 ++-- .../common/expr/evaluator_fuzz_test.cc | 2 +- .../header_to_metadata_filter_test.cc | 18 -- .../filters/http/jwt_authn/extractor_test.cc | 8 - .../filters/http/lua/wrappers_test.cc | 5 - .../http/rbac/rbac_filter_integration_test.cc | 100 ---------- test/per_file_coverage.sh | 2 +- test/test_common/utility.h | 3 - 34 files changed, 261 insertions(+), 682 deletions(-) delete mode 100644 docs/root/version_history/v1.15.0.rst delete mode 100644 docs/root/version_history/v1.15.1.rst diff --git a/VERSION b/VERSION index 3e192b03ce01..141f2e805beb 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.15.1-dev +1.15.0 diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index fc649bc0d0cb..d789abc26896 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -1,2 +1,181 @@ -1.15.1 (Pending) -================ +1.15.0 (July 6, 2020) +===================== + + +Incompatible Behavior Changes +----------------------------- +*Changes that are expected to cause an incompatibility if applicable; deployment changes are likely required* + +* build: official released binary is now built on Ubuntu 18.04, requires glibc >= 2.27. +* client_ssl_auth: the `auth_ip_white_list` stat has been renamed to + :ref:`auth_ip_allowlist `. +* header to metadata: on_header_missing rules with empty values are now rejected (they were skipped before). +* router: path_redirect now keeps query string by default. This behavior may be reverted by setting runtime feature `envoy.reloadable_features.preserve_query_string_in_path_redirects` to false. +* tls: fixed a bug where wilcard matching for "\*.foo.com" also matched domains of the form "a.b.foo.com". This behavior can be temporarily reverted by setting runtime feature `envoy.reloadable_features.fix_wildcard_matching` to false. + +Minor Behavior Changes +---------------------- +*Changes that may cause incompatibilities for some users, but should not for most* + +* access loggers: applied existing buffer limits to access logs, as well as :ref:`stats ` for logged / dropped logs. This can be reverted temporarily by setting runtime feature `envoy.reloadable_features.disallow_unbounded_access_logs` to false. +* build: runs as non-root inside Docker containers. Existing behaviour can be restored by setting the environment variable `ENVOY_UID` to `0`. `ENVOY_UID` and `ENVOY_GID` can be used to set the envoy user's `uid` and `gid` respectively. +* health check: in the health check filter the :ref:`percentage of healthy servers in upstream clusters ` is now interpreted as an integer. +* hot restart: added the option :option:`--use-dynamic-base-id` to select an unused base ID at startup and the option :option:`--base-id-path` to write the base id to a file (for reuse with later hot restarts). +* http: changed early error path for HTTP/1.1 so that responses consistently flow through the http connection manager, and the http filter chains. This behavior may be temporarily reverted by setting runtime feature `envoy.reloadable_features.early_errors_via_hcm` to false. +* http: fixed several bugs with applying correct connection close behavior across the http connection manager, health checker, and connection pool. This behavior may be temporarily reverted by setting runtime feature `envoy.reloadable_features.fix_connection_close` to false. +* http: fixed a bug where the upgrade header was not cleared on responses to non-upgrade requests. + Can be reverted temporarily by setting runtime feature `envoy.reloadable_features.fix_upgrade_response` to false. +* http: stopped overwriting `date` response headers. Responses without a `date` header will still have the header properly set. This behavior can be temporarily reverted by setting `envoy.reloadable_features.preserve_upstream_date` to false. +* http: stopped adding a synthetic path to CONNECT requests, meaning unconfigured CONNECT requests will now return 404 instead of 403. This behavior can be temporarily reverted by setting `envoy.reloadable_features.stop_faking_paths` to false. +* http: stopped allowing upstream 1xx or 204 responses with Transfer-Encoding or non-zero Content-Length headers. Content-Length of 0 is allowed, but stripped. This behavior can be temporarily reverted by setting `envoy.reloadable_features.strict_1xx_and_204_response_headers` to false. +* http: upstream connections will now automatically set ALPN when this value is not explicitly set elsewhere (e.g. on the upstream TLS config). This behavior may be temporarily reverted by setting runtime feature `envoy.reloadable_features.http_default_alpn` to false. +* listener: fixed a bug where when a static listener fails to be added to a worker, the listener was not removed from the active listener list. +* router: extended to allow retries of streaming or incomplete requests. This removes stat `rq_retry_skipped_request_not_complete`. +* router: extended to allow retries by default when upstream responds with :ref:`x-envoy-overloaded `. + +Bug Fixes +--------- +*Changes expected to improve the state of the world and are unlikely to have negative effects* + +* adaptive concurrency: fixed a minRTT calculation bug where requests started before the concurrency + limit was pinned to the minimum would skew the new minRTT value if the replies arrived after the + start of the new minRTT window. +* buffer: fixed CVE-2020-12603 by avoiding fragmentation, and tracking of HTTP/2 data and control frames in the output buffer. +* grpc-json: fixed a bug when in trailers only gRPC response (e.g. error) HTTP status code is not being re-written. +* http: fixed a bug in the grpc_http1_reverse_bridge filter where header-only requests were forwarded with a non-zero content length. +* http: fixed a bug where in some cases slash was moved from path to query string when :ref:`merging of adjacent slashes` is enabled. +* http: fixed CVE-2020-12604 by changing :ref:`stream_idle_timeout ` + to also defend against an HTTP/2 peer that does not open stream window once an entire response has been buffered to be sent to a downstream client. +* http: fixed CVE-2020-12605 by including request URL in request header size computation, and rejecting partial headers that exceed configured limits. +* http: fixed several bugs with applying correct connection close behavior across the http connection manager, health checker, and connection pool. This behavior may be temporarily reverted by setting runtime feature `envoy.reloadable_features.fix_connection_close` to false. +* listener: fixed CVE-2020-8663 by adding runtime support for :ref:`per-listener limits ` on active/accepted connections. +* overload management: fixed CVE-2020-8663 by adding runtime support for :ref:`global limits ` on active/accepted connections. +* prometheus stats: fixed the sort order of output lines to comply with the standard. +* udp: the :ref:`reuse_port ` listener option must now be + specified for UDP listeners if concurrency is > 1. This previously crashed so is considered a + bug fix. +* upstream: fixed a bug where Envoy would panic when receiving a GRPC SERVICE_UNKNOWN status on the health check. + +Removed Config or Runtime +------------------------- +*Normally occurs at the end of the* :ref:`deprecation period ` + +* http: removed legacy connection pool code and their runtime features: `envoy.reloadable_features.new_http1_connection_pool_behavior` and + `envoy.reloadable_features.new_http2_connection_pool_behavior`. + +New Features +------------ + +* access loggers: added file access logger config :ref:`log_format `. +* access loggers: added GRPC_STATUS operator on logging format. +* access loggers: added gRPC access logger config added :ref:`API version ` to explicitly set the version of gRPC service endpoint and message to be used. +* access loggers: extended specifier for FilterStateFormatter to output :ref:`unstructured log string `. +* admin: added support for dumping EDS config at :ref:`/config_dump?include_eds `. +* aggregate cluster: made route :ref:`retry_priority ` predicates work with :ref:`this cluster type `. +* build: official released binary is now built on Ubuntu 18.04, requires glibc >= 2.27. +* build: official released binary is now built with Clang 10.0.0. +* cluster: added an extension point for configurable :ref:`upstreams `. +* compressor: exposed generic :ref:`compressor ` filter to users. +* config: added :ref:`identifier ` stat that reflects control plane identifier. +* config: added :ref:`version_text ` stat that reflects xDS version. +* decompressor: exposed generic :ref:`decompressor ` filter to users. +* dynamic forward proxy: added :ref:`SNI based dynamic forward proxy ` support. +* dynamic forward proxy: added configurable :ref:`circuit breakers ` for resolver on DNS cache. + This behavior can be temporarily disabled by the runtime feature `envoy.reloadable_features.enable_dns_cache_circuit_breakers`. + If this runtime feature is disabled, the upstream circuit breakers for the cluster will be used even if the :ref:`DNS Cache circuit breakers ` are configured. +* dynamic forward proxy: added :ref:`allow_insecure_cluster_options` to allow disabling of auto_san_validation and auto_sni. +* ext_authz filter: added :ref:`v2 deny_at_disable `, :ref:`v3 deny_at_disable `. This allows force denying protected paths while filter gets disabled, by setting this key to true. +* ext_authz filter: added API version field for both :ref:`HTTP ` + and :ref:`Network ` filters to explicitly set the version of gRPC service endpoint and message to be used. +* ext_authz filter: added :ref:`v3 allowed_upstream_headers_to_append ` to allow appending multiple header entries (returned by the authorization server) with the same key to the original request headers. +* fault: added support for controlling the percentage of requests that abort, delay and response rate limits faults + are applied to using :ref:`HTTP headers ` to the HTTP fault filter. +* fault: added support for specifying grpc_status code in abort faults using + :ref:`HTTP header ` or abort fault configuration in HTTP fault filter. +* filter: added `upstram_rq_time` stats to the GPRC stats filter. + Disabled by default and can be enabled via :ref:`enable_upstream_stats `. +* grpc: added support for Google gRPC :ref:`custom channel arguments `. +* grpc-json: added support for streaming response using + `google.api.HttpBody `_. +* grpc-json: send a `x-envoy-original-method` header to grpc services. +* gzip filter: added option to set zlib's next output buffer size. +* hds: updated to allow to explicitly set the API version of gRPC service endpoint and message to be used. +* header to metadata: added support for regex substitutions on header values. +* health checks: allowed configuring health check transport sockets by specifying :ref:`transport socket match criteria `. +* http: added :ref:`local_reply config ` to http_connection_manager to customize :ref:`local reply `. +* http: added :ref:`stripping port from host header ` support. +* http: added support for proxying CONNECT requests, terminating CONNECT requests, and converting raw TCP streams into HTTP/2 CONNECT requests. See :ref:`upgrade documentation` for details. +* listener: added in place filter chain update flow for tcp listener update which doesn't close connections if the corresponding network filter chain is equivalent during the listener update. + Can be disabled by setting runtime feature `envoy.reloadable_features.listener_in_place_filterchain_update` to false. + Also added additional draining filter chain stat for :ref:`listener manager ` to track the number of draining filter chains and the number of in place update attempts. +* logger: added :option:`--log-format-prefix-with-location` command line option to prefix '%v' with file path and line number. +* lrs: added new *envoy_api_field_service.load_stats.v2.LoadStatsResponse.send_all_clusters* field + in LRS response, which allows management servers to avoid explicitly listing all clusters it is + interested in; behavior is allowed based on new "envoy.lrs.supports_send_all_clusters" capability + in :ref:`client_features` field. +* lrs: updated to allow to explicitly set the API version of gRPC service endpoint and message to be used. +* lua: added :ref:`per route config ` for Lua filter. +* lua: added tracing to the ``httpCall()`` API. +* metrics service: added :ref:`API version ` to explicitly set the version of gRPC service endpoint and message to be used. +* network filters: added a :ref:`postgres proxy filter `. +* network filters: added a :ref:`rocketmq proxy filter `. +* performance: enabled stats symbol table implementation by default. To disable it, add + `--use-fake-symbol-table 1` to the command-line arguments when starting Envoy. +* ratelimit: added support for use of dynamic metadata :ref:`dynamic_metadata ` as a ratelimit action. +* ratelimit: added :ref:`API version ` to explicitly set the version of gRPC service endpoint and message to be used. +* ratelimit: support specifying dynamic overrides in rate limit descriptors using :ref:`limit override ` config. +* redis: added acl support :ref:`downstream_auth_username ` for downstream client ACL authentication, and :ref:`auth_username ` to configure authentication usernames for upstream Redis 6+ server clusters with ACL enabled. +* regex: added support for enforcing max program size via runtime and stats to monitor program size for :ref:`Google RE2 `. +* request_id: added to :ref:`always_set_request_id_in_response setting ` + to set :ref:`x-request-id ` header in response even if + tracing is not forced. +* router: added more fine grained internal redirect configs to the :ref:`internal_redirect_policy + ` field. +* router: added regex substitution support for header based hashing. +* router: added support for RESPONSE_FLAGS and RESPONSE_CODE_DETAILS :ref:`header formatters + `. +* router: allow Rate Limiting Service to be called in case of missing request header for a descriptor if the :ref:`skip_if_absent ` field is set to true. +* runtime: added new gauge :ref:`deprecated_feature_seen_since_process_start ` that gets reset across hot restarts. +* server: added the option :option:`--drain-strategy` to enable different drain strategies for DrainManager::drainClose(). +* server: added :ref:`server.envoy_bug_failures ` statistic to count ENVOY_BUG failures. +* stats: added the option to :ref:`report counters as deltas ` to the metrics service stats sink. +* tracing: made tracing configuration fully dynamic and every HTTP connection manager + can now have a separate :ref:`tracing provider `. +* udp: upgraded :ref:`udp_proxy ` filter to v3 and promoted it out of alpha. +* dynamic_forward_proxy: added :ref:`use_tcp_for_dns_lookups` option to use TCP for DNS lookups in order to match the DNS options for :ref:`Clusters`. +* ext_authz filter: added support for emitting dynamic metadata for both :ref:`HTTP ` and :ref:`network ` filters. +* grpc-json: support specifying `response_body` field in for `google.api.HttpBody` message. +* http: introduced new HTTP/1 and HTTP/2 codec implementations that will remove the use of exceptions for control flow due to high risk factors and instead use error statuses. The old behavior is deprecated, but can be used during the removal period by setting the runtime feature `envoy.reloadable_features.new_codec_behavior` to false. The removal period will be one month. +* load balancer: added a :ref:`configuration` option to specify the active request bias used by the least request load balancer. +* redis: added fault injection support :ref:`fault injection for redis proxy `, described further in :ref:`configuration documentation `. +* router: added new + :ref:`envoy-ratelimited` + retry policy, which allows retrying envoy's own rate limited responses. +* stats: added optional histograms to :ref:`cluster stats ` + that track headers and body sizes of requests and responses. +* stats: allow configuring histogram buckets for stats sinks and admin endpoints that support it. +* tap: added :ref:`generic body matcher` to scan http requests and responses for text or hex patterns. +* tcp: switched the TCP connection pool to the new "shared" connection pool, sharing a common code base with HTTP and HTTP/2. Any unexpected behavioral changes can be temporarily reverted by setting `envoy.reloadable_features.new_tcp_connection_pool` to false. +* xds: added :ref:`extension config discovery` support for HTTP filters. + +Deprecated +---------- + +* Tracing provider configuration as part of :ref:`bootstrap config ` + has been deprecated in favor of configuration as part of :ref:`HTTP connection manager + `. +* The :ref:`HTTP Gzip filter ` has been deprecated in favor of + :ref:`Compressor `. +* The * :ref:`GoogleRE2.max_program_size` + field is now deprecated. Management servers are expected to validate regexp program sizes + instead of expecting the client to do it. Alternatively, the max program size can be enforced by Envoy via runtime. +* The :ref:`internal_redirect_action ` + field and :ref:`max_internal_redirects ` field + are now deprecated. This changes the implemented default cross scheme redirect behavior. + All cross scheme redirects are disallowed by default. To restore + the previous behavior, set allow_cross_scheme_redirect=true and use + :ref:`safe_cross_scheme`, + in :ref:`predicates `. +* File access logger fields :ref:`format `, :ref:`json_format ` and :ref:`typed_json_format ` are deprecated in favor of :ref:`log_format `. +* A warning is now logged when v2 xDS api is used. This behavior can be temporarily disabled by setting `envoy.reloadable_features.enable_deprecated_v2_api_warning` to `false`. +* Using cluster circuit breakers for DNS Cache is now deprecated in favor of :ref:`DNS cache circuit breakers `. This behavior can be temporarily disabled by setting `envoy.reloadable_features.enable_dns_cache_circuit_breakers` to `false`. diff --git a/docs/root/version_history/v1.15.0.rst b/docs/root/version_history/v1.15.0.rst deleted file mode 100644 index 94ac2469a69d..000000000000 --- a/docs/root/version_history/v1.15.0.rst +++ /dev/null @@ -1,166 +0,0 @@ -1.15.0 (July 6, 2020) -===================== - - -Incompatible Behavior Changes ------------------------------ -*Changes that are expected to cause an incompatibility if applicable; deployment changes are likely required* - -* build: official released binary is now built on Ubuntu 18.04, requires glibc >= 2.27. -* client_ssl_auth: the `auth_ip_white_list` stat has been renamed to - :ref:`auth_ip_allowlist `. -* header to metadata: on_header_missing rules with empty values are now rejected (they were skipped before). -* router: path_redirect now keeps query string by default. This behavior may be reverted by setting runtime feature `envoy.reloadable_features.preserve_query_string_in_path_redirects` to false. -* tls: fixed a bug where wilcard matching for "\*.foo.com" also matched domains of the form "a.b.foo.com". This behavior can be temporarily reverted by setting runtime feature `envoy.reloadable_features.fix_wildcard_matching` to false. - -Minor Behavior Changes ----------------------- -*Changes that may cause incompatibilities for some users, but should not for most* - -* access loggers: applied existing buffer limits to access logs, as well as :ref:`stats ` for logged / dropped logs. This can be reverted temporarily by setting runtime feature `envoy.reloadable_features.disallow_unbounded_access_logs` to false. -* build: runs as non-root inside Docker containers. Existing behaviour can be restored by setting the environment variable `ENVOY_UID` to `0`. `ENVOY_UID` and `ENVOY_GID` can be used to set the envoy user's `uid` and `gid` respectively. -* health check: in the health check filter the :ref:`percentage of healthy servers in upstream clusters ` is now interpreted as an integer. -* hot restart: added the option :option:`--use-dynamic-base-id` to select an unused base ID at startup and the option :option:`--base-id-path` to write the base id to a file (for reuse with later hot restarts). -* http: changed early error path for HTTP/1.1 so that responses consistently flow through the http connection manager, and the http filter chains. This behavior may be temporarily reverted by setting runtime feature `envoy.reloadable_features.early_errors_via_hcm` to false. -* http: fixed several bugs with applying correct connection close behavior across the http connection manager, health checker, and connection pool. This behavior may be temporarily reverted by setting runtime feature `envoy.reloadable_features.fix_connection_close` to false. -* http: fixed a bug where the upgrade header was not cleared on responses to non-upgrade requests. - Can be reverted temporarily by setting runtime feature `envoy.reloadable_features.fix_upgrade_response` to false. -* http: stopped overwriting `date` response headers. Responses without a `date` header will still have the header properly set. This behavior can be temporarily reverted by setting `envoy.reloadable_features.preserve_upstream_date` to false. -* http: stopped adding a synthetic path to CONNECT requests, meaning unconfigured CONNECT requests will now return 404 instead of 403. This behavior can be temporarily reverted by setting `envoy.reloadable_features.stop_faking_paths` to false. -* http: stopped allowing upstream 1xx or 204 responses with Transfer-Encoding or non-zero Content-Length headers. Content-Length of 0 is allowed, but stripped. This behavior can be temporarily reverted by setting `envoy.reloadable_features.strict_1xx_and_204_response_headers` to false. -* http: upstream connections will now automatically set ALPN when this value is not explicitly set elsewhere (e.g. on the upstream TLS config). This behavior may be temporarily reverted by setting runtime feature `envoy.reloadable_features.http_default_alpn` to false. -* listener: fixed a bug where when a static listener fails to be added to a worker, the listener was not removed from the active listener list. -* router: extended to allow retries of streaming or incomplete requests. This removes stat `rq_retry_skipped_request_not_complete`. -* router: extended to allow retries by default when upstream responds with :ref:`x-envoy-overloaded `. - -Bug Fixes ---------- -*Changes expected to improve the state of the world and are unlikely to have negative effects* - -* adaptive concurrency: fixed a minRTT calculation bug where requests started before the concurrency - limit was pinned to the minimum would skew the new minRTT value if the replies arrived after the - start of the new minRTT window. -* buffer: fixed CVE-2020-12603 by avoiding fragmentation, and tracking of HTTP/2 data and control frames in the output buffer. -* grpc-json: fixed a bug when in trailers only gRPC response (e.g. error) HTTP status code is not being re-written. -* http: fixed a bug in the grpc_http1_reverse_bridge filter where header-only requests were forwarded with a non-zero content length. -* http: fixed a bug where in some cases slash was moved from path to query string when :ref:`merging of adjacent slashes` is enabled. -* http: fixed CVE-2020-12604 by changing :ref:`stream_idle_timeout ` - to also defend against an HTTP/2 peer that does not open stream window once an entire response has been buffered to be sent to a downstream client. -* http: fixed CVE-2020-12605 by including request URL in request header size computation, and rejecting partial headers that exceed configured limits. -* http: fixed several bugs with applying correct connection close behavior across the http connection manager, health checker, and connection pool. This behavior may be temporarily reverted by setting runtime feature `envoy.reloadable_features.fix_connection_close` to false. -* listener: fixed CVE-2020-8663 by adding runtime support for :ref:`per-listener limits ` on active/accepted connections. -* overload management: fixed CVE-2020-8663 by adding runtime support for :ref:`global limits ` on active/accepted connections. -* prometheus stats: fixed the sort order of output lines to comply with the standard. -* udp: the :ref:`reuse_port ` listener option must now be - specified for UDP listeners if concurrency is > 1. This previously crashed so is considered a - bug fix. -* upstream: fixed a bug where Envoy would panic when receiving a GRPC SERVICE_UNKNOWN status on the health check. - -Removed Config or Runtime -------------------------- -*Normally occurs at the end of the* :ref:`deprecation period ` - -* http: removed legacy connection pool code and their runtime features: `envoy.reloadable_features.new_http1_connection_pool_behavior` and - `envoy.reloadable_features.new_http2_connection_pool_behavior`. - -New Features ------------- - -* access loggers: added file access logger config :ref:`log_format `. -* access loggers: added GRPC_STATUS operator on logging format. -* access loggers: added gRPC access logger config added :ref:`API version ` to explicitly set the version of gRPC service endpoint and message to be used. -* access loggers: extended specifier for FilterStateFormatter to output :ref:`unstructured log string `. -* admin: added support for dumping EDS config at :ref:`/config_dump?include_eds `. -* aggregate cluster: made route :ref:`retry_priority ` predicates work with :ref:`this cluster type `. -* build: official released binary is now built on Ubuntu 18.04, requires glibc >= 2.27. -* build: official released binary is now built with Clang 10.0.0. -* cluster: added an extension point for configurable :ref:`upstreams `. -* compressor: exposed generic :ref:`compressor ` filter to users. -* config: added :ref:`identifier ` stat that reflects control plane identifier. -* config: added :ref:`version_text ` stat that reflects xDS version. -* decompressor: exposed generic :ref:`decompressor ` filter to users. -* dynamic forward proxy: added :ref:`SNI based dynamic forward proxy ` support. -* dynamic forward proxy: added configurable :ref:`circuit breakers ` for resolver on DNS cache. - This behavior can be temporarily disabled by the runtime feature `envoy.reloadable_features.enable_dns_cache_circuit_breakers`. - If this runtime feature is disabled, the upstream circuit breakers for the cluster will be used even if the :ref:`DNS Cache circuit breakers ` are configured. -* dynamic forward proxy: added :ref:`allow_insecure_cluster_options` to allow disabling of auto_san_validation and auto_sni. -* ext_authz filter: added :ref:`v2 deny_at_disable `, :ref:`v3 deny_at_disable `. This allows force denying protected paths while filter gets disabled, by setting this key to true. -* ext_authz filter: added API version field for both :ref:`HTTP ` - and :ref:`Network ` filters to explicitly set the version of gRPC service endpoint and message to be used. -* ext_authz filter: added :ref:`v3 allowed_upstream_headers_to_append ` to allow appending multiple header entries (returned by the authorization server) with the same key to the original request headers. -* fault: added support for controlling the percentage of requests that abort, delay and response rate limits faults - are applied to using :ref:`HTTP headers ` to the HTTP fault filter. -* fault: added support for specifying grpc_status code in abort faults using - :ref:`HTTP header ` or abort fault configuration in HTTP fault filter. -* filter: added `upstram_rq_time` stats to the GPRC stats filter. - Disabled by default and can be enabled via :ref:`enable_upstream_stats `. -* grpc: added support for Google gRPC :ref:`custom channel arguments `. -* grpc-json: added support for streaming response using - `google.api.HttpBody `_. -* grpc-json: send a `x-envoy-original-method` header to grpc services. -* gzip filter: added option to set zlib's next output buffer size. -* hds: updated to allow to explicitly set the API version of gRPC service endpoint and message to be used. -* header to metadata: added support for regex substitutions on header values. -* health checks: allowed configuring health check transport sockets by specifying :ref:`transport socket match criteria `. -* http: added :ref:`local_reply config ` to http_connection_manager to customize :ref:`local reply `. -* http: added :ref:`stripping port from host header ` support. -* http: added support for proxying CONNECT requests, terminating CONNECT requests, and converting raw TCP streams into HTTP/2 CONNECT requests. See :ref:`upgrade documentation` for details. -* listener: added in place filter chain update flow for tcp listener update which doesn't close connections if the corresponding network filter chain is equivalent during the listener update. - Can be disabled by setting runtime feature `envoy.reloadable_features.listener_in_place_filterchain_update` to false. - Also added additional draining filter chain stat for :ref:`listener manager ` to track the number of draining filter chains and the number of in place update attempts. -* logger: added :option:`--log-format-prefix-with-location` command line option to prefix '%v' with file path and line number. -* lrs: added new *envoy_api_field_service.load_stats.v2.LoadStatsResponse.send_all_clusters* field - in LRS response, which allows management servers to avoid explicitly listing all clusters it is - interested in; behavior is allowed based on new "envoy.lrs.supports_send_all_clusters" capability - in :ref:`client_features` field. -* lrs: updated to allow to explicitly set the API version of gRPC service endpoint and message to be used. -* lua: added :ref:`per route config ` for Lua filter. -* lua: added tracing to the ``httpCall()`` API. -* metrics service: added :ref:`API version ` to explicitly set the version of gRPC service endpoint and message to be used. -* network filters: added a :ref:`postgres proxy filter `. -* network filters: added a :ref:`rocketmq proxy filter `. -* performance: enabled stats symbol table implementation by default. To disable it, add - `--use-fake-symbol-table 1` to the command-line arguments when starting Envoy. -* ratelimit: added support for use of dynamic metadata :ref:`dynamic_metadata ` as a ratelimit action. -* ratelimit: added :ref:`API version ` to explicitly set the version of gRPC service endpoint and message to be used. -* ratelimit: support specifying dynamic overrides in rate limit descriptors using :ref:`limit override ` config. -* redis: added acl support :ref:`downstream_auth_username ` for downstream client ACL authentication, and :ref:`auth_username ` to configure authentication usernames for upstream Redis 6+ server clusters with ACL enabled. -* regex: added support for enforcing max program size via runtime and stats to monitor program size for :ref:`Google RE2 `. -* request_id: added to :ref:`always_set_request_id_in_response setting ` - to set :ref:`x-request-id ` header in response even if - tracing is not forced. -* router: added more fine grained internal redirect configs to the :ref:`internal_redirect_policy - ` field. -* router: added regex substitution support for header based hashing. -* router: added support for RESPONSE_FLAGS and RESPONSE_CODE_DETAILS :ref:`header formatters - `. -* router: allow Rate Limiting Service to be called in case of missing request header for a descriptor if the :ref:`skip_if_absent ` field is set to true. -* runtime: added new gauge :ref:`deprecated_feature_seen_since_process_start ` that gets reset across hot restarts. -* server: added the option :option:`--drain-strategy` to enable different drain strategies for DrainManager::drainClose(). -* server: added :ref:`server.envoy_bug_failures ` statistic to count ENVOY_BUG failures. -* stats: added the option to :ref:`report counters as deltas ` to the metrics service stats sink. -* tracing: made tracing configuration fully dynamic and every HTTP connection manager - can now have a separate :ref:`tracing provider `. -* udp: upgraded :ref:`udp_proxy ` filter to v3 and promoted it out of alpha. - -Deprecated ----------- - -* Tracing provider configuration as part of :ref:`bootstrap config ` - has been deprecated in favor of configuration as part of :ref:`HTTP connection manager - `. -* The :ref:`HTTP Gzip filter ` has been deprecated in favor of - :ref:`Compressor `. -* The * :ref:`GoogleRE2.max_program_size` - field is now deprecated. Management servers are expected to validate regexp program sizes - instead of expecting the client to do it. Alternatively, the max program size can be enforced by Envoy via runtime. -* The :ref:`internal_redirect_action ` - field and :ref:`max_internal_redirects ` field - are now deprecated. This changes the implemented default cross scheme redirect behavior. - All cross scheme redirects are disallowed by default. To restore - the previous behavior, set allow_cross_scheme_redirect=true and use - :ref:`safe_cross_scheme`, - in :ref:`predicates `. -* File access logger fields :ref:`format `, :ref:`json_format ` and :ref:`typed_json_format ` are deprecated in favor of :ref:`log_format `. -* A warning is now logged when v2 xDS api is used. This behavior can be temporarily disabled by setting `envoy.reloadable_features.enable_deprecated_v2_api_warning` to `false`. -* Using cluster circuit breakers for DNS Cache is now deprecated in favor of :ref:`DNS cache circuit breakers `. This behavior can be temporarily disabled by setting `envoy.reloadable_features.enable_dns_cache_circuit_breakers` to `false`. diff --git a/docs/root/version_history/v1.15.1.rst b/docs/root/version_history/v1.15.1.rst deleted file mode 100644 index 57b2acb3f307..000000000000 --- a/docs/root/version_history/v1.15.1.rst +++ /dev/null @@ -1,27 +0,0 @@ -1.15.1 (TBD) -============ - -Changes -------- -* http: fixed CVE-2020-25017. Previously header matching did not match on all headers for non-inline - headers. This patch changes the default behavior to always logically match on all headers. - Multiple individual headers will be logically concatenated with ',' similar to what is done with - inline headers. This makes the behavior effectively consistent. This behavior can be temporary - reverted by setting the runtime value "envoy.reloadable_features.header_match_on_all_headers" to - "false". - - Targeted fixes have been additionally performed on the following extensions which make them - consider all duplicate headers by default as a comma concatenated list: - - 1. Any extension using CEL matching on headers. - 2. The header to metadata filter. - 3. The JWT filter. - 4. The Lua filter. - - Like primary header matching used in routing, RBAC, etc. this behavior can be disabled by setting - the runtime value "envoy.reloadable_features.header_match_on_all_headers" to false. -* http: The setCopy() header map API previously only set the first header in the case of duplicate - non-inline headers. setCopy() now behaves similarly to the other set*() APIs and replaces all found - headers with a single value. This may have had security implications in the extauth filter which - uses this API. This behavior can be disabled by setting the runtime value - "envoy.reloadable_features.http_set_copy_replace_all_headers" to false. diff --git a/docs/root/version_history/version_history.rst b/docs/root/version_history/version_history.rst index d9d9993e500c..2d7744bf2310 100644 --- a/docs/root/version_history/version_history.rst +++ b/docs/root/version_history/version_history.rst @@ -7,8 +7,6 @@ Version history :titlesonly: current - v1.15.1 - v1.15.0 v1.14.3 v1.14.2 v1.14.1 diff --git a/include/envoy/http/header_map.h b/include/envoy/http/header_map.h index 6f172d78e760..82a79d4fa6e5 100644 --- a/include/envoy/http/header_map.h +++ b/include/envoy/http/header_map.h @@ -513,31 +513,6 @@ class HeaderMap { */ virtual const HeaderEntry* get(const LowerCaseString& key) const PURE; - /** - * This is a wrapper for the return result from getAll(). It avoids a copy when translating from - * non-const HeaderEntry to const HeaderEntry and only provides const access to the result. - */ - using NonConstGetResult = absl::InlinedVector; - class GetResult { - public: - GetResult() = default; - explicit GetResult(NonConstGetResult&& result) : result_(std::move(result)) {} - - bool empty() const { return result_.empty(); } - size_t size() const { return result_.size(); } - const HeaderEntry* operator[](size_t i) const { return result_[i]; } - - private: - NonConstGetResult result_; - }; - - /** - * Get a header by key. - * @param key supplies the header key. - * @return all header entries matching the key. - */ - virtual GetResult getAll(const LowerCaseString& key) const PURE; - // aliases to make iterate() and iterateReverse() callbacks easier to read enum class Iterate { Continue, Break }; diff --git a/source/common/http/BUILD b/source/common/http/BUILD index 4aab9eecf43f..782eb8d28f16 100644 --- a/source/common/http/BUILD +++ b/source/common/http/BUILD @@ -275,7 +275,6 @@ envoy_cc_library( "//source/common/common:empty_string", "//source/common/common:non_copyable", "//source/common/common:utility_lib", - "//source/common/runtime:runtime_features_lib", "//source/common/singleton:const_singleton", ], ) diff --git a/source/common/http/header_map_impl.cc b/source/common/http/header_map_impl.cc index e29569510077..8803997b706d 100644 --- a/source/common/http/header_map_impl.cc +++ b/source/common/http/header_map_impl.cc @@ -10,7 +10,6 @@ #include "common/common/assert.h" #include "common/common/dump_state_utils.h" #include "common/common/empty_string.h" -#include "common/runtime/runtime_features.h" #include "common/singleton/const_singleton.h" #include "absl/strings/match.h" @@ -385,9 +384,9 @@ void HeaderMapImpl::addCopy(const LowerCaseString& key, absl::string_view value) void HeaderMapImpl::appendCopy(const LowerCaseString& key, absl::string_view value) { // TODO(#9221): converge on and document a policy for coalescing multiple headers. - auto entry = getExisting(key); - if (!entry.empty()) { - const uint64_t added_size = appendToHeader(entry[0]->value(), value); + auto* entry = getExisting(key); + if (entry) { + const uint64_t added_size = appendToHeader(entry->value(), value); addSize(added_size); } else { addCopy(key, value); @@ -395,27 +394,29 @@ void HeaderMapImpl::appendCopy(const LowerCaseString& key, absl::string_view val } void HeaderMapImpl::setReference(const LowerCaseString& key, absl::string_view value) { + HeaderString ref_key(key); + HeaderString ref_value(value); remove(key); - addReference(key, value); + insertByKey(std::move(ref_key), std::move(ref_value)); } void HeaderMapImpl::setReferenceKey(const LowerCaseString& key, absl::string_view value) { + HeaderString ref_key(key); + HeaderString new_value; + new_value.setCopy(value); remove(key); - addReferenceKey(key, value); + insertByKey(std::move(ref_key), std::move(new_value)); + ASSERT(new_value.empty()); // NOLINT(bugprone-use-after-move) } void HeaderMapImpl::setCopy(const LowerCaseString& key, absl::string_view value) { - if (!Runtime::runtimeFeatureEnabled( - "envoy.reloadable_features.http_set_copy_replace_all_headers")) { - auto entry = getExisting(key); - if (!entry.empty()) { - updateSize(entry[0]->value().size(), value.size()); - entry[0]->value(value); - } else { - addCopy(key, value); - } + // Replaces the first occurrence of a header if it exists, otherwise adds by copy. + // TODO(#9221): converge on and document a policy for coalescing multiple headers. + auto* entry = getExisting(key); + if (entry) { + updateSize(entry->value().size(), value.size()); + entry->value(value); } else { - remove(key); addCopy(key, value); } } @@ -433,26 +434,17 @@ void HeaderMapImpl::verifyByteSizeInternalForTest() const { } const HeaderEntry* HeaderMapImpl::get(const LowerCaseString& key) const { - const auto result = getAll(key); - return result.empty() ? nullptr : result[0]; + return const_cast(this)->getExisting(key); } -HeaderMap::GetResult HeaderMapImpl::getAll(const LowerCaseString& key) const { - return HeaderMap::GetResult(const_cast(this)->getExisting(key)); -} - -HeaderMap::NonConstGetResult HeaderMapImpl::getExisting(const LowerCaseString& key) { +HeaderEntry* HeaderMapImpl::getExisting(const LowerCaseString& key) { // Attempt a trie lookup first to see if the user is requesting an O(1) header. This may be // relatively common in certain header matching / routing patterns. // TODO(mattklein123): Add inline handle support directly to the header matcher code to support // this use case more directly. - HeaderMap::NonConstGetResult ret; auto lookup = staticLookup(key.get()); if (lookup.has_value()) { - if (*lookup.value().entry_ != nullptr) { - ret.push_back(*lookup.value().entry_); - } - return ret; + return *lookup.value().entry_; } // If the requested header is not an O(1) header we do a full scan. Doing the trie lookup is @@ -463,11 +455,11 @@ HeaderMap::NonConstGetResult HeaderMapImpl::getExisting(const LowerCaseString& k // implementation or potentially create a lazy map if the size of the map is above a threshold. for (HeaderEntryImpl& header : headers_) { if (header.key() == key.get().c_str()) { - ret.push_back(&header); + return &header; } } - return ret; + return nullptr; } void HeaderMapImpl::iterate(HeaderMap::ConstIterateCb cb, void* context) const { diff --git a/source/common/http/header_map_impl.h b/source/common/http/header_map_impl.h index 728c3248ef42..693762a21aeb 100644 --- a/source/common/http/header_map_impl.h +++ b/source/common/http/header_map_impl.h @@ -86,7 +86,6 @@ class HeaderMapImpl : NonCopyable { void setCopy(const LowerCaseString& key, absl::string_view value); uint64_t byteSize() const; const HeaderEntry* get(const LowerCaseString& key) const; - HeaderMap::GetResult getAll(const LowerCaseString& key) const; void iterate(HeaderMap::ConstIterateCb cb, void* context) const; void iterateReverse(HeaderMap::ConstIterateCb cb, void* context) const; void clear(); @@ -241,7 +240,7 @@ class HeaderMapImpl : NonCopyable { HeaderEntryImpl& maybeCreateInline(HeaderEntryImpl** entry, const LowerCaseString& key); HeaderEntryImpl& maybeCreateInline(HeaderEntryImpl** entry, const LowerCaseString& key, HeaderString&& value); - HeaderMap::NonConstGetResult getExisting(const LowerCaseString& key); + HeaderEntry* getExisting(const LowerCaseString& key); size_t removeInline(HeaderEntryImpl** entry); void updateSize(uint64_t from_size, uint64_t to_size); void addSize(uint64_t size); @@ -299,9 +298,6 @@ template class TypedHeaderMapImpl : public HeaderMapImpl, publ const HeaderEntry* get(const LowerCaseString& key) const override { return HeaderMapImpl::get(key); } - HeaderMap::GetResult getAll(const LowerCaseString& key) const override { - return HeaderMapImpl::getAll(key); - } void iterate(HeaderMap::ConstIterateCb cb, void* context) const override { HeaderMapImpl::iterate(cb, context); } diff --git a/source/common/http/header_utility.cc b/source/common/http/header_utility.cc index 66b3f5fb0348..b180ad3ead3b 100644 --- a/source/common/http/header_utility.cc +++ b/source/common/http/header_utility.cc @@ -106,65 +106,36 @@ bool HeaderUtility::matchHeaders(const HeaderMap& request_headers, return true; } -HeaderUtility::GetAllOfHeaderAsStringResult -HeaderUtility::getAllOfHeaderAsString(const HeaderMap& headers, const Http::LowerCaseString& key) { - GetAllOfHeaderAsStringResult result; - const auto header_value = headers.getAll(key); - - if (header_value.empty()) { - // Empty for clarity. Avoid handling the empty case in the block below if the runtime feature - // is disabled. - } else if (header_value.size() == 1 || - !Runtime::runtimeFeatureEnabled( - "envoy.reloadable_features.http_match_on_all_headers")) { - result.result_ = header_value[0]->value().getStringView(); - } else { - // In this case we concatenate all found headers using a ',' delimiter before performing the - // final match. We use an InlinedVector of absl::string_view to invoke the optimized join - // algorithm. This requires a copying phase before we invoke join. The 3 used as the inline - // size has been arbitrarily chosen. - // TODO(mattklein123): Do we need to normalize any whitespace here? - absl::InlinedVector string_view_vector; - string_view_vector.reserve(header_value.size()); - for (size_t i = 0; i < header_value.size(); i++) { - string_view_vector.push_back(header_value[i]->value().getStringView()); - } - result.result_backing_string_ = absl::StrJoin(string_view_vector, ","); - } - - return result; -} - bool HeaderUtility::matchHeaders(const HeaderMap& request_headers, const HeaderData& header_data) { - const auto header_value = getAllOfHeaderAsString(request_headers, header_data.name_); + const HeaderEntry* header = request_headers.get(header_data.name_); - if (!header_value.result().has_value()) { + if (header == nullptr) { return header_data.invert_match_ && header_data.header_match_type_ == HeaderMatchType::Present; } bool match; + const absl::string_view header_view = header->value().getStringView(); switch (header_data.header_match_type_) { case HeaderMatchType::Value: - match = header_data.value_.empty() || header_value.result().value() == header_data.value_; + match = header_data.value_.empty() || header_view == header_data.value_; break; case HeaderMatchType::Regex: - match = header_data.regex_->match(header_value.result().value()); + match = header_data.regex_->match(header_view); break; case HeaderMatchType::Range: { - int64_t header_int_value = 0; - match = absl::SimpleAtoi(header_value.result().value(), &header_int_value) && - header_int_value >= header_data.range_.start() && - header_int_value < header_data.range_.end(); + int64_t header_value = 0; + match = absl::SimpleAtoi(header_view, &header_value) && + header_value >= header_data.range_.start() && header_value < header_data.range_.end(); break; } case HeaderMatchType::Present: match = true; break; case HeaderMatchType::Prefix: - match = absl::StartsWith(header_value.result().value(), header_data.value_); + match = absl::StartsWith(header_view, header_data.value_); break; case HeaderMatchType::Suffix: - match = absl::EndsWith(header_value.result().value(), header_data.value_); + match = absl::EndsWith(header_view, header_data.value_); break; default: NOT_REACHED_GCOVR_EXCL_LINE; diff --git a/source/common/http/header_utility.h b/source/common/http/header_utility.h index 76378886a9bd..b357563b4c9a 100644 --- a/source/common/http/header_utility.h +++ b/source/common/http/header_utility.h @@ -33,35 +33,6 @@ class HeaderUtility { static void getAllOfHeader(const HeaderMap& headers, absl::string_view key, std::vector& out); - /** - * Get all header values as a single string. Multiple headers are concatenated with ','. - */ - class GetAllOfHeaderAsStringResult { - public: - // The ultimate result of the concatenation. If absl::nullopt, no header values were found. - // If the final string required a string allocation, the memory is held in - // backingString(). This allows zero allocation in the common case of a single header - // value. - absl::optional result() const { - // This is safe for move/copy of this class as the backing string will be moved or copied. - // Otherwise result_ is valid. The assert verifies that both are empty or only 1 is set. - ASSERT((!result_.has_value() && result_backing_string_.empty()) || - (result_.has_value() ^ !result_backing_string_.empty())); - return !result_backing_string_.empty() ? result_backing_string_ : result_; - } - - const std::string& backingString() const { return result_backing_string_; } - - private: - absl::optional result_; - // Valid only if result_ relies on memory allocation that must live beyond the call. See above. - std::string result_backing_string_; - - friend class HeaderUtility; - }; - static GetAllOfHeaderAsStringResult getAllOfHeaderAsString(const HeaderMap& headers, - const Http::LowerCaseString& key); - // A HeaderData specifies one of exact value or regex or range element // to match in a request's header, specified in the header_match_type_ member. // It is the runtime equivalent of the HeaderMatchSpecifier proto in RDS API. diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 3191336d19a8..ab28cf83f59b 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -71,8 +71,6 @@ constexpr const char* runtime_features[] = { "envoy.reloadable_features.fix_wildcard_matching", "envoy.reloadable_features.fixed_connection_close", "envoy.reloadable_features.http_default_alpn", - "envoy.reloadable_features.http_match_on_all_headers", - "envoy.reloadable_features.http_set_copy_replace_all_headers", "envoy.reloadable_features.listener_in_place_filterchain_update", "envoy.reloadable_features.preserve_query_string_in_path_redirects", "envoy.reloadable_features.preserve_upstream_date", diff --git a/source/extensions/filters/common/expr/context.cc b/source/extensions/filters/common/expr/context.cc index affb7a4095ec..97420096adcd 100644 --- a/source/extensions/filters/common/expr/context.cc +++ b/source/extensions/filters/common/expr/context.cc @@ -23,19 +23,6 @@ absl::optional convertHeaderEntry(const Http::HeaderEntry* header) { return CelValue::CreateStringView(header->value().getStringView()); } -absl::optional -convertHeaderEntry(Protobuf::Arena& arena, - Http::HeaderUtility::GetAllOfHeaderAsStringResult&& result) { - if (!result.result().has_value()) { - return {}; - } else if (!result.backingString().empty()) { - return CelValue::CreateString( - Protobuf::Arena::Create(&arena, result.backingString())); - } else { - return CelValue::CreateStringView(result.result().value()); - } -} - namespace { absl::optional extractSslInfo(const Ssl::ConnectionInfo& ssl_info, diff --git a/source/extensions/filters/common/expr/context.h b/source/extensions/filters/common/expr/context.h index 16206f66e2ea..f3a2aed0cef5 100644 --- a/source/extensions/filters/common/expr/context.h +++ b/source/extensions/filters/common/expr/context.h @@ -4,7 +4,6 @@ #include "envoy/stream_info/stream_info.h" #include "common/grpc/status.h" -#include "common/http/header_utility.h" #include "common/http/headers.h" #include "eval/public/cel_value.h" @@ -71,20 +70,16 @@ constexpr absl::string_view Upstream = "upstream"; class RequestWrapper; absl::optional convertHeaderEntry(const Http::HeaderEntry* header); -absl::optional -convertHeaderEntry(Protobuf::Arena& arena, - Http::HeaderUtility::GetAllOfHeaderAsStringResult&& result); template class HeadersWrapper : public google::api::expr::runtime::CelMap { public: - HeadersWrapper(Protobuf::Arena& arena, const T* value) : arena_(arena), value_(value) {} + HeadersWrapper(const T* value) : value_(value) {} absl::optional operator[](CelValue key) const override { if (value_ == nullptr || !key.IsString()) { return {}; } - return convertHeaderEntry( - arena_, Http::HeaderUtility::getAllOfHeaderAsString( - *value_, Http::LowerCaseString(std::string(key.StringOrDie().value())))); + auto out = value_->get(Http::LowerCaseString(std::string(key.StringOrDie().value()))); + return convertHeaderEntry(out); } int size() const override { return value_ == nullptr ? 0 : value_->size(); } bool empty() const override { return value_ == nullptr ? true : value_->empty(); } @@ -95,7 +90,6 @@ template class HeadersWrapper : public google::api::expr::runtime::Cel private: friend class RequestWrapper; friend class ResponseWrapper; - Protobuf::Arena& arena_; const T* value_; }; @@ -112,9 +106,8 @@ class BaseWrapper : public google::api::expr::runtime::CelMap, class RequestWrapper : public BaseWrapper { public: - RequestWrapper(Protobuf::Arena& arena, const Http::RequestHeaderMap* headers, - const StreamInfo::StreamInfo& info) - : headers_(arena, headers), info_(info) {} + RequestWrapper(const Http::RequestHeaderMap* headers, const StreamInfo::StreamInfo& info) + : headers_(headers), info_(info) {} absl::optional operator[](CelValue key) const override; private: @@ -124,9 +117,9 @@ class RequestWrapper : public BaseWrapper { class ResponseWrapper : public BaseWrapper { public: - ResponseWrapper(Protobuf::Arena& arena, const Http::ResponseHeaderMap* headers, - const Http::ResponseTrailerMap* trailers, const StreamInfo::StreamInfo& info) - : headers_(arena, headers), trailers_(arena, trailers), info_(info) {} + ResponseWrapper(const Http::ResponseHeaderMap* headers, const Http::ResponseTrailerMap* trailers, + const StreamInfo::StreamInfo& info) + : headers_(headers), trailers_(trailers), info_(info) {} absl::optional operator[](CelValue key) const override; private: diff --git a/source/extensions/filters/common/expr/evaluator.cc b/source/extensions/filters/common/expr/evaluator.cc index e316594821ad..e4920fd21fda 100644 --- a/source/extensions/filters/common/expr/evaluator.cc +++ b/source/extensions/filters/common/expr/evaluator.cc @@ -11,15 +11,14 @@ namespace Filters { namespace Common { namespace Expr { -ActivationPtr createActivation(Protobuf::Arena& arena, const StreamInfo::StreamInfo& info, +ActivationPtr createActivation(const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap* request_headers, const Http::ResponseHeaderMap* response_headers, const Http::ResponseTrailerMap* response_trailers) { auto activation = std::make_unique(); - activation->InsertValueProducer(Request, - std::make_unique(arena, request_headers, info)); - activation->InsertValueProducer(Response, std::make_unique( - arena, response_headers, response_trailers, info)); + activation->InsertValueProducer(Request, std::make_unique(request_headers, info)); + activation->InsertValueProducer( + Response, std::make_unique(response_headers, response_trailers, info)); activation->InsertValueProducer(Connection, std::make_unique(info)); activation->InsertValueProducer(Upstream, std::make_unique(info)); activation->InsertValueProducer(Source, std::make_unique(info, false)); @@ -66,14 +65,13 @@ ExpressionPtr createExpression(Builder& builder, const google::api::expr::v1alph return std::move(cel_expression_status.value()); } -absl::optional evaluate(const Expression& expr, Protobuf::Arena& arena, +absl::optional evaluate(const Expression& expr, Protobuf::Arena* arena, const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap* request_headers, const Http::ResponseHeaderMap* response_headers, const Http::ResponseTrailerMap* response_trailers) { - auto activation = - createActivation(arena, info, request_headers, response_headers, response_trailers); - auto eval_status = expr.Evaluate(*activation, &arena); + auto activation = createActivation(info, request_headers, response_headers, response_trailers); + auto eval_status = expr.Evaluate(*activation, arena); if (!eval_status.ok()) { return {}; } @@ -84,7 +82,7 @@ absl::optional evaluate(const Expression& expr, Protobuf::Arena& arena bool matches(const Expression& expr, const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap& headers) { Protobuf::Arena arena; - auto eval_status = Expr::evaluate(expr, arena, info, &headers, nullptr, nullptr); + auto eval_status = Expr::evaluate(expr, &arena, info, &headers, nullptr, nullptr); if (!eval_status.has_value()) { return false; } diff --git a/source/extensions/filters/common/expr/evaluator.h b/source/extensions/filters/common/expr/evaluator.h index 37fdf63ab1bf..116239fde1a5 100644 --- a/source/extensions/filters/common/expr/evaluator.h +++ b/source/extensions/filters/common/expr/evaluator.h @@ -25,7 +25,7 @@ using ExpressionPtr = std::unique_ptr; // Creates an activation providing the common context attributes. // The activation lazily creates wrappers during an evaluation using the evaluation arena. -ActivationPtr createActivation(Protobuf::Arena& arena, const StreamInfo::StreamInfo& info, +ActivationPtr createActivation(const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap* request_headers, const Http::ResponseHeaderMap* response_headers, const Http::ResponseTrailerMap* response_trailers); @@ -41,7 +41,7 @@ ExpressionPtr createExpression(Builder& builder, const google::api::expr::v1alph // Evaluates an expression for a request. The arena is used to hold intermediate computational // results and potentially the final value. -absl::optional evaluate(const Expression& expr, Protobuf::Arena& arena, +absl::optional evaluate(const Expression& expr, Protobuf::Arena* arena, const StreamInfo::StreamInfo& info, const Http::RequestHeaderMap* request_headers, const Http::ResponseHeaderMap* response_headers, diff --git a/source/extensions/filters/http/header_to_metadata/BUILD b/source/extensions/filters/http/header_to_metadata/BUILD index c9c65ef0847c..e0232d4d8d1c 100644 --- a/source/extensions/filters/http/header_to_metadata/BUILD +++ b/source/extensions/filters/http/header_to_metadata/BUILD @@ -19,7 +19,6 @@ envoy_cc_library( deps = [ "//include/envoy/server:filter_config_interface", "//source/common/common:base64_lib", - "//source/common/http:header_utility_lib", "//source/common/http:utility_lib", "//source/extensions/filters/http:well_known_names", "@envoy_api//envoy/extensions/filters/http/header_to_metadata/v3:pkg_cc_proto", diff --git a/source/extensions/filters/http/header_to_metadata/header_to_metadata_filter.cc b/source/extensions/filters/http/header_to_metadata/header_to_metadata_filter.cc index 418cec2223bb..f9c060960eb9 100644 --- a/source/extensions/filters/http/header_to_metadata/header_to_metadata_filter.cc +++ b/source/extensions/filters/http/header_to_metadata/header_to_metadata_filter.cc @@ -5,7 +5,6 @@ #include "common/common/base64.h" #include "common/common/regex.h" #include "common/config/well_known_names.h" -#include "common/http/header_utility.h" #include "common/http/utility.h" #include "common/protobuf/protobuf.h" @@ -181,11 +180,11 @@ void HeaderToMetadataFilter::writeHeaderToMetadata(Http::HeaderMap& headers, for (const auto& rule : rules) { const auto& header = rule.header(); const auto& proto_rule = rule.rule(); - const auto header_value = Http::HeaderUtility::getAllOfHeaderAsString(headers, header); + const Http::HeaderEntry* header_entry = headers.get(header); - if (header_value.result().has_value() && proto_rule.has_on_header_present()) { + if (header_entry != nullptr && proto_rule.has_on_header_present()) { const auto& keyval = proto_rule.on_header_present(); - absl::string_view value = header_value.result().value(); + absl::string_view value = header_entry->value().getStringView(); // This is used to hold the rewritten header value, so that it can // be bound to value without going out of scope. std::string rewritten_value; @@ -212,7 +211,7 @@ void HeaderToMetadataFilter::writeHeaderToMetadata(Http::HeaderMap& headers, headers.remove(header); } } - if (!header_value.result().has_value() && proto_rule.has_on_header_missing()) { + if (header_entry == nullptr && proto_rule.has_on_header_missing()) { // Add metadata for the header missing case. const auto& keyval = proto_rule.on_header_missing(); diff --git a/source/extensions/filters/http/jwt_authn/BUILD b/source/extensions/filters/http/jwt_authn/BUILD index 6c7aa80277e8..a2967b990132 100644 --- a/source/extensions/filters/http/jwt_authn/BUILD +++ b/source/extensions/filters/http/jwt_authn/BUILD @@ -14,7 +14,6 @@ envoy_cc_library( srcs = ["extractor.cc"], hdrs = ["extractor.h"], deps = [ - "//source/common/http:header_utility_lib", "//source/common/http:utility_lib", "@envoy_api//envoy/extensions/filters/http/jwt_authn/v3:pkg_cc_proto", ], diff --git a/source/extensions/filters/http/jwt_authn/extractor.cc b/source/extensions/filters/http/jwt_authn/extractor.cc index 1f7080505004..6e02093c0749 100644 --- a/source/extensions/filters/http/jwt_authn/extractor.cc +++ b/source/extensions/filters/http/jwt_authn/extractor.cc @@ -5,7 +5,6 @@ #include "envoy/extensions/filters/http/jwt_authn/v3/config.pb.h" #include "common/common/utility.h" -#include "common/http/header_utility.h" #include "common/http/headers.h" #include "common/http/utility.h" #include "common/singleton/const_singleton.h" @@ -187,10 +186,9 @@ ExtractorImpl::extract(const Http::RequestHeaderMap& headers) const { for (const auto& location_it : header_locations_) { const auto& location_spec = location_it.second; ENVOY_LOG(debug, "extract {}", location_it.first); - const auto result = - Http::HeaderUtility::getAllOfHeaderAsString(headers, location_spec->header_); - if (result.result().has_value()) { - auto value_str = result.result().value(); + const Http::HeaderEntry* entry = headers.get(location_spec->header_); + if (entry) { + auto value_str = entry->value().getStringView(); if (!location_spec->value_prefix_.empty()) { const auto pos = value_str.find(location_spec->value_prefix_); if (pos == absl::string_view::npos) { diff --git a/source/extensions/filters/http/lua/BUILD b/source/extensions/filters/http/lua/BUILD index e7e621381b07..657e3472a88f 100644 --- a/source/extensions/filters/http/lua/BUILD +++ b/source/extensions/filters/http/lua/BUILD @@ -42,7 +42,6 @@ envoy_cc_library( "//include/envoy/http:header_map_interface", "//include/envoy/stream_info:stream_info_interface", "//source/common/crypto:utility_lib", - "//source/common/http:header_utility_lib", "//source/common/http:utility_lib", "//source/extensions/common/crypto:utility_lib", "//source/extensions/filters/common/lua:lua_lib", diff --git a/source/extensions/filters/http/lua/wrappers.cc b/source/extensions/filters/http/lua/wrappers.cc index 87b672092478..a772f3c1edfe 100644 --- a/source/extensions/filters/http/lua/wrappers.cc +++ b/source/extensions/filters/http/lua/wrappers.cc @@ -1,6 +1,5 @@ #include "extensions/filters/http/lua/wrappers.h" -#include "common/http/header_utility.h" #include "common/http/utility.h" #include "extensions/filters/common/lua/wrappers.h" @@ -46,10 +45,10 @@ int HeaderMapWrapper::luaAdd(lua_State* state) { int HeaderMapWrapper::luaGet(lua_State* state) { const char* key = luaL_checkstring(state, 2); - const auto value = - Http::HeaderUtility::getAllOfHeaderAsString(headers_, Http::LowerCaseString(key)); - if (value.result().has_value()) { - lua_pushlstring(state, value.result().value().data(), value.result().value().length()); + const Http::HeaderEntry* entry = headers_.get(Http::LowerCaseString(key)); + if (entry != nullptr) { + lua_pushlstring(state, entry->value().getStringView().data(), + entry->value().getStringView().length()); return 1; } else { return 0; diff --git a/test/common/http/BUILD b/test/common/http/BUILD index 617f795ed4d5..e723a48abeb0 100644 --- a/test/common/http/BUILD +++ b/test/common/http/BUILD @@ -281,7 +281,6 @@ envoy_cc_test( "//source/common/http:header_list_view_lib", "//source/common/http:header_map_lib", "//source/common/http:header_utility_lib", - "//test/test_common:test_runtime_lib", "//test/test_common:utility_lib", ], ) @@ -323,7 +322,6 @@ envoy_cc_test( srcs = ["header_utility_test.cc"], deps = [ "//source/common/http:header_utility_lib", - "//test/test_common:test_runtime_lib", "//test/test_common:utility_lib", "@envoy_api//envoy/config/route/v3:pkg_cc_proto", ], diff --git a/test/common/http/header_map_impl_test.cc b/test/common/http/header_map_impl_test.cc index 80250f46141e..6e0eac4b19dc 100644 --- a/test/common/http/header_map_impl_test.cc +++ b/test/common/http/header_map_impl_test.cc @@ -7,7 +7,6 @@ #include "common/http/header_utility.h" #include "test/test_common/printers.h" -#include "test/test_common/test_runtime.h" #include "test/test_common/utility.h" #include "gtest/gtest.h" @@ -697,11 +696,7 @@ TEST(HeaderMapImplTest, SetReferenceKey) { EXPECT_EQ("monde", headers.get(foo)->value().getStringView()); } -TEST(HeaderMapImplTest, SetCopyOldBehavior) { - TestScopedRuntime runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.http_set_copy_replace_all_headers", "false"}}); - +TEST(HeaderMapImplTest, SetCopy) { TestRequestHeaderMapImpl headers; LowerCaseString foo("hello"); headers.setCopy(foo, "world"); @@ -753,57 +748,6 @@ TEST(HeaderMapImplTest, SetCopyOldBehavior) { EXPECT_EQ(headers.getPathValue(), "/foo"); } -TEST(HeaderMapImplTest, SetCopyNewBehavior) { - TestRequestHeaderMapImpl headers; - LowerCaseString foo("hello"); - headers.setCopy(foo, "world"); - EXPECT_EQ("world", headers.get(foo)->value().getStringView()); - - // Overwrite value. - headers.setCopy(foo, "monde"); - EXPECT_EQ("monde", headers.get(foo)->value().getStringView()); - - // Add another foo header. - headers.addCopy(foo, "monde2"); - EXPECT_EQ(headers.size(), 2); - - // The foo header is overridden. - headers.setCopy(foo, "override-monde"); - EXPECT_EQ(headers.size(), 1); - - using MockCb = testing::MockFunction; - MockCb cb; - - InSequence seq; - EXPECT_CALL(cb, Call("hello", "override-monde")); - headers.iterate( - [](const Http::HeaderEntry& header, void* cb_v) -> HeaderMap::Iterate { - static_cast(cb_v)->Call(std::string(header.key().getStringView()), - std::string(header.value().getStringView())); - return HeaderMap::Iterate::Continue; - }, - &cb); - - // Test setting an empty string and then overriding. - EXPECT_EQ(1UL, headers.remove(foo)); - EXPECT_EQ(headers.size(), 0); - const std::string empty; - headers.setCopy(foo, empty); - EXPECT_EQ(headers.size(), 1); - headers.setCopy(foo, "not-empty"); - EXPECT_EQ(headers.get(foo)->value().getStringView(), "not-empty"); - - // Use setCopy with inline headers both indirectly and directly. - headers.clear(); - EXPECT_EQ(headers.size(), 0); - headers.setCopy(Headers::get().Path, "/"); - EXPECT_EQ(headers.size(), 1); - EXPECT_EQ(headers.getPathValue(), "/"); - headers.setPath("/foo"); - EXPECT_EQ(headers.size(), 1); - EXPECT_EQ(headers.getPathValue(), "/foo"); -} - TEST(HeaderMapImplTest, AddCopy) { TestRequestHeaderMapImpl headers; diff --git a/test/common/http/header_utility_test.cc b/test/common/http/header_utility_test.cc index 2e3404fc4e5b..e28ec4fd17b9 100644 --- a/test/common/http/header_utility_test.cc +++ b/test/common/http/header_utility_test.cc @@ -8,7 +8,6 @@ #include "common/http/header_utility.h" #include "common/json/json_loader.h" -#include "test/test_common/test_runtime.h" #include "test/test_common/utility.h" #include "gtest/gtest.h" @@ -72,53 +71,6 @@ TEST_F(HeaderUtilityTest, RemovePortsFromHostConnect) { } } -TEST(GetAllOfHeaderAsStringTest, All) { - const LowerCaseString test_header("test"); - { - TestRequestHeaderMapImpl headers; - const auto ret = HeaderUtility::getAllOfHeaderAsString(headers, test_header); - EXPECT_FALSE(ret.result().has_value()); - EXPECT_TRUE(ret.backingString().empty()); - } - { - TestRequestHeaderMapImpl headers{{"test", "foo"}}; - const auto ret = HeaderUtility::getAllOfHeaderAsString(headers, test_header); - EXPECT_EQ("foo", ret.result().value()); - EXPECT_TRUE(ret.backingString().empty()); - } - { - TestRequestHeaderMapImpl headers{{"test", "foo"}, {"test", "bar"}}; - const auto ret = HeaderUtility::getAllOfHeaderAsString(headers, test_header); - EXPECT_EQ("foo,bar", ret.result().value()); - EXPECT_EQ("foo,bar", ret.backingString()); - } - { - TestRequestHeaderMapImpl headers{{"test", ""}, {"test", "bar"}}; - const auto ret = HeaderUtility::getAllOfHeaderAsString(headers, test_header); - EXPECT_EQ(",bar", ret.result().value()); - EXPECT_EQ(",bar", ret.backingString()); - } - { - TestRequestHeaderMapImpl headers{{"test", ""}, {"test", ""}}; - const auto ret = HeaderUtility::getAllOfHeaderAsString(headers, test_header); - EXPECT_EQ(",", ret.result().value()); - EXPECT_EQ(",", ret.backingString()); - } - { - TestRequestHeaderMapImpl headers{ - {"test", "a"}, {"test", "b"}, {"test", "c"}, {"test", ""}, {"test", ""}}; - const auto ret = HeaderUtility::getAllOfHeaderAsString(headers, test_header); - EXPECT_EQ("a,b,c,,", ret.result().value()); - EXPECT_EQ("a,b,c,,", ret.backingString()); - // Make sure copying the return value works correctly. - const auto ret2 = ret; // NOLINT(performance-unnecessary-copy-initialization) - EXPECT_EQ(ret2.result(), ret.result()); - EXPECT_EQ(ret2.backingString(), ret.backingString()); - EXPECT_EQ(ret2.result().value().data(), ret2.backingString().data()); - EXPECT_NE(ret2.result().value().data(), ret.backingString().data()); - } -} - TEST(HeaderDataConstructorTest, NoSpecifierSet) { const std::string yaml = R"EOF( name: test-header @@ -270,32 +222,8 @@ regex_match: (a|b) EXPECT_FALSE(HeaderUtility::matchHeaders(headers, header_data)); headers.addCopy("match-header", "a"); - // With a single "match-header" this regex will match. EXPECT_TRUE(HeaderUtility::matchHeaders(headers, header_data)); - headers.addCopy("match-header", "b"); - // With two "match-header" we now logically have "a,b" as the value, so the regex will not match. - EXPECT_FALSE(HeaderUtility::matchHeaders(headers, header_data)); - - header_data[0] = std::make_unique(parseHeaderMatcherFromYaml(R"EOF( -name: match-header -exact_match: a,b - )EOF")); - // Make sure that an exact match on "a,b" does in fact work. - EXPECT_TRUE(HeaderUtility::matchHeaders(headers, header_data)); - - TestScopedRuntime runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.http_match_on_all_headers", "false"}}); - // Flipping runtime to false should make "a,b" no longer match because we will match on the first - // header only. - EXPECT_FALSE(HeaderUtility::matchHeaders(headers, header_data)); - - header_data[0] = std::make_unique(parseHeaderMatcherFromYaml(R"EOF( -name: match-header -exact_match: a - )EOF")); - // With runtime off, exact match on "a" should pass. EXPECT_TRUE(HeaderUtility::matchHeaders(headers, header_data)); } diff --git a/test/extensions/common/wasm/BUILD b/test/extensions/common/wasm/BUILD index c4f126a786a2..22fd3d11ec38 100644 --- a/test/extensions/common/wasm/BUILD +++ b/test/extensions/common/wasm/BUILD @@ -15,9 +15,7 @@ envoy_cc_test( "//test/extensions/common/wasm/test_data:modules", ], # wasm (wee v8 etc) will not compile on Windows - tags = [ - "skip_on_windows", - ], + tags = ["skip_on_windows"], deps = [ "//source/extensions/common/wasm:wasm_lib", "//test/test_common:environment_lib", diff --git a/test/extensions/filters/common/expr/context_test.cc b/test/extensions/filters/common/expr/context_test.cc index 8071cd9fecf8..9ce4c6fcc756 100644 --- a/test/extensions/filters/common/expr/context_test.cc +++ b/test/extensions/filters/common/expr/context_test.cc @@ -23,8 +23,7 @@ namespace { constexpr absl::string_view Undefined = "undefined"; TEST(Context, EmptyHeadersAttributes) { - Protobuf::Arena arena; - HeadersWrapper headers(arena, nullptr); + HeadersWrapper headers(nullptr); auto header = headers[CelValue::CreateStringView(Referer)]; EXPECT_FALSE(header.has_value()); EXPECT_EQ(0, headers.size()); @@ -37,11 +36,10 @@ TEST(Context, RequestAttributes) { Http::TestRequestHeaderMapImpl header_map{ {":method", "POST"}, {":scheme", "http"}, {":path", "/meow?yes=1"}, {":authority", "kittens.com"}, {"referer", "dogs.com"}, {"user-agent", "envoy-mobile"}, - {"content-length", "10"}, {"x-request-id", "blah"}, {"double-header", "foo"}, - {"double-header", "bar"}}; - Protobuf::Arena arena; - RequestWrapper request(arena, &header_map, info); - RequestWrapper empty_request(arena, nullptr, empty_info); + {"content-length", "10"}, {"x-request-id", "blah"}, + }; + RequestWrapper request(&header_map, info); + RequestWrapper empty_request(nullptr, empty_info); EXPECT_CALL(info, bytesReceived()).WillRepeatedly(Return(10)); // "2018-04-03T23:06:09.123Z". @@ -138,7 +136,7 @@ TEST(Context, RequestAttributes) { EXPECT_TRUE(value.has_value()); ASSERT_TRUE(value.value().IsInt64()); // this includes the headers size - EXPECT_EQ(170, value.value().Int64OrDie()); + EXPECT_EQ(138, value.value().Int64OrDie()); } { @@ -162,17 +160,12 @@ TEST(Context, RequestAttributes) { ASSERT_TRUE(value.value().IsMap()); auto& map = *value.value().MapOrDie(); EXPECT_FALSE(map.empty()); - EXPECT_EQ(10, map.size()); + EXPECT_EQ(8, map.size()); auto header = map[CelValue::CreateStringView(Referer)]; EXPECT_TRUE(header.has_value()); ASSERT_TRUE(header.value().IsString()); EXPECT_EQ("dogs.com", header.value().StringOrDie().value()); - - auto header2 = map[CelValue::CreateStringView("double-header")]; - EXPECT_TRUE(header2.has_value()); - ASSERT_TRUE(header2.value().IsString()); - EXPECT_EQ("foo,bar", header2.value().StringOrDie().value()); } { @@ -207,8 +200,7 @@ TEST(Context, RequestFallbackAttributes) { {":scheme", "http"}, {":path", "/meow?yes=1"}, }; - Protobuf::Arena arena; - RequestWrapper request(arena, &header_map, info); + RequestWrapper request(&header_map, info); EXPECT_CALL(info, bytesReceived()).WillRepeatedly(Return(10)); @@ -235,9 +227,8 @@ TEST(Context, ResponseAttributes) { const std::string grpc_status = "grpc-status"; Http::TestResponseHeaderMapImpl header_map{{header_name, "a"}}; Http::TestResponseTrailerMapImpl trailer_map{{trailer_name, "b"}, {grpc_status, "8"}}; - Protobuf::Arena arena; - ResponseWrapper response(arena, &header_map, &trailer_map, info); - ResponseWrapper empty_response(arena, nullptr, nullptr, empty_info); + ResponseWrapper response(&header_map, &trailer_map, info); + ResponseWrapper empty_response(nullptr, nullptr, empty_info); EXPECT_CALL(info, responseCode()).WillRepeatedly(Return(404)); EXPECT_CALL(info, bytesSent()).WillRepeatedly(Return(123)); @@ -334,8 +325,7 @@ TEST(Context, ResponseAttributes) { { Http::TestResponseHeaderMapImpl header_map{{header_name, "a"}, {grpc_status, "7"}}; Http::TestResponseTrailerMapImpl trailer_map{{trailer_name, "b"}}; - Protobuf::Arena arena; - ResponseWrapper response_header_status(arena, &header_map, &trailer_map, info); + ResponseWrapper response_header_status(&header_map, &trailer_map, info); auto value = response_header_status[CelValue::CreateStringView(GrpcStatus)]; EXPECT_TRUE(value.has_value()); ASSERT_TRUE(value.value().IsInt64()); @@ -344,8 +334,7 @@ TEST(Context, ResponseAttributes) { { Http::TestResponseHeaderMapImpl header_map{{header_name, "a"}}; Http::TestResponseTrailerMapImpl trailer_map{{trailer_name, "b"}}; - Protobuf::Arena arena; - ResponseWrapper response_no_status(arena, &header_map, &trailer_map, info); + ResponseWrapper response_no_status(&header_map, &trailer_map, info); auto value = response_no_status[CelValue::CreateStringView(GrpcStatus)]; EXPECT_TRUE(value.has_value()); ASSERT_TRUE(value.value().IsInt64()); @@ -355,8 +344,7 @@ TEST(Context, ResponseAttributes) { NiceMock info_without_code; Http::TestResponseHeaderMapImpl header_map{{header_name, "a"}}; Http::TestResponseTrailerMapImpl trailer_map{{trailer_name, "b"}}; - Protobuf::Arena arena; - ResponseWrapper response_no_status(arena, &header_map, &trailer_map, info_without_code); + ResponseWrapper response_no_status(&header_map, &trailer_map, info_without_code); auto value = response_no_status[CelValue::CreateStringView(GrpcStatus)]; EXPECT_FALSE(value.has_value()); } diff --git a/test/extensions/filters/common/expr/evaluator_fuzz_test.cc b/test/extensions/filters/common/expr/evaluator_fuzz_test.cc index 7421df57016a..95fdc2413c8c 100644 --- a/test/extensions/filters/common/expr/evaluator_fuzz_test.cc +++ b/test/extensions/filters/common/expr/evaluator_fuzz_test.cc @@ -44,7 +44,7 @@ DEFINE_PROTO_FUZZER(const test::extensions::filters::common::expr::EvaluatorTest // Evaluate the CEL expression. Protobuf::Arena arena; - Expr::evaluate(*expr, arena, *stream_info, &request_headers, &response_headers, + Expr::evaluate(*expr, &arena, *stream_info, &request_headers, &response_headers, &response_trailers); } catch (const CelException& e) { ENVOY_LOG_MISC(debug, "CelException: {}", e.what()); diff --git a/test/extensions/filters/http/header_to_metadata/header_to_metadata_filter_test.cc b/test/extensions/filters/http/header_to_metadata/header_to_metadata_filter_test.cc index da8340e8d27a..cf09a67ae718 100644 --- a/test/extensions/filters/http/header_to_metadata/header_to_metadata_filter_test.cc +++ b/test/extensions/filters/http/header_to_metadata/header_to_metadata_filter_test.cc @@ -109,24 +109,6 @@ TEST_F(HeaderToMetadataTest, BasicRequestTest) { filter_->onDestroy(); } -// Verify concatenation works. -TEST_F(HeaderToMetadataTest, BasicRequestDoubleHeadersTest) { - initializeFilter(request_config_yaml); - Http::TestRequestHeaderMapImpl incoming_headers{{"X-VERSION", "foo"}, {"X-VERSION", "bar"}}; - std::map expected = {{"version", "foo,bar"}}; - - EXPECT_CALL(decoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(req_info_)); - EXPECT_CALL(req_info_, setDynamicMetadata("envoy.lb", MapEq(expected))); - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(incoming_headers, false)); - Http::MetadataMap metadata_map{{"metadata", "metadata"}}; - EXPECT_EQ(Http::FilterMetadataStatus::Continue, filter_->decodeMetadata(metadata_map)); - Buffer::OwnedImpl data("data"); - EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data, false)); - Http::TestRequestTrailerMapImpl incoming_trailers; - EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(incoming_trailers)); - filter_->onDestroy(); -} - TEST_F(HeaderToMetadataTest, PerRouteOverride) { // Global config is empty. initializeFilter("{}"); diff --git a/test/extensions/filters/http/jwt_authn/extractor_test.cc b/test/extensions/filters/http/jwt_authn/extractor_test.cc index a32d819fbeae..d91f2c7dfee1 100644 --- a/test/extensions/filters/http/jwt_authn/extractor_test.cc +++ b/test/extensions/filters/http/jwt_authn/extractor_test.cc @@ -170,14 +170,6 @@ TEST_F(ExtractorTest, TestCustomHeaderToken) { EXPECT_FALSE(headers.get(Http::LowerCaseString("token-header"))); } -// Make sure a double custom header concatenates the token -TEST_F(ExtractorTest, TestDoubleCustomHeaderToken) { - auto headers = TestRequestHeaderMapImpl{{"token-header", "jwt_token"}, {"token-header", "foo"}}; - auto tokens = extractor_->extract(headers); - EXPECT_EQ(tokens.size(), 1); - EXPECT_EQ(tokens[0]->token(), "jwt_token,foo"); -} - // Test extracting token from the custom header: "prefix-header" // value prefix doesn't match. It has to be either "AAA" or "AAABBB". TEST_F(ExtractorTest, TestPrefixHeaderNotMatch) { diff --git a/test/extensions/filters/http/lua/wrappers_test.cc b/test/extensions/filters/http/lua/wrappers_test.cc index 145d6f80e66e..bc4c3200d520 100644 --- a/test/extensions/filters/http/lua/wrappers_test.cc +++ b/test/extensions/filters/http/lua/wrappers_test.cc @@ -44,10 +44,6 @@ TEST_F(LuaHeaderMapWrapperTest, Methods) { for key, value in pairs(object) do testPrint(string.format("'%s' '%s'", key, value)) end - - object:add("header3", "foo") - object:add("header3", "bar") - testPrint(object:get("header3")) end )EOF"}; @@ -62,7 +58,6 @@ TEST_F(LuaHeaderMapWrapperTest, Methods) { EXPECT_CALL(*this, testPrint("'header2' 'foo'")); EXPECT_CALL(*this, testPrint("'hello' 'WORLD'")); EXPECT_CALL(*this, testPrint("'header2' 'foo'")); - EXPECT_CALL(*this, testPrint("foo,bar")); start("callMe"); } diff --git a/test/extensions/filters/http/rbac/rbac_filter_integration_test.cc b/test/extensions/filters/http/rbac/rbac_filter_integration_test.cc index 8a9c02e92c39..28e4420db014 100644 --- a/test/extensions/filters/http/rbac/rbac_filter_integration_test.cc +++ b/test/extensions/filters/http/rbac/rbac_filter_integration_test.cc @@ -64,33 +64,6 @@ name: rbac - any: true )EOF"; -const std::string RBAC_CONFIG_HEADER_MATCH_CONDITION = R"EOF( -name: rbac -typed_config: - "@type": type.googleapis.com/envoy.extensions.filters.http.rbac.v3.RBAC - rules: - policies: - foo: - permissions: - - any: true - principals: - - any: true - condition: - call_expr: - function: _==_ - args: - - select_expr: - operand: - select_expr: - operand: - ident_expr: - name: request - field: headers - field: xxx - - const_expr: - string_value: {} -)EOF"; - using RBACIntegrationTest = HttpProtocolIntegrationTest; INSTANTIATE_TEST_SUITE_P(Protocols, RBACIntegrationTest, @@ -304,78 +277,5 @@ TEST_P(RBACIntegrationTest, PathIgnoreCase) { } } -// Basic CEL match on a header value. -TEST_P(RBACIntegrationTest, HeaderMatchCondition) { - config_helper_.addFilter(fmt::format(RBAC_CONFIG_HEADER_MATCH_CONDITION, "yyy")); - initialize(); - - codec_client_ = makeHttpConnection(lookupPort("http")); - - auto response = codec_client_->makeRequestWithBody( - Http::TestRequestHeaderMapImpl{ - {":method", "POST"}, - {":path", "/path"}, - {":scheme", "http"}, - {":authority", "host"}, - {"xxx", "yyy"}, - }, - 1024); - waitForNextUpstreamRequest(); - upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); - - response->waitForEndStream(); - ASSERT_TRUE(response->complete()); - EXPECT_EQ("200", response->headers().getStatusValue()); -} - -// CEL match on a header value in which the header is a duplicate. Verifies we handle string -// copying correctly inside the CEL expression. -TEST_P(RBACIntegrationTest, HeaderMatchConditionDuplicateHeaderNoMatch) { - config_helper_.addFilter(fmt::format(RBAC_CONFIG_HEADER_MATCH_CONDITION, "yyy")); - initialize(); - - codec_client_ = makeHttpConnection(lookupPort("http")); - - auto response = codec_client_->makeRequestWithBody( - Http::TestRequestHeaderMapImpl{ - {":method", "POST"}, - {":path", "/path"}, - {":scheme", "http"}, - {":authority", "host"}, - {"xxx", "yyy"}, - {"xxx", "zzz"}, - }, - 1024); - response->waitForEndStream(); - ASSERT_TRUE(response->complete()); - EXPECT_EQ("403", response->headers().getStatusValue()); -} - -// CEL match on a header value in which the header is a duplicate. Verifies we handle string -// copying correctly inside the CEL expression. -TEST_P(RBACIntegrationTest, HeaderMatchConditionDuplicateHeaderMatch) { - config_helper_.addFilter(fmt::format(RBAC_CONFIG_HEADER_MATCH_CONDITION, "yyy,zzz")); - initialize(); - - codec_client_ = makeHttpConnection(lookupPort("http")); - - auto response = codec_client_->makeRequestWithBody( - Http::TestRequestHeaderMapImpl{ - {":method", "POST"}, - {":path", "/path"}, - {":scheme", "http"}, - {":authority", "host"}, - {"xxx", "yyy"}, - {"xxx", "zzz"}, - }, - 1024); - waitForNextUpstreamRequest(); - upstream_request_->encodeHeaders(Http::TestResponseHeaderMapImpl{{":status", "200"}}, true); - - response->waitForEndStream(); - ASSERT_TRUE(response->complete()); - EXPECT_EQ("200", response->headers().getStatusValue()); -} - } // namespace } // namespace Envoy diff --git a/test/per_file_coverage.sh b/test/per_file_coverage.sh index fc1c13fa6918..cb7f14b81b51 100755 --- a/test/per_file_coverage.sh +++ b/test/per_file_coverage.sh @@ -10,7 +10,7 @@ declare -a KNOWN_LOW_COVERAGE=( "source/extensions/common/wasm/null:77.8" "source/extensions/common/wasm/v8:85.4" "source/extensions/filters/common:94.6" -"source/extensions/filters/common/expr:91.8" +"source/extensions/filters/common/expr:92.2" "source/extensions/filters/common/fault:95.8" "source/extensions/filters/common/lua:95.9" "source/extensions/filters/common/rbac:87.2" diff --git a/test/test_common/utility.h b/test/test_common/utility.h index a3af42ffa284..e95302726494 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -907,9 +907,6 @@ template class TestHeaderMapImplBase : public Inte const HeaderEntry* get(const LowerCaseString& key) const override { return header_map_->get(key); } - HeaderMap::GetResult getAll(const LowerCaseString& key) const override { - return header_map_->getAll(key); - } void iterate(HeaderMap::ConstIterateCb cb, void* context) const override { header_map_->iterate(cb, context); }