From 6782c8500d3f4ac86961dfd7b472895c44675814 Mon Sep 17 00:00:00 2001 From: Lizan Zhou Date: Thu, 15 Oct 2020 17:59:09 -0700 Subject: [PATCH 01/16] build: update rules_rust to allow Rustc in RBE (#13595) Signed-off-by: Lizan Zhou Signed-off-by: Piotr Sikora --- .bazelrc | 2 -- bazel/repository_locations.bzl | 8 ++++---- bazel/wasm/wasm.bzl | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/.bazelrc b/.bazelrc index 53940f9526cbf..19d36e85dcd6d 100644 --- a/.bazelrc +++ b/.bazelrc @@ -192,8 +192,6 @@ build:remote --spawn_strategy=remote,sandboxed,local build:remote --strategy=Javac=remote,sandboxed,local build:remote --strategy=Closure=remote,sandboxed,local build:remote --strategy=Genrule=remote,sandboxed,local -# rules_rust is not remote runnable (yet) -build:remote --strategy=Rustc=sandboxed,local build:remote --remote_timeout=7200 build:remote --auth_enabled=true build:remote --remote_download_toplevel diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index d95b881f41125..de598a8aa2d6d 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -904,14 +904,14 @@ DEPENDENCY_REPOSITORIES_SPEC = dict( project_name = "Bazel rust rules", project_desc = "Bazel rust rules (used by Wasm)", project_url = "https://github.com/bazelbuild/rules_rust", - version = "fda9a1ce6482973adfda022cadbfa6b300e269c3", - sha256 = "484a2b2b67cd2d1fa1054876de7f8d291c4b203fd256bc8cbea14d749bb864ce", + version = "fb90a7484800157fbb8a5904fbeb608dc1effc0c", + sha256 = "cbb253b8c5ab1a3c1787790f900e7d6774e95ba038714fc0f710935e62f30f5f", # Last commit where "out_binary = True" works. # See: https://github.com/bazelbuild/rules_rust/issues/386 strip_prefix = "rules_rust-{version}", urls = ["https://github.com/bazelbuild/rules_rust/archive/{version}.tar.gz"], - use_category = ["build"], - last_updated = "2020-07-29", + use_category = ["test_only"], + last_updated = "2020-10-15", ), rules_antlr = dict( project_name = "ANTLR Rules for Bazel", diff --git a/bazel/wasm/wasm.bzl b/bazel/wasm/wasm.bzl index 0e7a84da2e755..5a20b46837a1e 100644 --- a/bazel/wasm/wasm.bzl +++ b/bazel/wasm/wasm.bzl @@ -115,7 +115,7 @@ def envoy_wasm_cc_binary(name, tags = [], **kwargs): wasm_cc_binary(name, tags, repository = "@envoy", **kwargs) def wasm_rust_binary(name, tags = [], **kwargs): - wasm_name = "_wasm_" + (name if not ".wasm" in name else name.strip(".wasm")) + wasm_name = "_wasm_" + name.replace(".", "_") kwargs.setdefault("visibility", ["//visibility:public"]) rust_binary( From 4ddf22ec26638a4ff915ab169f34e5d81b342072 Mon Sep 17 00:00:00 2001 From: Rama Chavali Date: Thu, 15 Oct 2020 09:20:02 +0530 Subject: [PATCH 02/16] fix macos v8 build (#13572) Signed-off-by: Rama Chavali --- bazel/external/wee8.BUILD | 3 +++ bazel/external/wee8.genrule_cmd | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/bazel/external/wee8.BUILD b/bazel/external/wee8.BUILD index 2cc17dcb2374e..3a62ecd9ebf43 100644 --- a/bazel/external/wee8.BUILD +++ b/bazel/external/wee8.BUILD @@ -13,6 +13,9 @@ cc_library( "wee8/include/v8-version.h", "wee8/third_party/wasm-api/wasm.hh", ], + copts = [ + "-Wno-range-loop-analysis", + ], defines = ["ENVOY_WASM_V8"], includes = [ "wee8/include", diff --git a/bazel/external/wee8.genrule_cmd b/bazel/external/wee8.genrule_cmd index cb688bcf45f9f..d8cbd1981a642 100644 --- a/bazel/external/wee8.genrule_cmd +++ b/bazel/external/wee8.genrule_cmd @@ -19,7 +19,7 @@ pushd $$ROOT/wee8 rm -rf out/wee8 # Export compiler configuration. -export CXXFLAGS="$${CXXFLAGS-} -Wno-sign-compare -Wno-deprecated-copy -Wno-unknown-warning-option" +export CXXFLAGS="$${CXXFLAGS-} -Wno-sign-compare -Wno-deprecated-copy -Wno-unknown-warning-option -Wno-range-loop-analysis" if [[ ( `uname` == "Darwin" && $${CXX-} == "" ) || $${CXX-} == *"clang"* ]]; then export IS_CLANG=true export CC=$${CC:-clang} From 1037c3d1c8d4b4610eac1d5a2ade2eb60036b2ca Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Sat, 17 Oct 2020 05:29:32 +0900 Subject: [PATCH 03/16] wasm: update proxy-wasm-cpp-host (#13606) The PR updates proxy-wasm-cpp-host dependency for enhancing the capability and fixing a bug in WASM extensions. The change consists of three PRs in proxy-wasm-cpp-host: 1. proxy-wasm/proxy-wasm-cpp-host#68 @PiotrSikora 2. proxy-wasm/proxy-wasm-cpp-host#65 @mathetake (me) 3. proxy-wasm/proxy-wasm-cpp-host#64 @mathetake (me) The code change can be found at proxy-wasm/proxy-wasm-cpp-host@49ed20e...c5658d3 . 1 & 2 enhance WASM capability, and 3 fixes a bug in situations where users share vm_id for multiple filters. For details, please take a look at these original PRs. Signed-off-by: mathetake Signed-off-by: Piotr Sikora --- bazel/repository_locations.bzl | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index de598a8aa2d6d..1e440acab8740 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -869,9 +869,8 @@ DEPENDENCY_REPOSITORIES_SPEC = dict( project_name = "WebAssembly for Proxies (C++ host implementation)", project_desc = "WebAssembly for Proxies (C++ host implementation)", project_url = "https://github.com/proxy-wasm/proxy-wasm-cpp-host", - # 2020-09-10 - version = "49ed20e895b728aae6b811950a2939ecbaf76f7c", - sha256 = "fa03293d01450b9164f8f56ef9227301f7d1af4f373f996400f75c93f6ebc822", + version = "c5658d34979abece30882b1eeaa95b6ee965d825", + sha256 = "dc3a794424b7679c3dbcf23548e202aa01e9f9093791b95446b99e8524e03c4f", strip_prefix = "proxy-wasm-cpp-host-{version}", urls = ["https://github.com/proxy-wasm/proxy-wasm-cpp-host/archive/{version}.tar.gz"], use_category = ["dataplane_ext"], @@ -882,7 +881,7 @@ DEPENDENCY_REPOSITORIES_SPEC = dict( "envoy.filters.network.wasm", "envoy.stat_sinks.wasm", ], - last_updated = "2020-07-29", + last_updated = "2020-10-16", cpe = "N/A", ), # TODO: upgrade to the latest version (1.41 currently fails tests) From 434e6ff10ca5e417a3b1901348e077185634147a Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Sat, 17 Oct 2020 13:02:54 -0700 Subject: [PATCH 04/16] wasm: re-enable tests with precompiled modules. (#13583) Fixes #12335. Signed-off-by: Piotr Sikora --- test/extensions/common/wasm/wasm_vm_test.cc | 35 --------------------- 1 file changed, 35 deletions(-) diff --git a/test/extensions/common/wasm/wasm_vm_test.cc b/test/extensions/common/wasm/wasm_vm_test.cc index e0775dfb0866d..af7816f20d0ad 100644 --- a/test/extensions/common/wasm/wasm_vm_test.cc +++ b/test/extensions/common/wasm/wasm_vm_test.cc @@ -144,13 +144,6 @@ TEST_P(WasmVmTest, V8BadCode) { } TEST_P(WasmVmTest, V8Code) { -#ifndef NDEBUG - // Do not execute pre-compilation tests in debug mode because V8 will fail to load because the - // flags do not match. TODO: restore this test when the rust toolchain is integrated. - if (GetParam() == 1) { - return; - } -#endif auto wasm_vm = createWasmVm("envoy.wasm.runtime.v8", scope_); ASSERT_TRUE(wasm_vm != nullptr); EXPECT_TRUE(wasm_vm->runtime() == "v8"); @@ -171,13 +164,6 @@ TEST_P(WasmVmTest, V8Code) { } TEST_P(WasmVmTest, V8BadHostFunctions) { -#ifndef NDEBUG - // Do not execute pre-compilation tests in debug mode because V8 will fail to load because the - // flags do not match. TODO: restore this test when the rust toolchain is integrated. - if (GetParam() == 1) { - return; - } -#endif auto wasm_vm = createWasmVm("envoy.wasm.runtime.v8", scope_); ASSERT_TRUE(wasm_vm != nullptr); @@ -199,13 +185,6 @@ TEST_P(WasmVmTest, V8BadHostFunctions) { } TEST_P(WasmVmTest, V8BadModuleFunctions) { -#ifndef NDEBUG - // Do not execute pre-compilation tests in debug mode because V8 will fail to load because the - // flags do not match. TODO: restore this test when the rust toolchain is integrated. - if (GetParam() == 1) { - return; - } -#endif auto wasm_vm = createWasmVm("envoy.wasm.runtime.v8", scope_); ASSERT_TRUE(wasm_vm != nullptr); @@ -234,13 +213,6 @@ TEST_P(WasmVmTest, V8BadModuleFunctions) { } TEST_P(WasmVmTest, V8FunctionCalls) { -#ifndef NDEBUG - // Do not execute pre-compilation tests in debug mode because V8 will fail to load because the - // flags do not match. TODO: restore this test when the rust toolchain is integrated. - if (GetParam() == 1) { - return; - } -#endif auto wasm_vm = createWasmVm("envoy.wasm.runtime.v8", scope_); ASSERT_TRUE(wasm_vm != nullptr); @@ -279,13 +251,6 @@ TEST_P(WasmVmTest, V8FunctionCalls) { } TEST_P(WasmVmTest, V8Memory) { -#ifndef NDEBUG - // Do not execute pre-compilation tests in debug mode because V8 will fail to load because the - // flags do not match. TODO: restore this test when the rust toolchain is integrated. - if (GetParam() == 1) { - return; - } -#endif auto wasm_vm = createWasmVm("envoy.wasm.runtime.v8", scope_); ASSERT_TRUE(wasm_vm != nullptr); From 25268b35c47fcaace5409ca9c635ba51099e5c73 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Mon, 19 Oct 2020 13:14:23 -0700 Subject: [PATCH 05/16] wasm: flip the meaning of the "repository" in envoy_wasm_cc_binary(). (#13621) Change the meaning of the "repository" parameter to refer to an external Bazel repository, instead of using "@envoy" in targets that are included in the Envoy repository. This aligns with other envoy_* rules. Signed-off-by: Piotr Sikora --- bazel/wasm/wasm.bzl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bazel/wasm/wasm.bzl b/bazel/wasm/wasm.bzl index 5a20b46837a1e..a3d89067e4969 100644 --- a/bazel/wasm/wasm.bzl +++ b/bazel/wasm/wasm.bzl @@ -88,9 +88,9 @@ def wasm_cc_binary(name, tags = [], repository = "", **kwargs): kwargs.setdefault("additional_linker_inputs", ["@proxy_wasm_cpp_sdk//:jslib", "@envoy//source/extensions/common/wasm/ext:jslib"]) if repository == "@envoy": - envoy_js = "--js-library source/extensions/common/wasm/ext/envoy_wasm_intrinsics.js" - else: envoy_js = "--js-library external/envoy/source/extensions/common/wasm/ext/envoy_wasm_intrinsics.js" + else: + envoy_js = "--js-library source/extensions/common/wasm/ext/envoy_wasm_intrinsics.js" kwargs.setdefault("linkopts", [ envoy_js, "--js-library external/proxy_wasm_cpp_sdk/proxy_wasm_intrinsics.js", @@ -112,7 +112,7 @@ def wasm_cc_binary(name, tags = [], repository = "", **kwargs): ) def envoy_wasm_cc_binary(name, tags = [], **kwargs): - wasm_cc_binary(name, tags, repository = "@envoy", **kwargs) + wasm_cc_binary(name, tags, repository = "", **kwargs) def wasm_rust_binary(name, tags = [], **kwargs): wasm_name = "_wasm_" + name.replace(".", "_") From 7d0b15bb7006092eae0294befd9f58088d387557 Mon Sep 17 00:00:00 2001 From: cmluciano Date: Tue, 20 Oct 2020 17:32:08 -0400 Subject: [PATCH 06/16] build: support ppc64le with wasm (#13657) The build has only been tested with gn git sha 5da62d5 as recommended by ppc64 maintainers of the v8 runtime. Signed-off-by: Christopher M. Luciano --- bazel/external/wee8.genrule_cmd | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/bazel/external/wee8.genrule_cmd b/bazel/external/wee8.genrule_cmd index d8cbd1981a642..bb5b9b7f84906 100644 --- a/bazel/external/wee8.genrule_cmd +++ b/bazel/external/wee8.genrule_cmd @@ -2,12 +2,12 @@ set -e -# This works only on Linux-{x86_64,s390x,aarch64} and macOS-x86_64. +# This works only on Linux-{x86_64,s390x,aarch64,ppc64le} and macOS-x86_64. case "$$(uname -s)-$$(uname -m)" in -Linux-x86_64|Linux-s390x|Linux-aarch64|Darwin-x86_64) +Linux-x86_64|Linux-s390x|Linux-aarch64|Linux-ppc64le|Darwin-x86_64) ;; *) - echo "ERROR: wee8 is currently supported only on Linux-{x86_64,s390x,aarch64} and macOS-x86_64." >&2 + echo "ERROR: wee8 is currently supported only on Linux-{x86_64,s390x,aarch64,ppc64le} and macOS-x86_64." >&2 exit 1 esac @@ -88,6 +88,11 @@ WEE8_BUILD_ARGS+=" v8_enable_shared_ro_heap=false" if [[ `uname -m` == "aarch64" ]]; then WEE8_BUILD_ARGS+=" target_cpu=\"arm64\"" fi +# Support ppc64 +# Only tests with gn 5da62d5 +if [[ `uname -m` == "ppc64le" ]]; then + WEE8_BUILD_ARGS+=" target_cpu=\"ppc64\"" +fi # Build wee8. if [[ -f /etc/centos-release ]] && [[ $$(cat /etc/centos-release) =~ "CentOS Linux release 7" ]] && [[ -x "$$(command -v gn)" ]]; then From f6589dde78ad80586fb243216b48f957ce552243 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Wed, 21 Oct 2020 16:30:07 -0700 Subject: [PATCH 07/16] wasm: remove no longer needed Emscripten metadata. (#13667) Signed-off-by: Piotr Sikora --- bazel/repository_locations.bzl | 6 ++-- test/extensions/bootstrap/wasm/wasm_test.cc | 15 -------- test/extensions/common/wasm/wasm_test.cc | 38 --------------------- test/extensions/common/wasm/wasm_vm_test.cc | 1 - 4 files changed, 3 insertions(+), 57 deletions(-) diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 1e440acab8740..f8e4221d27ed3 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -869,8 +869,8 @@ DEPENDENCY_REPOSITORIES_SPEC = dict( project_name = "WebAssembly for Proxies (C++ host implementation)", project_desc = "WebAssembly for Proxies (C++ host implementation)", project_url = "https://github.com/proxy-wasm/proxy-wasm-cpp-host", - version = "c5658d34979abece30882b1eeaa95b6ee965d825", - sha256 = "dc3a794424b7679c3dbcf23548e202aa01e9f9093791b95446b99e8524e03c4f", + version = "2d4bfe9c29fc957c3c4e81e6575773801eedade1", + sha256 = "02b50d42ee715f07314f928dd959ff558a32ea6332a698ee6e21e93632c64f52", strip_prefix = "proxy-wasm-cpp-host-{version}", urls = ["https://github.com/proxy-wasm/proxy-wasm-cpp-host/archive/{version}.tar.gz"], use_category = ["dataplane_ext"], @@ -881,7 +881,7 @@ DEPENDENCY_REPOSITORIES_SPEC = dict( "envoy.filters.network.wasm", "envoy.stat_sinks.wasm", ], - last_updated = "2020-10-16", + last_updated = "2020-10-21", cpe = "N/A", ), # TODO: upgrade to the latest version (1.41 currently fails tests) diff --git a/test/extensions/bootstrap/wasm/wasm_test.cc b/test/extensions/bootstrap/wasm/wasm_test.cc index 9511a91c96a9c..2befcbd97cba7 100644 --- a/test/extensions/bootstrap/wasm/wasm_test.cc +++ b/test/extensions/bootstrap/wasm/wasm_test.cc @@ -222,21 +222,6 @@ TEST_P(WasmTest, DivByZero) { wasm_->isFailed(); } -TEST_P(WasmTest, EmscriptenVersion) { - createWasm(); - const auto code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute( - "{{ test_rundir }}/test/extensions/bootstrap/wasm/test_data/segv_cpp.wasm")); - EXPECT_FALSE(code.empty()); - EXPECT_TRUE(wasm_->initialize(code, false)); - uint32_t major = 9, minor = 9, abi_major = 9, abi_minor = 9; - EXPECT_TRUE(wasm_->getEmscriptenVersion(&major, &minor, &abi_major, &abi_minor)); - EXPECT_EQ(major, 0); - EXPECT_LE(minor, 3); - // Up to (at least) emsdk 1.39.6. - EXPECT_EQ(abi_major, 0); - EXPECT_LE(abi_minor, 20); -} - TEST_P(WasmTest, IntrinsicGlobals) { createWasm(); const auto code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute( diff --git a/test/extensions/common/wasm/wasm_test.cc b/test/extensions/common/wasm/wasm_test.cc index f17d0ca859d6d..24b05bdb6163d 100644 --- a/test/extensions/common/wasm/wasm_test.cc +++ b/test/extensions/common/wasm/wasm_test.cc @@ -348,44 +348,6 @@ TEST_P(WasmCommonTest, DivByZero) { wasm->start(plugin); } -TEST_P(WasmCommonTest, EmscriptenVersion) { - if (GetParam() != "v8") { - return; - } - Stats::IsolatedStoreImpl stats_store; - Api::ApiPtr api = Api::createApiForTest(stats_store); - Upstream::MockClusterManager cluster_manager; - Event::DispatcherPtr dispatcher(api->allocateDispatcher("wasm_test")); - auto scope = Stats::ScopeSharedPtr(stats_store.createScope("wasm.")); - NiceMock local_info; - auto name = ""; - auto root_id = ""; - auto vm_id = ""; - auto vm_configuration = ""; - auto plugin_configuration = ""; - const auto code = TestEnvironment::readFileToStringForTest(TestEnvironment::substitute( - "{{ test_rundir }}/test/extensions/common/wasm/test_data/test_cpp.wasm")); - EXPECT_FALSE(code.empty()); - auto plugin = std::make_shared( - name, root_id, vm_id, GetParam(), plugin_configuration, false, - envoy::config::core::v3::TrafficDirection::UNSPECIFIED, local_info, nullptr); - auto vm_key = proxy_wasm::makeVmKey(vm_id, vm_configuration, code); - auto wasm = std::make_unique( - absl::StrCat("envoy.wasm.runtime.", GetParam()), vm_id, vm_configuration, vm_key, scope, - cluster_manager, *dispatcher); - EXPECT_NE(wasm, nullptr); - auto context = std::make_unique(wasm.get()); - EXPECT_TRUE(wasm->initialize(code, false)); - - uint32_t major = 9, minor = 9, abi_major = 9, abi_minor = 9; - EXPECT_TRUE(wasm->getEmscriptenVersion(&major, &minor, &abi_major, &abi_minor)); - EXPECT_EQ(major, 0); - EXPECT_LE(minor, 3); - // Up to (at least) emsdk 1.39.6. - EXPECT_EQ(abi_major, 0); - EXPECT_LE(abi_minor, 20); -} - TEST_P(WasmCommonTest, IntrinsicGlobals) { Stats::IsolatedStoreImpl stats_store; Api::ApiPtr api = Api::createApiForTest(stats_store); diff --git a/test/extensions/common/wasm/wasm_vm_test.cc b/test/extensions/common/wasm/wasm_vm_test.cc index af7816f20d0ad..dd32f0a1ed2b6 100644 --- a/test/extensions/common/wasm/wasm_vm_test.cc +++ b/test/extensions/common/wasm/wasm_vm_test.cc @@ -157,7 +157,6 @@ TEST_P(WasmVmTest, V8Code) { EXPECT_TRUE(!wasm_vm->getCustomSection(wasm_vm->getPrecompiledSectionName()).empty()); } EXPECT_THAT(wasm_vm->getCustomSection("producers"), HasSubstr("rustc")); - EXPECT_TRUE(wasm_vm->getCustomSection("emscripten_metadata").empty()); EXPECT_TRUE(wasm_vm->cloneable() == Cloneable::CompiledBytecode); EXPECT_TRUE(wasm_vm->clone() != nullptr); From d6caf0b42660e27f7438200f26a85504170ffc30 Mon Sep 17 00:00:00 2001 From: asraa Date: Mon, 26 Oct 2020 21:22:44 -0400 Subject: [PATCH 08/16] fix wasm compilation (#13765) Signed-off-by: Asra Ali --- bazel/external/wee8.genrule_cmd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bazel/external/wee8.genrule_cmd b/bazel/external/wee8.genrule_cmd index bb5b9b7f84906..8a97bd82d1207 100644 --- a/bazel/external/wee8.genrule_cmd +++ b/bazel/external/wee8.genrule_cmd @@ -19,7 +19,7 @@ pushd $$ROOT/wee8 rm -rf out/wee8 # Export compiler configuration. -export CXXFLAGS="$${CXXFLAGS-} -Wno-sign-compare -Wno-deprecated-copy -Wno-unknown-warning-option -Wno-range-loop-analysis" +export CXXFLAGS="$${CXXFLAGS-} -Wno-sign-compare -Wno-deprecated-copy -Wno-unknown-warning-option -Wno-range-loop-analysis -Wno-shorten-64-to-32" if [[ ( `uname` == "Darwin" && $${CXX-} == "" ) || $${CXX-} == *"clang"* ]]; then export IS_CLANG=true export CC=$${CC:-clang} From c1816d9f7f13e582d76dad64e8646d8714d95d9e Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Thu, 29 Oct 2020 15:52:01 -0700 Subject: [PATCH 09/16] wasm: strip only Custom Sections with precompiled Wasm modules. (#13775) Signed-off-by: Piotr Sikora --- bazel/repository_locations.bzl | 6 +++--- test/tools/wee8_compile/wee8_compile.cc | 18 +++++++++++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index f8e4221d27ed3..0ef79554c0a4e 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -869,8 +869,8 @@ DEPENDENCY_REPOSITORIES_SPEC = dict( project_name = "WebAssembly for Proxies (C++ host implementation)", project_desc = "WebAssembly for Proxies (C++ host implementation)", project_url = "https://github.com/proxy-wasm/proxy-wasm-cpp-host", - version = "2d4bfe9c29fc957c3c4e81e6575773801eedade1", - sha256 = "02b50d42ee715f07314f928dd959ff558a32ea6332a698ee6e21e93632c64f52", + version = "40fd3d03842c07d65fed907a6b6ed0f89d68d531", + sha256 = "b5ae746e66b6209ea0cce86d6c21de99dacbec1da9cdadd53a9ec46bc296a3ba", strip_prefix = "proxy-wasm-cpp-host-{version}", urls = ["https://github.com/proxy-wasm/proxy-wasm-cpp-host/archive/{version}.tar.gz"], use_category = ["dataplane_ext"], @@ -881,7 +881,7 @@ DEPENDENCY_REPOSITORIES_SPEC = dict( "envoy.filters.network.wasm", "envoy.stat_sinks.wasm", ], - last_updated = "2020-10-21", + last_updated = "2020-10-27", cpe = "N/A", ), # TODO: upgrade to the latest version (1.41 currently fails tests) diff --git a/test/tools/wee8_compile/wee8_compile.cc b/test/tools/wee8_compile/wee8_compile.cc index 42cbfea08a18f..a1b2906ab281c 100644 --- a/test/tools/wee8_compile/wee8_compile.cc +++ b/test/tools/wee8_compile/wee8_compile.cc @@ -126,10 +126,22 @@ wasm::vec stripWasmModule(const wasm::vec& module) { std::cerr << "ERROR: Failed to parse corrupted Wasm module." << std::endl; return wasm::vec::invalid(); } - if (section_type != 0 /* custom section */) { - stripped.insert(stripped.end(), section_start, pos + section_len); + if (section_type == 0 /* custom section */) { + const auto section_data_start = pos; + const auto section_name_len = parseVarint(pos, end); + if (section_name_len == static_cast(-1) || pos + section_name_len > end) { + std::cerr << "ERROR: Failed to parse corrupted Wasm module." << std::endl; + return wasm::vec::invalid(); + } + auto section_name = std::string(pos, section_name_len); + if (section_name.find("precompiled_") == std::string::npos) { + stripped.insert(stripped.end(), section_start, section_data_start + section_len); + } + pos = section_data_start + section_len; + } else { + pos += section_len; + stripped.insert(stripped.end(), section_start, pos /* section end */); } - pos += section_len; } return wasm::vec::make(stripped.size(), stripped.data()); From f8a7f896344756222c8a171f3c7c7484c1c5e3e8 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Thu, 22 Oct 2020 10:56:19 -0700 Subject: [PATCH 10/16] build: don't build shared libraries for zlib and zlib-ng. (#13652) Signed-off-by: Piotr Sikora --- bazel/foreign_cc/BUILD | 4 +-- bazel/foreign_cc/zlib.patch | 55 +++++++++++++++++++++++++++++++++++-- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/bazel/foreign_cc/BUILD b/bazel/foreign_cc/BUILD index 907098af180c2..4e0071300984e 100644 --- a/bazel/foreign_cc/BUILD +++ b/bazel/foreign_cc/BUILD @@ -351,10 +351,10 @@ envoy_cmake_external( envoy_cmake_external( name = "zlib", cache_entries = { - "BUILD_SHARED_LIBS": "off", "CMAKE_CXX_COMPILER_FORCED": "on", "CMAKE_C_COMPILER_FORCED": "on", "SKIP_BUILD_EXAMPLES": "on", + "BUILD_SHARED_LIBS": "off", # The following entries are for zlib-ng. Since zlib and zlib-ng are compatible source # codes and CMake ignores unknown cache entries, it is fine to combine it into one @@ -382,7 +382,7 @@ envoy_cmake_external( "//conditions:default": "@net_zlib//:all", }), static_libraries = select({ - "//bazel:windows_x86_64": ["zlibstatic.lib"], + "//bazel:windows_x86_64": ["zlib.lib"], "//conditions:default": ["libz.a"], }), ) diff --git a/bazel/foreign_cc/zlib.patch b/bazel/foreign_cc/zlib.patch index d8a7354dc6daa..aeeeed3147fde 100644 --- a/bazel/foreign_cc/zlib.patch +++ b/bazel/foreign_cc/zlib.patch @@ -1,8 +1,59 @@ diff --git a/CMakeLists.txt b/CMakeLists.txt -index 0fe939d..2f0475a 100644 +index e108c16..2cd82ef 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt -@@ -229,21 +229,22 @@ endif() +@@ -183,10 +183,18 @@ if(MINGW) + set(ZLIB_DLL_SRCS ${CMAKE_CURRENT_BINARY_DIR}/zlib1rc.obj) + endif(MINGW) + +-add_library(zlib SHARED ${ZLIB_SRCS} ${ZLIB_ASMS} ${ZLIB_DLL_SRCS} ${ZLIB_PUBLIC_HDRS} ${ZLIB_PRIVATE_HDRS}) +-add_library(zlibstatic STATIC ${ZLIB_SRCS} ${ZLIB_ASMS} ${ZLIB_PUBLIC_HDRS} ${ZLIB_PRIVATE_HDRS}) +-set_target_properties(zlib PROPERTIES DEFINE_SYMBOL ZLIB_DLL) +-set_target_properties(zlib PROPERTIES SOVERSION 1) ++if(NOT DEFINED BUILD_SHARED_LIBS) ++ add_library(zlib SHARED ${ZLIB_SRCS} ${ZLIB_ASMS} ${ZLIB_DLL_SRCS} ${ZLIB_PUBLIC_HDRS} ${ZLIB_PRIVATE_HDRS}) ++ add_library(zlibstatic STATIC ${ZLIB_SRCS} ${ZLIB_ASMS} ${ZLIB_PUBLIC_HDRS} ${ZLIB_PRIVATE_HDRS}) ++ set_target_properties(zlib PROPERTIES DEFINE_SYMBOL ZLIB_DLL) ++ set_target_properties(zlib PROPERTIES SOVERSION 1) ++ ++ set(ZLIB_INSTALL_LIBRARIES zlib zlibstatic) ++else() ++ add_library(zlib ${ZLIB_SRCS} ${ZLIB_ASMS} ${ZLIB_PUBLIC_HDRS} ${ZLIB_PRIVATE_HDRS}) ++ ++ set(ZLIB_INSTALL_LIBRARIES zlib) ++endif() + + if(NOT CYGWIN) + # This property causes shared libraries on Linux to have the full version +@@ -196,22 +204,22 @@ if(NOT CYGWIN) + # + # This has no effect with MSVC, on that platform the version info for + # the DLL comes from the resource file win32/zlib1.rc +- set_target_properties(zlib PROPERTIES VERSION ${ZLIB_FULL_VERSION}) ++ set_target_properties(${ZLIB_INSTALL_LIBRARIES} PROPERTIES VERSION ${ZLIB_FULL_VERSION}) + endif() + + if(UNIX) + # On unix-like platforms the library is almost always called libz +- set_target_properties(zlib zlibstatic PROPERTIES OUTPUT_NAME z) ++ set_target_properties(${ZLIB_INSTALL_LIBRARIES} PROPERTIES OUTPUT_NAME z) + if(NOT APPLE) +- set_target_properties(zlib PROPERTIES LINK_FLAGS "-Wl,--version-script,\"${CMAKE_CURRENT_SOURCE_DIR}/zlib.map\"") ++ set_target_properties(${ZLIB_INSTALL_LIBRARIES} PROPERTIES LINK_FLAGS "-Wl,--version-script,\"${CMAKE_CURRENT_SOURCE_DIR}/zlib.map\"") + endif() + elseif(BUILD_SHARED_LIBS AND WIN32) + # Creates zlib1.dll when building shared library version +- set_target_properties(zlib PROPERTIES SUFFIX "1.dll") ++ set_target_properties(${ZLIB_INSTALL_LIBRARIES} PROPERTIES SUFFIX "1.dll") + endif() + + if(NOT SKIP_INSTALL_LIBRARIES AND NOT SKIP_INSTALL_ALL ) +- install(TARGETS zlib zlibstatic ++ install(TARGETS ${ZLIB_INSTALL_LIBRARIES} + RUNTIME DESTINATION "${INSTALL_BIN_DIR}" + ARCHIVE DESTINATION "${INSTALL_LIB_DIR}" + LIBRARY DESTINATION "${INSTALL_LIB_DIR}" ) +@@ -229,21 +237,22 @@ endif() #============================================================================ # Example binaries #============================================================================ From 3bcf4faf3e1c586358a7f98c4f33515eaff77a19 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Fri, 30 Oct 2020 08:31:49 -0700 Subject: [PATCH 11/16] wasm: update V8 to v8.7.220.10. (#13568) Signed-off-by: Piotr Sikora --- bazel/external/wee8.BUILD | 11 +++++++---- bazel/external/wee8.patch | 19 ++++++++++++++++--- bazel/repository_locations.bzl | 6 +++--- test/tools/wee8_compile/wee8_compile.cc | 12 ++++++++---- tools/code_format/check_format.py | 2 +- 5 files changed, 35 insertions(+), 15 deletions(-) diff --git a/bazel/external/wee8.BUILD b/bazel/external/wee8.BUILD index 3a62ecd9ebf43..ce16c32af7995 100644 --- a/bazel/external/wee8.BUILD +++ b/bazel/external/wee8.BUILD @@ -9,15 +9,18 @@ cc_library( srcs = [ "libwee8.a", ], - hdrs = [ - "wee8/include/v8-version.h", - "wee8/third_party/wasm-api/wasm.hh", - ], + hdrs = + glob([ + "wee8/include/**/*.h", + "wee8/src/**/*.h", + "wee8/third_party/wasm-api/wasm.hh", + ]), copts = [ "-Wno-range-loop-analysis", ], defines = ["ENVOY_WASM_V8"], includes = [ + "wee8", "wee8/include", "wee8/third_party", ], diff --git a/bazel/external/wee8.patch b/bazel/external/wee8.patch index cce3eecde614e..50255793070ea 100644 --- a/bazel/external/wee8.patch +++ b/bazel/external/wee8.patch @@ -1,9 +1,10 @@ # 1. Fix linking with unbundled toolchain on macOS. # 2. Increase VSZ limit to 4TiB (allows us to start up to 409 VMs). # 3. Fix MSAN linking. +# 4. Fix Wasm module deserialization (http://crbug.com/v8/11024). --- wee8/build/toolchain/gcc_toolchain.gni +++ wee8/build/toolchain/gcc_toolchain.gni -@@ -329,6 +329,8 @@ template("gcc_toolchain") { +@@ -348,6 +348,8 @@ template("gcc_toolchain") { # AIX does not support either -D (deterministic output) or response # files. command = "$ar -X64 {{arflags}} -r -c -s {{output}} {{inputs}}" @@ -12,7 +13,7 @@ } else { rspfile = "{{output}}.rsp" rspfile_content = "{{inputs}}" -@@ -507,7 +509,7 @@ template("gcc_toolchain") { +@@ -543,7 +545,7 @@ template("gcc_toolchain") { start_group_flag = "" end_group_flag = "" @@ -51,5 +52,17 @@ - is_msan && (msan_track_origins == 0 || msan_track_origins == 2) +prebuilt_instrumented_libraries_available = false - if (use_libfuzzer && is_linux) { + if (use_libfuzzer && (is_linux || is_chromeos)) { if (is_asan) { +--- wee8/src/wasm/module-compiler.cc ++++ wee8/src/wasm/module-compiler.cc +@@ -2901,6 +2901,9 @@ void CompilationStateImpl::InitializeCompilationProgressAfterDeserialization() { + RequiredBaselineTierField::encode(ExecutionTier::kTurbofan) | + RequiredTopTierField::encode(ExecutionTier::kTurbofan) | + ReachedTierField::encode(ExecutionTier::kTurbofan); ++ finished_events_.Add(CompilationEvent::kFinishedExportWrappers); ++ finished_events_.Add(CompilationEvent::kFinishedBaselineCompilation); ++ finished_events_.Add(CompilationEvent::kFinishedTopTierCompilation); + compilation_progress_.assign(module->num_declared_functions, + kProgressAfterDeserialization); + } diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index 0ef79554c0a4e..cfca022a6c437 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -689,10 +689,10 @@ DEPENDENCY_REPOSITORIES_SPEC = dict( project_name = "V8", project_desc = "Google’s open source high-performance JavaScript and WebAssembly engine, written in C++", project_url = "https://v8.dev", - version = "8.5.210.20", + version = "8.7.220.10", # This archive was created using https://storage.googleapis.com/envoyproxy-wee8/wee8-archive.sh # and contains complete checkout of V8 with all dependencies necessary to build wee8. - sha256 = "ef404643d7da6854b76b9fb9950a79a1acbd037b7a26f02c585ac379b0f7dee1", + sha256 = "f22734640e0515bc34d1ca3772513aef24374fafa44d0489d3a9a57cadec69fb", urls = ["https://storage.googleapis.com/envoyproxy-wee8/wee8-{version}.tar.gz"], use_category = ["dataplane_ext"], extensions = [ @@ -702,7 +702,7 @@ DEPENDENCY_REPOSITORIES_SPEC = dict( "envoy.filters.network.wasm", "envoy.stat_sinks.wasm", ], - last_updated = "2020-08-31", + last_updated = "2020-10-27", cpe = "cpe:2.3:a:google:v8:*", ), com_googlesource_quiche = dict( diff --git a/test/tools/wee8_compile/wee8_compile.cc b/test/tools/wee8_compile/wee8_compile.cc index a1b2906ab281c..fe257a95e1a6d 100644 --- a/test/tools/wee8_compile/wee8_compile.cc +++ b/test/tools/wee8_compile/wee8_compile.cc @@ -1,12 +1,13 @@ // NOLINT(namespace-envoy) -#include - +#include #include #include #include +#include #include +#include "src/wasm/c-api.h" #include "v8-version.h" #include "wasm-api/wasm.hh" @@ -166,8 +167,11 @@ wasm::vec serializeWasmModule(const char* path, const wasm::vec& return wasm::vec::invalid(); } - // TODO(PiotrSikora): figure out how to hook the completion callback. - sleep(3); + wasm::StoreImpl* store_impl = reinterpret_cast(store.get()); + auto isolate = store_impl->isolate(); + while (isolate->HasPendingBackgroundTasks()) { + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + } return module->serialize(); } diff --git a/tools/code_format/check_format.py b/tools/code_format/check_format.py index 5fd0fa59ded3b..54b73fb5b6388 100755 --- a/tools/code_format/check_format.py +++ b/tools/code_format/check_format.py @@ -58,7 +58,7 @@ "./test/test_common/simulated_time_system.cc", "./test/test_common/simulated_time_system.h", "./test/test_common/test_time.cc", "./test/test_common/test_time.h", "./test/test_common/utility.cc", "./test/test_common/utility.h", - "./test/integration/integration.h") + "./test/integration/integration.h", "./test/tools/wee8_compile/wee8_compile.cc") # Tests in these paths may make use of the Registry::RegisterFactory constructor or the # REGISTER_FACTORY macro. Other locations should use the InjectFactory helper class to From 4f1f244a1389dabcce34c961bb5432bd196b5b81 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Mon, 2 Nov 2020 14:05:23 -0800 Subject: [PATCH 12/16] build: exclude wee8/out from inputs (#13866) If you build without sandboxing for performance, the output files from this custom build genrule contained timestamps which caused it to rebuild every single build. Signed-off-by: Keith Smiley --- bazel/external/wee8.BUILD | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bazel/external/wee8.BUILD b/bazel/external/wee8.BUILD index ce16c32af7995..0ea13769ccd4b 100644 --- a/bazel/external/wee8.BUILD +++ b/bazel/external/wee8.BUILD @@ -29,7 +29,10 @@ cc_library( genrule( name = "build", - srcs = glob(["wee8/**"]), + srcs = glob( + ["wee8/**"], + exclude = ["wee8/out/**"], + ), outs = [ "libwee8.a", ], From 6dffa95ec2a92bf25da4523013c9278a0623c2d3 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Wed, 4 Nov 2020 13:20:33 -0800 Subject: [PATCH 13/16] tls: fix detection of the upstream connection close event. (#13858) Fixes #13856. Signed-off-by: Piotr Sikora --- docs/root/version_history/current.rst | 1 + .../transport_sockets/tls/ssl_handshaker.h | 2 +- .../transport_sockets/tls/ssl_socket.cc | 10 +- .../transport_sockets/tls/ssl_socket_test.cc | 179 ++++++++++++++++++ 4 files changed, 190 insertions(+), 2 deletions(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 23975333efd45..b60b2d239bb39 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -78,6 +78,7 @@ Bug Fixes * rocketmq_proxy: fixed an issue involving incorrect header lengths. In debug mode it causes crash and in release mode it causes underflow. * thrift_proxy: fixed crashing bug on request overflow. * udp_proxy: fixed a crash due to UDP packets being processed after listener removal. +* tls: fix detection of the upstream connection close event. Removed Config or Runtime ------------------------- diff --git a/source/extensions/transport_sockets/tls/ssl_handshaker.h b/source/extensions/transport_sockets/tls/ssl_handshaker.h index 8eaec861a8f13..50090f6f43a74 100644 --- a/source/extensions/transport_sockets/tls/ssl_handshaker.h +++ b/source/extensions/transport_sockets/tls/ssl_handshaker.h @@ -67,7 +67,7 @@ class SslHandshakerImpl : public Ssl::ConnectionInfo, public Ssl::Handshaker { // Ssl::Handshaker Network::PostIoAction doHandshake() override; - Ssl::SocketState state() { return state_; } + Ssl::SocketState state() const { return state_; } void setState(Ssl::SocketState state) { state_ = state; } SSL* ssl() const { return ssl_.get(); } Ssl::HandshakeCallbacks* handshakeCallbacks() { return handshake_callbacks_; } diff --git a/source/extensions/transport_sockets/tls/ssl_socket.cc b/source/extensions/transport_sockets/tls/ssl_socket.cc index 4854684430963..f004947630406 100644 --- a/source/extensions/transport_sockets/tls/ssl_socket.cc +++ b/source/extensions/transport_sockets/tls/ssl_socket.cc @@ -140,10 +140,18 @@ Network::IoResult SslSocket::doRead(Buffer::Instance& read_buffer) { case SSL_ERROR_WANT_READ: break; case SSL_ERROR_ZERO_RETURN: + // Graceful shutdown using close_notify TLS alert. end_stream = true; break; + case SSL_ERROR_SYSCALL: + if (result.error_.value() == 0) { + // Non-graceful shutdown by closing the underlying socket. + end_stream = true; + break; + } + FALLTHRU; case SSL_ERROR_WANT_WRITE: - // Renegotiation has started. We don't handle renegotiation so just fall through. + // Renegotiation has started. We don't handle renegotiation so just fall through. default: drainErrorQueue(); action = PostIoAction::Close; diff --git a/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index b4bdb84e57370..dec59d696ff16 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -2534,6 +2534,185 @@ TEST_P(SslSocketTest, HalfClose) { dispatcher_->run(Event::Dispatcher::RunType::Block); } +TEST_P(SslSocketTest, ShutdownWithCloseNotify) { + const std::string server_ctx_yaml = R"EOF( + common_tls_context: + tls_certificates: + certificate_chain: + filename: "{{ test_tmpdir }}/unittestcert.pem" + private_key: + filename: "{{ test_tmpdir }}/unittestkey.pem" + validation_context: + trusted_ca: + filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_certificates.pem" +)EOF"; + + envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext server_tls_context; + TestUtility::loadFromYaml(TestEnvironment::substitute(server_ctx_yaml), server_tls_context); + auto server_cfg = std::make_unique(server_tls_context, factory_context_); + ContextManagerImpl manager(time_system_); + Stats::TestUtil::TestStore server_stats_store; + ServerSslSocketFactory server_ssl_socket_factory(std::move(server_cfg), manager, + server_stats_store, std::vector{}); + + auto socket = std::make_shared( + Network::Test::getCanonicalLoopbackAddress(GetParam()), nullptr, true); + Network::MockTcpListenerCallbacks listener_callbacks; + Network::MockConnectionHandler connection_handler; + Network::ListenerPtr listener = + dispatcher_->createListener(socket, listener_callbacks, true, ENVOY_TCP_BACKLOG_SIZE); + std::shared_ptr server_read_filter(new Network::MockReadFilter()); + std::shared_ptr client_read_filter(new Network::MockReadFilter()); + + const std::string client_ctx_yaml = R"EOF( + common_tls_context: + )EOF"; + + envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; + TestUtility::loadFromYaml(TestEnvironment::substitute(client_ctx_yaml), tls_context); + auto client_cfg = std::make_unique(tls_context, factory_context_); + Stats::TestUtil::TestStore client_stats_store; + ClientSslSocketFactory client_ssl_socket_factory(std::move(client_cfg), manager, + client_stats_store); + Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( + socket->localAddress(), Network::Address::InstanceConstSharedPtr(), + client_ssl_socket_factory.createTransportSocket(nullptr), nullptr); + Network::MockConnectionCallbacks client_connection_callbacks; + client_connection->enableHalfClose(true); + client_connection->addReadFilter(client_read_filter); + client_connection->addConnectionCallbacks(client_connection_callbacks); + client_connection->connect(); + + Network::ConnectionPtr server_connection; + Network::MockConnectionCallbacks server_connection_callbacks; + EXPECT_CALL(listener_callbacks, onAccept_(_)) + .WillOnce(Invoke([&](Network::ConnectionSocketPtr& socket) -> void { + server_connection = dispatcher_->createServerConnection( + std::move(socket), server_ssl_socket_factory.createTransportSocket(nullptr), + stream_info_); + server_connection->enableHalfClose(true); + server_connection->addReadFilter(server_read_filter); + server_connection->addConnectionCallbacks(server_connection_callbacks); + })); + EXPECT_CALL(*server_read_filter, onNewConnection()); + EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { + Buffer::OwnedImpl data("hello"); + server_connection->write(data, true); + EXPECT_EQ(data.length(), 0); + })); + + EXPECT_CALL(*client_read_filter, onNewConnection()) + .WillOnce(Return(Network::FilterStatus::Continue)); + EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)); + EXPECT_CALL(*client_read_filter, onData(BufferStringEqual("hello"), true)) + .WillOnce(Invoke([&](Buffer::Instance& read_buffer, bool) -> Network::FilterStatus { + read_buffer.drain(read_buffer.length()); + client_connection->close(Network::ConnectionCloseType::NoFlush); + return Network::FilterStatus::StopIteration; + })); + EXPECT_CALL(*server_read_filter, onData(_, true)); + + EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)); + EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::RemoteClose)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { + server_connection->close(Network::ConnectionCloseType::NoFlush); + dispatcher_->exit(); + })); + + dispatcher_->run(Event::Dispatcher::RunType::Block); +} + +TEST_P(SslSocketTest, ShutdownWithoutCloseNotify) { + const std::string server_ctx_yaml = R"EOF( + common_tls_context: + tls_certificates: + certificate_chain: + filename: "{{ test_tmpdir }}/unittestcert.pem" + private_key: + filename: "{{ test_tmpdir }}/unittestkey.pem" + validation_context: + trusted_ca: + filename: "{{ test_rundir }}/test/extensions/transport_sockets/tls/test_data/ca_certificates.pem" +)EOF"; + + envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext server_tls_context; + TestUtility::loadFromYaml(TestEnvironment::substitute(server_ctx_yaml), server_tls_context); + auto server_cfg = std::make_unique(server_tls_context, factory_context_); + ContextManagerImpl manager(time_system_); + Stats::TestUtil::TestStore server_stats_store; + ServerSslSocketFactory server_ssl_socket_factory(std::move(server_cfg), manager, + server_stats_store, std::vector{}); + + auto socket = std::make_shared( + Network::Test::getCanonicalLoopbackAddress(GetParam()), nullptr, true); + Network::MockTcpListenerCallbacks listener_callbacks; + Network::MockConnectionHandler connection_handler; + Network::ListenerPtr listener = + dispatcher_->createListener(socket, listener_callbacks, true, ENVOY_TCP_BACKLOG_SIZE); + std::shared_ptr server_read_filter(new Network::MockReadFilter()); + std::shared_ptr client_read_filter(new Network::MockReadFilter()); + + const std::string client_ctx_yaml = R"EOF( + common_tls_context: + )EOF"; + + envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext tls_context; + TestUtility::loadFromYaml(TestEnvironment::substitute(client_ctx_yaml), tls_context); + auto client_cfg = std::make_unique(tls_context, factory_context_); + Stats::TestUtil::TestStore client_stats_store; + ClientSslSocketFactory client_ssl_socket_factory(std::move(client_cfg), manager, + client_stats_store); + Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( + socket->localAddress(), Network::Address::InstanceConstSharedPtr(), + client_ssl_socket_factory.createTransportSocket(nullptr), nullptr); + Network::MockConnectionCallbacks client_connection_callbacks; + client_connection->enableHalfClose(true); + client_connection->addReadFilter(client_read_filter); + client_connection->addConnectionCallbacks(client_connection_callbacks); + client_connection->connect(); + + Network::ConnectionPtr server_connection; + Network::MockConnectionCallbacks server_connection_callbacks; + EXPECT_CALL(listener_callbacks, onAccept_(_)) + .WillOnce(Invoke([&](Network::ConnectionSocketPtr& socket) -> void { + server_connection = dispatcher_->createServerConnection( + std::move(socket), server_ssl_socket_factory.createTransportSocket(nullptr), + stream_info_); + server_connection->enableHalfClose(true); + server_connection->addReadFilter(server_read_filter); + server_connection->addConnectionCallbacks(server_connection_callbacks); + })); + EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { + Buffer::OwnedImpl data("hello"); + server_connection->write(data, false); + EXPECT_EQ(data.length(), 0); + // Close without sending close_notify alert. + const SslHandshakerImpl* ssl_socket = + dynamic_cast(server_connection->ssl().get()); + EXPECT_EQ(ssl_socket->state(), Ssl::SocketState::HandshakeComplete); + SSL_set_quiet_shutdown(ssl_socket->ssl(), 1); + server_connection->close(Network::ConnectionCloseType::NoFlush); + })); + + EXPECT_CALL(*client_read_filter, onNewConnection()) + .WillOnce(Return(Network::FilterStatus::Continue)); + EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::Connected)); + EXPECT_CALL(*client_read_filter, onData(BufferStringEqual("hello"), true)) + .WillOnce(Invoke([&](Buffer::Instance& read_buffer, bool) -> Network::FilterStatus { + read_buffer.drain(read_buffer.length()); + client_connection->close(Network::ConnectionCloseType::NoFlush); + return Network::FilterStatus::StopIteration; + })); + + EXPECT_CALL(server_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)); + EXPECT_CALL(client_connection_callbacks, onEvent(Network::ConnectionEvent::LocalClose)) + .WillOnce(Invoke([&](Network::ConnectionEvent) -> void { dispatcher_->exit(); })); + + dispatcher_->run(Event::Dispatcher::RunType::Block); +} + TEST_P(SslSocketTest, ClientAuthMultipleCAs) { const std::string server_ctx_yaml = R"EOF( common_tls_context: From 3b270cb73dda61b12545accc80395a3d9768aa5e Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Wed, 11 Nov 2020 16:34:35 +0900 Subject: [PATCH 14/16] wasm: Force stop iteration after local response is sent (#13930) Resolves https://github.com/envoyproxy/envoy/issues/13857 ref: -https://github.com/proxy-wasm/proxy-wasm-rust-sdk/issues/44 -https://github.com/proxy-wasm/proxy-wasm-cpp-host/pull/88 -https://github.com/proxy-wasm/proxy-wasm-cpp-host/pull/93 Signed-off-by: mathetake Signed-off-by: Piotr Sikora --- bazel/repository_locations.bzl | 6 +++--- test/extensions/filters/http/wasm/wasm_filter_test.cc | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index cfca022a6c437..ec5a90cdfeb04 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -869,8 +869,8 @@ DEPENDENCY_REPOSITORIES_SPEC = dict( project_name = "WebAssembly for Proxies (C++ host implementation)", project_desc = "WebAssembly for Proxies (C++ host implementation)", project_url = "https://github.com/proxy-wasm/proxy-wasm-cpp-host", - version = "40fd3d03842c07d65fed907a6b6ed0f89d68d531", - sha256 = "b5ae746e66b6209ea0cce86d6c21de99dacbec1da9cdadd53a9ec46bc296a3ba", + version = "b7d3d13d75bb6b50f192252258bb9583bf723fa4", + sha256 = "ae639f94a80adbe915849bccb32346720c59a579db09918e44aefafed6cbb100", strip_prefix = "proxy-wasm-cpp-host-{version}", urls = ["https://github.com/proxy-wasm/proxy-wasm-cpp-host/archive/{version}.tar.gz"], use_category = ["dataplane_ext"], @@ -881,7 +881,7 @@ DEPENDENCY_REPOSITORIES_SPEC = dict( "envoy.filters.network.wasm", "envoy.stat_sinks.wasm", ], - last_updated = "2020-10-27", + last_updated = "2020-11-10", cpe = "N/A", ), # TODO: upgrade to the latest version (1.41 currently fails tests) diff --git a/test/extensions/filters/http/wasm/wasm_filter_test.cc b/test/extensions/filters/http/wasm/wasm_filter_test.cc index 538481f36f6c8..ea010b9ae71da 100644 --- a/test/extensions/filters/http/wasm/wasm_filter_test.cc +++ b/test/extensions/filters/http/wasm/wasm_filter_test.cc @@ -1436,7 +1436,8 @@ TEST_P(WasmHttpFilterTest, RootId2) { setupFilter("context2"); EXPECT_CALL(filter(), log_(spdlog::level::debug, Eq(absl::string_view("onRequestHeaders2 2")))); Http::TestRequestHeaderMapImpl request_headers; - EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter().decodeHeaders(request_headers, true)); + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, + filter().decodeHeaders(request_headers, true)); } } // namespace Wasm From 36a3e6f26cc81b30a377835e36e2fe1061f2bda7 Mon Sep 17 00:00:00 2001 From: Piotr Sikora Date: Fri, 13 Nov 2020 09:02:41 -0800 Subject: [PATCH 15/16] wasm: fix order of callbacks for paused requests. (#13840) Fixes proxy-wasm/proxy-wasm-rust-sdk#43. Signed-off-by: Piotr Sikora --- bazel/repository_locations.bzl | 6 +- source/extensions/common/wasm/context.cc | 6 +- test/extensions/filters/http/wasm/BUILD | 1 + .../filters/http/wasm/test_data/BUILD | 11 +++ .../http/wasm/test_data/resume_call_rust.rs | 39 ++++++++++ .../wasm/test_data/test_resume_call_cpp.cc | 53 +++++++++++++ .../filters/http/wasm/wasm_filter_test.cc | 78 +++++++++++++++---- 7 files changed, 175 insertions(+), 19 deletions(-) create mode 100644 test/extensions/filters/http/wasm/test_data/resume_call_rust.rs create mode 100644 test/extensions/filters/http/wasm/test_data/test_resume_call_cpp.cc diff --git a/bazel/repository_locations.bzl b/bazel/repository_locations.bzl index ec5a90cdfeb04..1f42b0d758de3 100644 --- a/bazel/repository_locations.bzl +++ b/bazel/repository_locations.bzl @@ -869,8 +869,8 @@ DEPENDENCY_REPOSITORIES_SPEC = dict( project_name = "WebAssembly for Proxies (C++ host implementation)", project_desc = "WebAssembly for Proxies (C++ host implementation)", project_url = "https://github.com/proxy-wasm/proxy-wasm-cpp-host", - version = "b7d3d13d75bb6b50f192252258bb9583bf723fa4", - sha256 = "ae639f94a80adbe915849bccb32346720c59a579db09918e44aefafed6cbb100", + version = "a044a3a5bec75ce57c12d9e2b0e95e2a14f9f944", + sha256 = "619e61997682931e07e92f5b64a4268715598d3aa22a41cadeeca816103d731f", strip_prefix = "proxy-wasm-cpp-host-{version}", urls = ["https://github.com/proxy-wasm/proxy-wasm-cpp-host/archive/{version}.tar.gz"], use_category = ["dataplane_ext"], @@ -881,7 +881,7 @@ DEPENDENCY_REPOSITORIES_SPEC = dict( "envoy.filters.network.wasm", "envoy.stat_sinks.wasm", ], - last_updated = "2020-11-10", + last_updated = "2020-11-12", cpe = "N/A", ), # TODO: upgrade to the latest version (1.41 currently fails tests) diff --git a/source/extensions/common/wasm/context.cc b/source/extensions/common/wasm/context.cc index 25e5ab77533e8..41506a7e30fea 100644 --- a/source/extensions/common/wasm/context.cc +++ b/source/extensions/common/wasm/context.cc @@ -1488,12 +1488,14 @@ WasmResult Context::continueStream(WasmStreamType stream_type) { switch (stream_type) { case WasmStreamType::Request: if (decoder_callbacks_) { - decoder_callbacks_->continueDecoding(); + // We are in a reentrant call, so defer. + wasm()->addAfterVmCallAction([this] { decoder_callbacks_->continueDecoding(); }); } break; case WasmStreamType::Response: if (encoder_callbacks_) { - encoder_callbacks_->continueEncoding(); + // We are in a reentrant call, so defer. + wasm()->addAfterVmCallAction([this] { encoder_callbacks_->continueEncoding(); }); } break; default: diff --git a/test/extensions/filters/http/wasm/BUILD b/test/extensions/filters/http/wasm/BUILD index c36e60799cf67..99050e23f5e60 100644 --- a/test/extensions/filters/http/wasm/BUILD +++ b/test/extensions/filters/http/wasm/BUILD @@ -24,6 +24,7 @@ envoy_extension_cc_test( "//test/extensions/filters/http/wasm/test_data:body_rust.wasm", "//test/extensions/filters/http/wasm/test_data:headers_rust.wasm", "//test/extensions/filters/http/wasm/test_data:metadata_rust.wasm", + "//test/extensions/filters/http/wasm/test_data:resume_call_rust.wasm", "//test/extensions/filters/http/wasm/test_data:shared_data_rust.wasm", "//test/extensions/filters/http/wasm/test_data:shared_queue_rust.wasm", "//test/extensions/filters/http/wasm/test_data:test_cpp.wasm", diff --git a/test/extensions/filters/http/wasm/test_data/BUILD b/test/extensions/filters/http/wasm/test_data/BUILD index 81741d0bb37cf..61e6b4699b7ec 100644 --- a/test/extensions/filters/http/wasm/test_data/BUILD +++ b/test/extensions/filters/http/wasm/test_data/BUILD @@ -45,6 +45,15 @@ wasm_rust_binary( ], ) +wasm_rust_binary( + name = "resume_call_rust.wasm", + srcs = ["resume_call_rust.rs"], + deps = [ + "//bazel/external/cargo:log", + "//bazel/external/cargo:proxy_wasm", + ], +) + wasm_rust_binary( name = "shared_data_rust.wasm", srcs = ["shared_data_rust/src/lib.rs"], @@ -72,6 +81,7 @@ envoy_cc_library( "test_cpp_null_plugin.cc", "test_grpc_call_cpp.cc", "test_grpc_stream_cpp.cc", + "test_resume_call_cpp.cc", "test_shared_data_cpp.cc", "test_shared_queue_cpp.cc", ], @@ -97,6 +107,7 @@ envoy_wasm_cc_binary( "test_cpp.cc", "test_grpc_call_cpp.cc", "test_grpc_stream_cpp.cc", + "test_resume_call_cpp.cc", "test_shared_data_cpp.cc", "test_shared_queue_cpp.cc", ], diff --git a/test/extensions/filters/http/wasm/test_data/resume_call_rust.rs b/test/extensions/filters/http/wasm/test_data/resume_call_rust.rs new file mode 100644 index 0000000000000..d9eb08b1fa33c --- /dev/null +++ b/test/extensions/filters/http/wasm/test_data/resume_call_rust.rs @@ -0,0 +1,39 @@ +use log::info; +use proxy_wasm::traits::{Context, HttpContext}; +use proxy_wasm::types::*; +use std::time::Duration; + +#[no_mangle] +pub fn _start() { + proxy_wasm::set_log_level(LogLevel::Trace); + proxy_wasm::set_http_context(|_, _| -> Box { Box::new(TestStream) }); +} + +struct TestStream; + +impl HttpContext for TestStream { + fn on_http_request_headers(&mut self, _: usize) -> Action { + self.dispatch_http_call( + "cluster", + vec![(":method", "POST"), (":path", "/"), (":authority", "foo")], + Some(b"resume"), + vec![], + Duration::from_secs(1), + ) + .unwrap(); + info!("onRequestHeaders"); + Action::Pause + } + + fn on_http_request_body(&mut self, _: usize, _: bool) -> Action { + info!("onRequestBody"); + Action::Continue + } +} + +impl Context for TestStream { + fn on_http_call_response(&mut self, _: u32, _: usize, _: usize, _: usize) { + info!("continueRequest"); + self.resume_http_request(); + } +} diff --git a/test/extensions/filters/http/wasm/test_data/test_resume_call_cpp.cc b/test/extensions/filters/http/wasm/test_data/test_resume_call_cpp.cc new file mode 100644 index 0000000000000..f557eb143859c --- /dev/null +++ b/test/extensions/filters/http/wasm/test_data/test_resume_call_cpp.cc @@ -0,0 +1,53 @@ +// NOLINT(namespace-envoy) +#include +#include +#include + +#ifndef NULL_PLUGIN +#include "proxy_wasm_intrinsics_lite.h" +#else +#include "extensions/common/wasm/ext/envoy_null_plugin.h" +#endif + +START_WASM_PLUGIN(HttpWasmTestCpp) + +class ResumeCallContext : public Context { +public: + explicit ResumeCallContext(uint32_t id, RootContext* root) : Context(id, root) {} + + FilterHeadersStatus onRequestHeaders(uint32_t, bool) override; + FilterDataStatus onRequestBody(size_t, bool) override; +}; + +class ResumeCallRootContext : public RootContext { +public: + explicit ResumeCallRootContext(uint32_t id, std::string_view root_id) + : RootContext(id, root_id) {} +}; + +static RegisterContextFactory register_ResumeCallContext(CONTEXT_FACTORY(ResumeCallContext), + ROOT_FACTORY(ResumeCallRootContext), + "resume_call"); + +FilterHeadersStatus ResumeCallContext::onRequestHeaders(uint32_t, bool) { + auto context_id = id(); + auto resume_callback = [context_id](uint32_t, size_t, uint32_t) { + getContext(context_id)->setEffectiveContext(); + logInfo("continueRequest"); + continueRequest(); + }; + if (root()->httpCall("cluster", {{":method", "POST"}, {":path", "/"}, {":authority", "foo"}}, + "resume", {}, 1000, resume_callback) != WasmResult::Ok) { + logError("unexpected failure"); + return FilterHeadersStatus::StopIteration; + } + logInfo("onRequestHeaders"); + return FilterHeadersStatus::StopIteration; +} + +FilterDataStatus ResumeCallContext::onRequestBody(size_t, bool) { + logInfo("onRequestBody"); + return FilterDataStatus::Continue; +} + +END_WASM_PLUGIN diff --git a/test/extensions/filters/http/wasm/wasm_filter_test.cc b/test/extensions/filters/http/wasm/wasm_filter_test.cc index ea010b9ae71da..521fb3e39771b 100644 --- a/test/extensions/filters/http/wasm/wasm_filter_test.cc +++ b/test/extensions/filters/http/wasm/wasm_filter_test.cc @@ -7,6 +7,7 @@ #include "test/test_common/wasm_base.h" using testing::Eq; +using testing::InSequence; using testing::Invoke; using testing::Return; using testing::ReturnRef; @@ -211,7 +212,7 @@ TEST_P(WasmHttpFilterTest, HeadersStopAndContinue) { EXPECT_CALL(filter(), log_(spdlog::level::info, Eq(absl::string_view("header path /")))); EXPECT_CALL(filter(), log_(spdlog::level::warn, Eq(absl::string_view("onDone 2")))); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}, {"server", "envoy-wasm-pause"}}; - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter().decodeHeaders(request_headers, true)); root_context_->onTick(0); filter().clearRouteCache(); @@ -612,7 +613,56 @@ TEST_P(WasmHttpFilterTest, AsyncCall) { callbacks->onSuccess(request, std::move(response_message)); return proxy_wasm::WasmResult::Ok; })); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, + filter().decodeHeaders(request_headers, false)); + + EXPECT_NE(callbacks, nullptr); +} + +TEST_P(WasmHttpFilterTest, StopAndResumeViaAsyncCall) { + setupTest("resume_call"); + setupFilter("resume_call"); + + InSequence s; + + Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; + Http::MockAsyncClientRequest request(&cluster_manager_.async_client_); + Http::AsyncClient::Callbacks* callbacks = nullptr; + EXPECT_CALL(cluster_manager_, get(Eq("cluster"))); + EXPECT_CALL(cluster_manager_, httpAsyncClientForCluster("cluster")); + EXPECT_CALL(cluster_manager_.async_client_, send_(_, _, _)) + .WillOnce( + Invoke([&](Http::RequestMessagePtr& message, Http::AsyncClient::Callbacks& cb, + const Http::AsyncClient::RequestOptions&) -> Http::AsyncClient::Request* { + EXPECT_EQ((Http::TestRequestHeaderMapImpl{{":method", "POST"}, + {":path", "/"}, + {":authority", "foo"}, + {"content-length", "6"}}), + message->headers()); + callbacks = &cb; + return &request; + })); + + EXPECT_CALL(filter(), log_(spdlog::level::info, Eq("onRequestHeaders"))) + .WillOnce(Invoke([&](uint32_t, absl::string_view) -> proxy_wasm::WasmResult { + Http::ResponseMessagePtr response_message(new Http::ResponseMessageImpl( + Http::ResponseHeaderMapPtr{new Http::TestResponseHeaderMapImpl{{":status", "200"}}})); + NiceMock span; + Http::TestResponseHeaderMapImpl response_header{{":status", "200"}}; + callbacks->onBeforeFinalizeUpstreamSpan(span, &response_header); + callbacks->onSuccess(request, std::move(response_message)); + return proxy_wasm::WasmResult::Ok; + })); + EXPECT_CALL(filter(), log_(spdlog::level::info, Eq("continueRequest"))); + + Http::MockStreamDecoderFilterCallbacks decoder_callbacks; + filter().setDecoderFilterCallbacks(decoder_callbacks); + EXPECT_CALL(decoder_callbacks, continueDecoding()).WillOnce(Invoke([&]() { + // Verify that we're not resuming processing from within Wasm callback. + EXPECT_EQ(proxy_wasm::current_context_, nullptr); + })); + + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter().decodeHeaders(request_headers, false)); EXPECT_NE(callbacks, nullptr); @@ -677,7 +727,7 @@ TEST_P(WasmHttpFilterTest, AsyncCallFailure) { } else { EXPECT_CALL(rootContext(), log_(spdlog::level::info, Eq("async_call failed"))); } - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter().decodeHeaders(request_headers, false)); EXPECT_NE(callbacks, nullptr); @@ -708,7 +758,7 @@ TEST_P(WasmHttpFilterTest, AsyncCallAfterDestroyed) { })); EXPECT_CALL(filter(), log_(spdlog::level::info, Eq("onRequestHeaders"))); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter().decodeHeaders(request_headers, false)); EXPECT_CALL(request, cancel()).WillOnce([&]() { callbacks = nullptr; }); @@ -768,7 +818,7 @@ TEST_P(WasmHttpFilterTest, GrpcCall) { })); EXPECT_CALL(rootContext(), log_(spdlog::level::debug, Eq("response"))); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter().decodeHeaders(request_headers, false)); ProtobufWkt::Value value; @@ -852,7 +902,7 @@ TEST_P(WasmHttpFilterTest, GrpcCallFailure) { })); EXPECT_CALL(rootContext(), log_(spdlog::level::debug, Eq("failure bad"))); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter().decodeHeaders(request_headers, false)); // Test some additional error paths. @@ -913,7 +963,7 @@ TEST_P(WasmHttpFilterTest, GrpcCallCancel) { return std::move(client_factory); })); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter().decodeHeaders(request_headers, false)); rootContext().onQueueReady(0); @@ -957,7 +1007,7 @@ TEST_P(WasmHttpFilterTest, GrpcCallClose) { return std::move(client_factory); })); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter().decodeHeaders(request_headers, false)); rootContext().onQueueReady(1); @@ -1002,7 +1052,7 @@ TEST_P(WasmHttpFilterTest, GrpcCallAfterDestroyed) { })); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter().decodeHeaders(request_headers, false)); EXPECT_CALL(request, cancel()).WillOnce([&]() { callbacks = nullptr; }); @@ -1066,7 +1116,7 @@ TEST_P(WasmHttpFilterTest, GrpcStream) { EXPECT_CALL(rootContext(), log_(spdlog::level::debug, Eq("response response"))); EXPECT_CALL(rootContext(), log_(spdlog::level::debug, Eq("close done"))); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter().decodeHeaders(request_headers, false)); ProtobufWkt::Value value; @@ -1097,7 +1147,7 @@ TEST_P(WasmHttpFilterTest, GrpcStreamCloseLocal) { EXPECT_CALL(rootContext(), log_(spdlog::level::debug, Eq("response close"))); EXPECT_CALL(rootContext(), log_(spdlog::level::debug, Eq("close ok"))); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter().decodeHeaders(request_headers, false)); ProtobufWkt::Value value; @@ -1127,7 +1177,7 @@ TEST_P(WasmHttpFilterTest, GrpcStreamCloseRemote) { EXPECT_CALL(rootContext(), log_(spdlog::level::debug, Eq("response response"))); EXPECT_CALL(rootContext(), log_(spdlog::level::debug, Eq("close close"))); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter().decodeHeaders(request_headers, false)); ProtobufWkt::Value value; @@ -1154,7 +1204,7 @@ TEST_P(WasmHttpFilterTest, GrpcStreamCancel) { setupGrpcStreamTest(callbacks); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter().decodeHeaders(request_headers, false)); ProtobufWkt::Value value; @@ -1182,7 +1232,7 @@ TEST_P(WasmHttpFilterTest, GrpcStreamOpenAtShutdown) { EXPECT_CALL(rootContext(), log_(spdlog::level::debug, Eq("response response"))); Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter().decodeHeaders(request_headers, false)); ProtobufWkt::Value value; From 35f5ec91a0a73e508c6113b570469b5d8c538432 Mon Sep 17 00:00:00 2001 From: Kuat Date: Thu, 19 Nov 2020 10:48:25 -0800 Subject: [PATCH 16/16] wasm: fix network leak (#13836) Signed-off-by: Kuat Yessenov --- source/extensions/common/wasm/context.cc | 6 +----- test/extensions/filters/network/wasm/wasm_filter_test.cc | 4 ---- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/source/extensions/common/wasm/context.cc b/source/extensions/common/wasm/context.cc index 41506a7e30fea..8a09daa0f243e 100644 --- a/source/extensions/common/wasm/context.cc +++ b/source/extensions/common/wasm/context.cc @@ -853,11 +853,7 @@ BufferInterface* Context::getBuffer(WasmBufferType type) { void Context::onDownstreamConnectionClose(CloseType close_type) { ContextBase::onDownstreamConnectionClose(close_type); downstream_closed_ = true; - // Call close on TCP connection, if upstream connection closed or there was a failure seen in - // this connection. - if (upstream_closed_ || getRequestStreamInfo()->hasAnyResponseFlag()) { - onCloseTCP(); - } + onCloseTCP(); } void Context::onUpstreamConnectionClose(CloseType close_type) { diff --git a/test/extensions/filters/network/wasm/wasm_filter_test.cc b/test/extensions/filters/network/wasm/wasm_filter_test.cc index 6bf1ca8151e63..35d0ce6e6eec1 100644 --- a/test/extensions/filters/network/wasm/wasm_filter_test.cc +++ b/test/extensions/filters/network/wasm/wasm_filter_test.cc @@ -171,15 +171,11 @@ TEST_P(WasmNetworkFilterTest, CloseStream) { // Create context. EXPECT_CALL(filter(), log_(spdlog::level::trace, Eq(absl::string_view("onNewConnection 2")))); EXPECT_EQ(Network::FilterStatus::Continue, filter().onNewConnection()); - EXPECT_CALL(filter(), - log_(spdlog::level::trace, Eq(absl::string_view("onDownstreamConnectionClose 2 1")))); EXPECT_CALL(filter(), log_(spdlog::level::trace, Eq(absl::string_view("onDownstreamConnectionClose 2 2")))); filter().onEvent(static_cast(9999)); // Does nothing. filter().onEvent(Network::ConnectionEvent::RemoteClose); - filter().closeStream(proxy_wasm::WasmStreamType::Downstream); - filter().closeStream(proxy_wasm::WasmStreamType::Upstream); } TEST_P(WasmNetworkFilterTest, SegvFailOpen) {