Skip to content

Commit

Permalink
nghttp2: adding a compile-out option (#34568)
Browse files Browse the repository at this point in the history
This allows compiling nghttp2 out of Envoy, and caught some lingering
uses of nghttp2 in the Envoy code base (now fixed)
This removes nghttp2 from the OSS Envoy Mobile build

Risk Level: low
Testing: Envoy CI tests
Docs Changes: n/a
Release Notes: n/a

---------

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
  • Loading branch information
alyssawilk committed Jul 9, 2024
1 parent 10ac2b6 commit e8da3a5
Show file tree
Hide file tree
Showing 12 changed files with 211 additions and 78 deletions.
5 changes: 5 additions & 0 deletions bazel/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,11 @@ selects.config_setting_group(
],
)

config_setting(
name = "disable_nghttp2",
values = {"define": "nghttp2=disabled"},
)

config_setting(
name = "disable_google_grpc",
values = {"define": "google_grpc=disabled"},
Expand Down
2 changes: 2 additions & 0 deletions bazel/envoy_build_system.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ load(
_envoy_select_envoy_mobile_listener = "envoy_select_envoy_mobile_listener",
_envoy_select_google_grpc = "envoy_select_google_grpc",
_envoy_select_hot_restart = "envoy_select_hot_restart",
_envoy_select_nghttp2 = "envoy_select_nghttp2",
_envoy_select_signal_trace = "envoy_select_signal_trace",
_envoy_select_static_extension_registration = "envoy_select_static_extension_registration",
_envoy_select_wasm_cpp_tests = "envoy_select_wasm_cpp_tests",
Expand Down Expand Up @@ -242,6 +243,7 @@ envoy_select_enable_yaml = _envoy_select_enable_yaml
envoy_select_disable_exceptions = _envoy_select_disable_exceptions
envoy_select_enable_exceptions = _envoy_select_enable_exceptions
envoy_select_hot_restart = _envoy_select_hot_restart
envoy_select_nghttp2 = _envoy_select_nghttp2
envoy_select_enable_http_datagrams = _envoy_select_enable_http_datagrams
envoy_select_signal_trace = _envoy_select_signal_trace
envoy_select_wasm_cpp_tests = _envoy_select_wasm_cpp_tests
Expand Down
3 changes: 2 additions & 1 deletion bazel/envoy_internal.bzl
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# DO NOT LOAD THIS FILE. Targets from this file should be considered private
# and not used outside of the @envoy//bazel package.
load(":envoy_select.bzl", "envoy_select_admin_html", "envoy_select_disable_exceptions", "envoy_select_disable_logging", "envoy_select_google_grpc", "envoy_select_hot_restart", "envoy_select_signal_trace", "envoy_select_static_extension_registration")
load(":envoy_select.bzl", "envoy_select_admin_html", "envoy_select_disable_exceptions", "envoy_select_disable_logging", "envoy_select_google_grpc", "envoy_select_hot_restart", "envoy_select_nghttp2", "envoy_select_signal_trace", "envoy_select_static_extension_registration")

# Compute the final copts based on various options.
def envoy_copts(repository, test = False):
Expand Down Expand Up @@ -119,6 +119,7 @@ def envoy_copts(repository, test = False):
repository + "//bazel:uhv_enabled": ["-DENVOY_ENABLE_UHV"],
"//conditions:default": [],
}) + envoy_select_hot_restart(["-DENVOY_HOT_RESTART"], repository) + \
envoy_select_nghttp2(["-DENVOY_NGHTTP2"], repository) + \
envoy_select_disable_exceptions(["-fno-exceptions"], repository) + \
envoy_select_admin_html(["-DENVOY_ADMIN_HTML"], repository) + \
envoy_select_static_extension_registration(["-DENVOY_STATIC_EXTENSION_REGISTRATION"], repository) + \
Expand Down
7 changes: 7 additions & 0 deletions bazel/envoy_select.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,13 @@ def envoy_select_hot_restart(xs, repository = ""):
"//conditions:default": xs,
})

# Selects the given values if hot restart is enabled in the current build.
def envoy_select_nghttp2(xs, repository = ""):
return select({
repository + "//bazel:disable_nghttp2": [],
"//conditions:default": xs,
})

# Selects the given values if full protos are enabled in the current build.
def envoy_select_enable_full_protos(xs, repository = ""):
return select({
Expand Down
1 change: 1 addition & 0 deletions mobile/.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ build --xcode_version=14.1
build --use_top_level_targets_for_symlinks
build --experimental_repository_downloader_retries=2
build --define=google_grpc=disabled
build --define=nghttp2=disabled
build --define=envoy_yaml=disabled
build --define=envoy_full_protos=disabled
build --define envoy_exceptions=disabled
Expand Down
8 changes: 6 additions & 2 deletions source/common/http/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ load(
"envoy_package",
"envoy_select_enable_http3",
"envoy_select_enable_http_datagrams",
"envoy_select_nghttp2",
)
load(
"//bazel:envoy_internal.bzl",
"envoy_external_dep_path",
)

licenses(["notice"]) # Apache 2
Expand Down Expand Up @@ -545,7 +550,6 @@ envoy_cc_library(
srcs = ["header_utility.cc"],
hdrs = ["header_utility.h"],
external_deps = [
"nghttp2",
"quiche_http2_adapter",
],
deps = [
Expand All @@ -568,7 +572,7 @@ envoy_cc_library(
"@envoy_api//envoy/type/v3:pkg_cc_proto",
] + envoy_select_enable_http_datagrams([
"@com_github_google_quiche//:quiche_common_structured_headers_lib",
]),
]) + envoy_select_nghttp2([envoy_external_dep_path("nghttp2")]),
)

envoy_cc_library(
Expand Down
13 changes: 9 additions & 4 deletions source/common/http/header_utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
#include "source/common/runtime/runtime_features.h"

#include "absl/strings/match.h"
#include "nghttp2/nghttp2.h"

#ifdef ENVOY_NGHTTP2
#include "nghttp2/nghttp2.h"
#endif
#ifdef ENVOY_ENABLE_HTTP_DATAGRAMS
#include "quiche/common/structured_headers.h"
#endif
Expand Down Expand Up @@ -203,12 +205,14 @@ bool HeaderUtility::headerValueIsValid(const absl::string_view header_value) {

bool HeaderUtility::headerNameIsValid(absl::string_view header_key) {
if (!header_key.empty() && header_key[0] == ':') {
#ifdef ENVOY_NGHTTP2
if (!Runtime::runtimeFeatureEnabled(
"envoy.reloadable_features.sanitize_http2_headers_without_nghttp2")) {
// For HTTP/2 pseudo header, use the HTTP/2 semantics for checking validity
return nghttp2_check_header_name(reinterpret_cast<const uint8_t*>(header_key.data()),
header_key.size()) != 0;
}
#endif
header_key.remove_prefix(1);
if (header_key.empty()) {
return false;
Expand All @@ -232,13 +236,14 @@ bool HeaderUtility::headerNameContainsUnderscore(const absl::string_view header_
}

bool HeaderUtility::authorityIsValid(const absl::string_view header_value) {
if (Runtime::runtimeFeatureEnabled(
#ifdef ENVOY_NGHTTP2
if (!Runtime::runtimeFeatureEnabled(
"envoy.reloadable_features.http2_validate_authority_with_quiche")) {
return http2::adapter::HeaderValidator::IsValidAuthority(header_value);
} else {
return nghttp2_check_authority(reinterpret_cast<const uint8_t*>(header_value.data()),
header_value.size()) != 0;
}
#endif
return http2::adapter::HeaderValidator::IsValidAuthority(header_value);
}

bool HeaderUtility::isSpecial1xx(const ResponseHeaderMap& response_headers) {
Expand Down
15 changes: 8 additions & 7 deletions source/common/http/http2/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ load(
"//bazel:envoy_build_system.bzl",
"envoy_cc_library",
"envoy_package",
"envoy_select_nghttp2",
)
load(
"//bazel:envoy_internal.bzl",
"envoy_external_dep_path",
)

licenses(["notice"]) # Apache 2
Expand All @@ -24,7 +29,6 @@ envoy_cc_library(
srcs = ["codec_impl.cc"],
hdrs = ["codec_impl.h"],
external_deps = [
"nghttp2",
"quiche_http2_adapter",
"abseil_optional",
"abseil_inlined_vector",
Expand Down Expand Up @@ -62,7 +66,7 @@ envoy_cc_library(
"//source/common/network:common_connection_filter_states_lib",
"//source/common/runtime:runtime_features_lib",
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
],
] + envoy_select_nghttp2([envoy_external_dep_path("nghttp2")]),
)

# Separate library for some nghttp2 setup stuff to avoid having tests take a
Expand Down Expand Up @@ -117,22 +121,19 @@ envoy_cc_library(
"//source/common/common:assert_lib",
"//source/common/common:minimal_logger_lib",
"//source/common/runtime:runtime_features_lib",
],
] + envoy_select_nghttp2([envoy_external_dep_path("nghttp2")]),
)

envoy_cc_library(
name = "protocol_constraints_lib",
srcs = ["protocol_constraints.cc"],
hdrs = ["protocol_constraints.h"],
external_deps = [
"nghttp2",
],
deps = [
":codec_stats_lib",
"//envoy/network:connection_interface",
"//source/common/common:assert_lib",
"//source/common/common:dump_state_utils",
"//source/common/http:status_lib",
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
],
] + envoy_select_nghttp2([envoy_external_dep_path("nghttp2")]),
)
Loading

0 comments on commit e8da3a5

Please sign in to comment.