From fa75a60839a570eec54a086d24f1735f2e1d96e1 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Fri, 7 Jan 2022 11:17:52 -0800 Subject: [PATCH 01/21] bazel: change bazelisk for M1 support (#1966) This uses a fat binary version of bazelisk to better support arm bazel Signed-off-by: Keith Smiley keithbsmiley@gmail.com --- bazelw | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bazelw b/bazelw index e9e44c76fd..bf6205aaf6 100755 --- a/bazelw +++ b/bazelw @@ -4,9 +4,8 @@ set -euo pipefail readonly bazelisk_version="1.10.1" if [[ $OSTYPE == darwin* ]]; then - # TODO: Support M1 once https://github.com/envoyproxy/envoy/issues/16482 - readonly bazel_platform="darwin-amd64" - readonly bazel_version_sha="e485bbf84532d02a60b0eb23c702610b5408df3a199087a4f2b5e0995bbf2d5a" + readonly bazel_platform="darwin" + readonly bazel_version_sha="d3014e478fe6ebbc290e6eccdcc154e93f894300b63d521ec8d22c2e24f32892" else readonly bazel_platform="linux-amd64" readonly bazel_version_sha="4cb534c52cdd47a6223d4596d530e7c9c785438ab3b0a49ff347e991c210b2cd" From 0a726e91877d701db037d48bd2ecb6d666304c65 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Fri, 7 Jan 2022 14:21:52 -0500 Subject: [PATCH 02/21] Decompress even when `no-transform` is specified (#1995) Commit Message: Decompress even when `no-transform` is specified Additional Description: This leverages the flag added in https://github.com/envoyproxy/envoy/pull/19399 to always decompress responses, regardless of the presence of a `no-transform` cache control header, matching the more lenient behavior of other client-side networking libraries such as OkHttp or URLSession, especially in cases where the remote server is not under the client developer's control. Risk Level: Low, responses that would previously fail to decode will now succeed when a compressed response comes with a `no-transform` cache control header. Testing: Added unit tests in Envoy and manually confirmed in an Android app that gzipped responses with `no-transform` that previously failed now succeed. Docs Changes: Not needed. Release Notes: Added. Platform Specific Features: This is configured to apply regardless of the platform, so both iOS & Android will get this new behavior. --- bazel/apple_test.bzl | 6 ++++-- docs/root/intro/version_history.rst | 2 ++ envoy | 2 +- library/common/config/config.cc | 3 +++ test/common/integration/BUILD | 4 ++++ test/swift/integration/BUILD | 4 ++++ 6 files changed, 18 insertions(+), 3 deletions(-) diff --git a/bazel/apple_test.bzl b/bazel/apple_test.bzl index e392604b79..393c30b1f5 100644 --- a/bazel/apple_test.bzl +++ b/bazel/apple_test.bzl @@ -18,7 +18,7 @@ load("@rules_cc//cc:defs.bzl", "objc_library") # ], # ) # -def envoy_mobile_swift_test(name, srcs, data = [], deps = [], repository = ""): +def envoy_mobile_swift_test(name, srcs, data = [], deps = [], tags = [], repository = ""): test_lib_name = name + "_lib" swift_library( name = test_lib_name, @@ -36,9 +36,10 @@ def envoy_mobile_swift_test(name, srcs, data = [], deps = [], repository = ""): data = data, deps = [test_lib_name], minimum_os_version = "11.0", + tags = tags, ) -def envoy_mobile_objc_test(name, srcs, data = [], deps = []): +def envoy_mobile_objc_test(name, srcs, data = [], deps = [], tags = []): test_lib_name = name + "_lib" objc_library( name = test_lib_name, @@ -53,4 +54,5 @@ def envoy_mobile_objc_test(name, srcs, data = [], deps = []): data = data, deps = [test_lib_name], minimum_os_version = "11.0", + tags = tags, ) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index cf87f2842f..6cebf3960f 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -6,6 +6,8 @@ Pending Release Bugfixes: +- Decompressor: decompress even when `no-transform` is specified (:issue:`#1995 <1995>`) + Features: 0.4.4 (December 30, 2021) diff --git a/envoy b/envoy index 8a75ac1047..fb06327905 160000 --- a/envoy +++ b/envoy @@ -1 +1 @@ -Subproject commit 8a75ac104746fea9efb4ce7e8155afa49d9ef78d +Subproject commit fb06327905ef28444a6abbbccabcc395de9e0cbc diff --git a/library/common/config/config.cc b/library/common/config/config.cc index 52a0cca20f..6e2ed8cedd 100644 --- a/library/common/config/config.cc +++ b/library/common/config/config.cc @@ -294,6 +294,9 @@ R"( enabled: default_value: false runtime_key: request_decompressor_enabled + response_direction_config: + common_config: + ignore_no_transform_header: true - name: envoy.router typed_config: "@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router diff --git a/test/common/integration/BUILD b/test/common/integration/BUILD index 277e316c0c..41a2180478 100644 --- a/test/common/integration/BUILD +++ b/test/common/integration/BUILD @@ -8,6 +8,10 @@ envoy_cc_test( name = "client_integration_test", srcs = ["client_integration_test.cc"], repository = "@envoy", + # TODO(jpsim): Fix remote execution for these tests + tags = [ + "no-remote-exec", + ], deps = [ "//library/common/extensions/filters/http/local_error:config", "//library/common/extensions/filters/http/local_error:filter_cc_proto", diff --git a/test/swift/integration/BUILD b/test/swift/integration/BUILD index 3269f4b454..3294c1c0ef 100644 --- a/test/swift/integration/BUILD +++ b/test/swift/integration/BUILD @@ -30,6 +30,10 @@ envoy_mobile_swift_test( "SetLoggerTest.swift", "StatFlushIntegrationTest.swift", ], + # TODO(jpsim): Fix remote execution for these tests + tags = [ + "no-remote-exec", + ], deps = [ ":test_extensions", "//library/objective-c:envoy_engine_objc_lib", From 4710ef42000ba358654ef193a5b145b122218368 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Fri, 7 Jan 2022 16:35:02 -0500 Subject: [PATCH 03/21] Revert "bazel: change bazelisk for M1 support (#1966)" (#1998) This reverts commit fa75a60839a570eec54a086d24f1735f2e1d96e1. Unfortunately still not working on M1. E.g. ``` $ ./bazelw build //library/common:envoy_main_interface_lib ERROR: While resolving toolchains for target @com_envoyproxy_protoc_gen_validate//:protoc-gen-validate: no matching toolchains found for types @io_bazel_rules_go//go:toolchain ERROR: Analysis of target '//library/common:envoy_main_interface_lib' failed; build aborted: no matching toolchains found for types @io_bazel_rules_go//go:toolchain ``` --- bazelw | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bazelw b/bazelw index bf6205aaf6..e9e44c76fd 100755 --- a/bazelw +++ b/bazelw @@ -4,8 +4,9 @@ set -euo pipefail readonly bazelisk_version="1.10.1" if [[ $OSTYPE == darwin* ]]; then - readonly bazel_platform="darwin" - readonly bazel_version_sha="d3014e478fe6ebbc290e6eccdcc154e93f894300b63d521ec8d22c2e24f32892" + # TODO: Support M1 once https://github.com/envoyproxy/envoy/issues/16482 + readonly bazel_platform="darwin-amd64" + readonly bazel_version_sha="e485bbf84532d02a60b0eb23c702610b5408df3a199087a4f2b5e0995bbf2d5a" else readonly bazel_platform="linux-amd64" readonly bazel_version_sha="4cb534c52cdd47a6223d4596d530e7c9c785438ab3b0a49ff347e991c210b2cd" From a7f96cf444177e259fc58f9703d3ed1feaafcff0 Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Thu, 13 Jan 2022 18:58:55 -0500 Subject: [PATCH 04/21] release: cutting the 0.4.5 release (#2000) * release: cutting the 0.4.5 release Signed-off-by: Alyssa Wilk --- EnvoyMobile.podspec | 2 +- VERSION | 2 +- docs/root/intro/version_history.rst | 12 ++++++++++++ envoy | 2 +- 4 files changed, 15 insertions(+), 3 deletions(-) diff --git a/EnvoyMobile.podspec b/EnvoyMobile.podspec index 7dbc311a18..ae90d69f11 100644 --- a/EnvoyMobile.podspec +++ b/EnvoyMobile.podspec @@ -1,6 +1,6 @@ Pod::Spec.new do |s| s.name = 'EnvoyMobile' - s.version = '0.4.4' + s.version = '0.4.5' s.author = 'Envoy Mobile Project Authors' s.summary = 'Multiplatform client HTTP/networking library built on the Envoy project's core networking layer' s.homepage = 'https://envoy-mobile.github.io' diff --git a/VERSION b/VERSION index 6f2743d65d..0bfccb0804 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.4.4 +0.4.5 diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 6cebf3960f..0547e8bc7b 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -6,10 +6,22 @@ Pending Release Bugfixes: +Features: + +0.4.5 (January 13, 2022) +======================== + +Based off Envoy `v1.21.0 `_ + +Bugfixes: + - Decompressor: decompress even when `no-transform` is specified (:issue:`#1995 <1995>`) Features: +- HTTP: any negotiated ALPN now passed up as `x-envoy-upstream-alpn` header (:issue: `#1965 <1965>`) + + 0.4.4 (December 30, 2021) ========================= diff --git a/envoy b/envoy index fb06327905..a9d72603c6 160000 --- a/envoy +++ b/envoy @@ -1 +1 @@ -Subproject commit fb06327905ef28444a6abbbccabcc395de9e0cbc +Subproject commit a9d72603c68da3a10a1c0d021d01c7877e6f2a30 From 6be5982852264e7678f6d12d5b9e6f9df36d2863 Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Tue, 18 Jan 2022 08:10:44 -0500 Subject: [PATCH 05/21] docs: addding release notes (#2001) Signed-off-by: Alyssa Wilk --- RELEASE.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 RELEASE.md diff --git a/RELEASE.md b/RELEASE.md new file mode 100644 index 0000000000..1a97784df7 --- /dev/null +++ b/RELEASE.md @@ -0,0 +1,27 @@ +# Release Process + +## Active development + +Active development is happening on the `main` branch, and a new versions will +be released on major development milestones and/or following upstream Envoy +releases. + +## Cutting a release + +* Update the [release notes](docs/root/intro/version_history.rst). + * Add a new `Pending Release` section with `Bugfixes:` and `Features:` section at the top of the file. + * Update the old `Pending Release` section with the release version and release date. + * Make sure all changes since the last release (``git log `git rev-list --tags --max-count=1`..HEAD``) are reflected in the current release. + * If building the quarterly release, also note the Envoy tagged release (e.g. as done in the [0.4.5](https://github.com/envoyproxy/envoy-mobile/pull/2000/files#diff-02cdc1a64b58714360c0cbdf245da06616364b110af4acd72ca364a524021eedR10) release +* If building the quarterly release, ensure the Envoy checkout points to the latest [envoy tagged release](https://github.com/envoyproxy/envoy/tags) +* Bump the version in [EnvoyMobile.podspec](EnvoyMobile.podspec) and [VERSION](VERSION) files +* Create a PR with the above changes, get it approved and merged +* Optionally, wait for CI to pass on main +* Draft a new release + [here](https://github.com/envoyproxy/envoy-mobile/releases). Copy in the release notes you just created and publish it. +* Wait for the [artifacts run](https://github.com/envoyproxy/envoy-mobile/actions/workflows/artifacts.yml) + for the tag release to finish running. Click on the workflow run to find the + generated artifacts (e.g. [this](https://github.com/envoyproxy/envoy-mobile/actions/runs/1638634901)). + Download `envoy_android_aar_sources`, `envoy_ios_cocoapods`, and `envoy_ios_framework`. +* Go back to your tagged release, edit the release, and upload the 3 files from + the previous step. From d473980fc5ecef0098a8dbc52d5e0c51407aa235 Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Tue, 18 Jan 2022 10:23:47 -0500 Subject: [PATCH 06/21] bump boringssl to Envoy's version (#1999) Signed-off-by: Alyssa Wilk --- bazel/envoy_mobile_repositories.bzl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bazel/envoy_mobile_repositories.bzl b/bazel/envoy_mobile_repositories.bzl index 1597fd8f91..bc5897e029 100644 --- a/bazel/envoy_mobile_repositories.bzl +++ b/bazel/envoy_mobile_repositories.bzl @@ -54,9 +54,9 @@ def upstream_envoy_overrides(): http_archive( name = "boringssl", patches = ["@envoy_mobile//bazel:boringssl.patch"], - sha256 = "70e9d8737e35d67f94b9e742ca59c02c36f30f1d822d5a3706511a23798d8049", - strip_prefix = "boringssl-75edea1922aefe415e0e60ac576116634b0a94f8", - urls = ["https://github.com/google/boringssl/archive/75edea1922aefe415e0e60ac576116634b0a94f8.tar.gz"], + sha256 = "579cb415458e9f3642da0a39a72f79fdfe6dc9c1713b3a823f1e276681b9703e", + strip_prefix = "boringssl-648cbaf033401b7fe7acdce02f275b06a88aab5c", + urls = ["https://github.com/google/boringssl/archive/648cbaf033401b7fe7acdce02f275b06a88aab5c.tar.gz"], ) # Envoy uses rules_python v0.1.0, which does not include tooling for packaging Python. The From 100a566b85f49aac9ded0fd6949eaac9e380845f Mon Sep 17 00:00:00 2001 From: Charles Le Borgne <80125532+carloseltuerto@users.noreply.github.com> Date: Thu, 20 Jan 2022 17:56:42 +0000 Subject: [PATCH 07/21] QuicTestServer (#1989) Signed-off-by: Charles Le Borgne Co-authored-by: Chidera Olibie --- bazel/kotlin_lib.bzl | 3 +- bazel/kotlin_test.bzl | 1 + test/common/integration/BUILD | 32 +- test/common/integration/quic_test_server.cc | 68 +++++ test/common/integration/quic_test_server.h | 52 ++++ .../integration/quic_test_server_interface.cc | 34 +++ .../integration/quic_test_server_interface.h | 30 ++ test/common/jni/BUILD | 32 ++ .../jni/quic_test_server_jni_interface.cc | 32 ++ .../envoymobile/engine/testing/BUILD | 35 +++ .../engine/testing/QuicTestServer.java | 55 ++++ .../engine/testing/QuicTestServerTest.java | 282 ++++++++++++++++++ test/java/org/chromium/net/testing/BUILD | 1 + 13 files changed, 655 insertions(+), 2 deletions(-) create mode 100644 test/common/integration/quic_test_server.cc create mode 100644 test/common/integration/quic_test_server.h create mode 100644 test/common/integration/quic_test_server_interface.cc create mode 100644 test/common/integration/quic_test_server_interface.h create mode 100644 test/common/jni/BUILD create mode 100644 test/common/jni/quic_test_server_jni_interface.cc create mode 100644 test/java/io/envoyproxy/envoymobile/engine/testing/BUILD create mode 100644 test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServer.java create mode 100644 test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServerTest.java diff --git a/bazel/kotlin_lib.bzl b/bazel/kotlin_lib.bzl index 2a3de87274..e21a360b37 100644 --- a/bazel/kotlin_lib.bzl +++ b/bazel/kotlin_lib.bzl @@ -46,12 +46,13 @@ def envoy_mobile_kt_library(name, visibility = None, srcs = [], deps = []): # name = "java_jni_lib.jnilib", # native_dep = "libjava_jni_lib.so", # ) -def envoy_mobile_so_to_jni_lib(name, native_dep): +def envoy_mobile_so_to_jni_lib(name, native_dep, testonly = False): lib_name = native_lib_name(native_dep) output = "{}.jnilib".format(lib_name) return native.genrule( name = name, + testonly = testonly, outs = [output], srcs = [native_dep], cmd = """ diff --git a/bazel/kotlin_test.bzl b/bazel/kotlin_test.bzl index edfb79c3cd..3a2f909649 100644 --- a/bazel/kotlin_test.bzl +++ b/bazel/kotlin_test.bzl @@ -74,6 +74,7 @@ def envoy_mobile_android_test(name, srcs, deps = [], native_deps = [], repositor visibility = ["//visibility:public"], data = native_deps, exports = deps, + testonly = True, ) native.android_local_test( diff --git a/test/common/integration/BUILD b/test/common/integration/BUILD index 41a2180478..5dafd395dc 100644 --- a/test/common/integration/BUILD +++ b/test/common/integration/BUILD @@ -1,4 +1,4 @@ -load("@envoy//bazel:envoy_build_system.bzl", "envoy_cc_test", "envoy_package") +load("@envoy//bazel:envoy_build_system.bzl", "envoy_cc_test", "envoy_cc_test_library", "envoy_package") licenses(["notice"]) # Apache 2 @@ -21,5 +21,35 @@ envoy_cc_test( "@envoy//test/common/http:common_lib", "@envoy//test/integration:http_integration_lib", "@envoy//test/server:utility_lib", + "@envoy//test/test_common:environment_lib", ], ) + +# interface libs for quic test server's jni implementation +envoy_cc_test_library( + name = "quic_test_server_interface_lib", + srcs = [ + "quic_test_server.cc", + "quic_test_server_interface.cc", + ], + hdrs = [ + "quic_test_server.h", + "quic_test_server_interface.h", + ], + data = [ + "@envoy//test/config/integration/certs", + ], + repository = "@envoy", + deps = [ + "@envoy//source/exe:process_wide_lib", + "@envoy//test/config:utility_lib", + "@envoy//test/integration:autonomous_upstream_lib", + "@envoy//test/mocks/server:transport_socket_factory_context_mocks", + "@envoy//test/test_common:environment_lib", + ] + select({ + "@envoy//bazel:disable_signal_trace": [], + "//conditions:default": [ + "@envoy//source/common/signal:sigaction_lib", + ], + }), +) diff --git a/test/common/integration/quic_test_server.cc b/test/common/integration/quic_test_server.cc new file mode 100644 index 0000000000..c145501ffe --- /dev/null +++ b/test/common/integration/quic_test_server.cc @@ -0,0 +1,68 @@ +#include "quic_test_server.h" + +#include "test/test_common/environment.h" + +namespace Envoy { + +Network::TransportSocketFactoryPtr QuicTestServer::createUpstreamTlsContext( + testing::NiceMock& factory_context) { + envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext tls_context; + Extensions::TransportSockets::Tls::ContextManagerImpl context_manager{time_system_}; + const std::string yaml = absl::StrFormat( + R"EOF( +common_tls_context: + alpn_protocols: h3 + tls_certificates: + - certificate_chain: + filename: ../envoy/test/config/integration/certs/upstreamcert.pem + private_key: + filename: ../envoy/test/config/integration/certs/upstreamkey.pem +)EOF"); + TestUtility::loadFromYaml(yaml, tls_context); + envoy::extensions::transport_sockets::quic::v3::QuicDownstreamTransport quic_config; + quic_config.mutable_downstream_tls_context()->MergeFrom(tls_context); + + std::vector server_names; + auto& config_factory = Config::Utility::getAndCheckFactoryByName< + Server::Configuration::DownstreamTransportSocketConfigFactory>( + "envoy.transport_sockets.quic"); + + return config_factory.createTransportSocketFactory(quic_config, factory_context, server_names); +} + +QuicTestServer::QuicTestServer() + : api_(Api::createApiForTest(stats_store_, time_system_)), + version_(Network::Address::IpVersion::v4), upstream_config_(time_system_), port_(0) { + ON_CALL(factory_context_, api()).WillByDefault(testing::ReturnRef(*api_)); + ON_CALL(factory_context_, scope()).WillByDefault(testing::ReturnRef(stats_store_)); + upstream_config_.udp_fake_upstream_ = FakeUpstreamConfig::UdpConfig(); +} + +void QuicTestServer::startQuicTestServer() { + ASSERT(!upstream_); + // pre-setup: see https://github.com/envoyproxy/envoy/blob/main/test/test_runner.cc + Logger::Context logging_state(spdlog::level::level_enum::err, + "[%Y-%m-%d %T.%e][%t][%l][%n] [%g:%#] %v", lock, false, false); + // end pre-setup + + upstream_config_.upstream_protocol_ = Http::CodecType::HTTP3; + + Network::TransportSocketFactoryPtr factory = createUpstreamTlsContext(factory_context_); + + upstream_ = std::make_unique(std::move(factory), port_, version_, + upstream_config_, false); + + // see upstream address + ENVOY_LOG_MISC(debug, "Upstream now listening on {}", upstream_->localAddress()->asString()); +} + +void QuicTestServer::shutdownQuicTestServer() { + ASSERT(upstream_); + upstream_.reset(); +} + +int QuicTestServer::getServerPort() { + ASSERT(upstream_); + return upstream_->localAddress()->ip()->port(); +} +} // namespace Envoy diff --git a/test/common/integration/quic_test_server.h b/test/common/integration/quic_test_server.h new file mode 100644 index 0000000000..261624a68b --- /dev/null +++ b/test/common/integration/quic_test_server.h @@ -0,0 +1,52 @@ +#pragma once + +#include "source/extensions/transport_sockets/tls/ssl_socket.h" + +// test_runner setups +#include "source/exe/process_wide.h" + +#include "envoy/extensions/transport_sockets/quic/v3/quic_transport.pb.h" +#include "test/mocks/server/transport_socket_factory_context.h" +#include "test/integration/autonomous_upstream.h" +#include "test/config/utility.h" + +namespace Envoy { +class QuicTestServer { +private: + testing::NiceMock factory_context_; + Stats::IsolatedStoreImpl stats_store_; + Event::GlobalTimeSystem time_system_; + Api::ApiPtr api_; + Network::Address::IpVersion version_; + std::unique_ptr upstream_; + FakeUpstreamConfig upstream_config_; + int port_; + Thread::SkipAsserts skip_asserts_; + ProcessWide process_wide; + Thread::MutexBasicLockable lock; + + Network::TransportSocketFactoryPtr createUpstreamTlsContext( + testing::NiceMock&); + +public: + QuicTestServer(); + + /** + * Starts the server. Can only have one server active per JVM. This is blocking until the port can + * start accepting requests. + */ + void startQuicTestServer(); + + /** + * Shutdowns the server. Can be restarted later. This is blocking until the server has freed all + * resources. + */ + void shutdownQuicTestServer(); + + /** + * Returns the port that got attributed. Can only be called once the server has been started. + */ + int getServerPort(); +}; + +} // namespace Envoy diff --git a/test/common/integration/quic_test_server_interface.cc b/test/common/integration/quic_test_server_interface.cc new file mode 100644 index 0000000000..4fd695f337 --- /dev/null +++ b/test/common/integration/quic_test_server_interface.cc @@ -0,0 +1,34 @@ +#include "test/common/integration/quic_test_server_interface.h" + +// NOLINT(namespace-envoy) + +static std::shared_ptr strong_quic_test_server_; +static std::weak_ptr weak_quic_test_server_; + +static std::shared_ptr quic_test_server() { + return weak_quic_test_server_.lock(); +} + +void start_server() { + strong_quic_test_server_ = std::make_shared(); + weak_quic_test_server_ = strong_quic_test_server_; + + if (auto e = quic_test_server()) { + e->startQuicTestServer(); + } +} + +void shutdown_server() { + // Reset the primary handle to the test_server, + // but retain it long enough to synchronously shutdown. + auto e = strong_quic_test_server_; + strong_quic_test_server_.reset(); + e->shutdownQuicTestServer(); +} + +int get_server_port() { + if (auto e = quic_test_server()) { + return e->getServerPort(); + } + return -1; // failure +} diff --git a/test/common/integration/quic_test_server_interface.h b/test/common/integration/quic_test_server_interface.h new file mode 100644 index 0000000000..2691caff24 --- /dev/null +++ b/test/common/integration/quic_test_server_interface.h @@ -0,0 +1,30 @@ +#pragma once + +#include "test/common/integration/quic_test_server.h" + +// NOLINT(namespace-envoy) + +#ifdef __cplusplus +extern "C" { // functions +#endif + +/** + * Starts the server. Can only have one server active per JVM. This is blocking until the port can + * start accepting requests. + */ +void start_server(); + +/** + * Shutdowns the server. Can be restarted later. This is blocking until the server has freed all + * resources. + */ +void shutdown_server(); + +/** + * Returns the port that got attributed. Can only be called once the server has been started. + */ +int get_server_port(); + +#ifdef __cplusplus +} // functions +#endif diff --git a/test/common/jni/BUILD b/test/common/jni/BUILD new file mode 100644 index 0000000000..3a167db040 --- /dev/null +++ b/test/common/jni/BUILD @@ -0,0 +1,32 @@ +load("@envoy//bazel:envoy_build_system.bzl", "envoy_package") +load("@rules_cc//cc:defs.bzl", "cc_binary") +load("//bazel:kotlin_lib.bzl", "envoy_mobile_so_to_jni_lib") + +licenses(["notice"]) # Apache 2 + +envoy_package() + +# OS X binary (.jnilib) for Quic Test Server +envoy_mobile_so_to_jni_lib( + name = "libquic_test_server_jni.jnilib", + testonly = True, + native_dep = "libquic_test_server_jni.so", +) + +# Binary for Quic Test Server +cc_binary( + name = "libquic_test_server_jni.so", + testonly = True, + srcs = [ + "quic_test_server_jni_interface.cc", + "//library/common/jni:android_test_jni_interface.cc", + "//library/common/jni:jni_interface.cc", + "@local_jdk//:jni_header", + ], + copts = ["-std=c++17"], + linkshared = True, + deps = [ + "//library/common/jni:base_java_jni_lib", + "//test/common/integration:quic_test_server_interface_lib", + ], +) diff --git a/test/common/jni/quic_test_server_jni_interface.cc b/test/common/jni/quic_test_server_jni_interface.cc new file mode 100644 index 0000000000..f1c5581f2f --- /dev/null +++ b/test/common/jni/quic_test_server_jni_interface.cc @@ -0,0 +1,32 @@ +#include + +#include "test/common/integration/quic_test_server_interface.h" + +#include "library/common/jni/jni_support.h" +#include "library/common/jni/jni_utility.h" +#include "library/common/jni/jni_version.h" + +// NOLINT(namespace-envoy) + +// Quic Test ServerJniLibrary + +extern "C" JNIEXPORT void JNICALL +Java_io_envoyproxy_envoymobile_engine_testing_QuicTestServer_nativeStartQuicTestServer( + JNIEnv* env, jclass clazz) { + jni_log("[QTS]", "starting server"); + start_server(); +} + +extern "C" JNIEXPORT jint JNICALL +Java_io_envoyproxy_envoymobile_engine_testing_QuicTestServer_nativeGetServerPort(JNIEnv* env, + jclass clazz) { + jni_log("[QTS]", "getting server port"); + return get_server_port(); +} + +extern "C" JNIEXPORT void JNICALL +Java_io_envoyproxy_envoymobile_engine_testing_QuicTestServer_nativeShutdownQuicTestServer( + JNIEnv* env, jclass clazz) { + jni_log("[QTS]", "shutting down server"); + shutdown_server(); +} diff --git a/test/java/io/envoyproxy/envoymobile/engine/testing/BUILD b/test/java/io/envoyproxy/envoymobile/engine/testing/BUILD new file mode 100644 index 0000000000..fcdd93891c --- /dev/null +++ b/test/java/io/envoyproxy/envoymobile/engine/testing/BUILD @@ -0,0 +1,35 @@ +load("@envoy//bazel:envoy_build_system.bzl", "envoy_package") +load("@envoy_mobile//bazel:kotlin_test.bzl", "envoy_mobile_android_test") + +licenses(["notice"]) # Apache 2 + +envoy_package() + +android_library( + name = "testing", + testonly = True, + srcs = [ + "QuicTestServer.java", + ], + visibility = ["//test:__subpackages__"], +) + +envoy_mobile_android_test( + name = "quic_test_server_test", + srcs = [ + "QuicTestServerTest.java", + ], + exec_properties = { + # TODO(lfpino): Remove this once the sandboxNetwork=off works for ipv4 localhost addresses. + "sandboxNetwork": "standard", + }, + library_path = "test/common/jni", + native_deps = [ + "//test/common/jni:libquic_test_server_jni.so", + "//test/common/jni:libquic_test_server_jni.jnilib", + ], + deps = [ + ":testing", + "//library/kotlin/io/envoyproxy/envoymobile:envoy_lib", + ], +) diff --git a/test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServer.java b/test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServer.java new file mode 100644 index 0000000000..db57b7f28b --- /dev/null +++ b/test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServer.java @@ -0,0 +1,55 @@ +package io.envoyproxy.envoymobile.engine.testing; + +import java.util.concurrent.atomic.AtomicBoolean; + +/** + * Wrapper class to start a Quic test server. + */ +public final class QuicTestServer { + + private static final AtomicBoolean sServerRunning = new AtomicBoolean(); + + /* + * Starts the server. Throws an {@link IllegalStateException} if already started. + */ + public static void startQuicTestServer() { + if (!sServerRunning.compareAndSet(false, true)) { + throw new IllegalStateException("Quic server is already running"); + } + nativeStartQuicTestServer(); + } + + /** + * Shutdowns the server. No-op if the server is already shutdown. + */ + public static void shutdownQuicTestServer() { + if (!sServerRunning.compareAndSet(true, false)) { + return; + } + nativeShutdownQuicTestServer(); + } + + public static String getServerURL() { + return "https://" + getServerHost() + ":" + getServerPort() + "/"; + } + + public static String getServerHost() { return "test.example.com"; } + + /** + * Returns the server attributed port. Throws an {@link IllegalStateException} if not started. + */ + public static int getServerPort() { + if (!sServerRunning.get()) { + throw new IllegalStateException("Quic server not started."); + } + return nativeGetServerPort(); + } + + private static native void nativeStartQuicTestServer(); + + private static native void nativeShutdownQuicTestServer(); + + private static native int nativeGetServerPort(); + + private QuicTestServer() {} +} diff --git a/test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServerTest.java b/test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServerTest.java new file mode 100644 index 0000000000..982b4f5a21 --- /dev/null +++ b/test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServerTest.java @@ -0,0 +1,282 @@ +package io.envoyproxy.envoymobile.engine.testing; + +import static org.assertj.core.api.Assertions.assertThat; + +import android.content.Context; +import androidx.test.core.app.ApplicationProvider; +import io.envoyproxy.envoymobile.AndroidEngineBuilder; +import io.envoyproxy.envoymobile.Custom; +import io.envoyproxy.envoymobile.Engine; +import io.envoyproxy.envoymobile.EnvoyError; +import io.envoyproxy.envoymobile.LogLevel; +import io.envoyproxy.envoymobile.RequestHeaders; +import io.envoyproxy.envoymobile.RequestHeadersBuilder; +import io.envoyproxy.envoymobile.RequestMethod; +import io.envoyproxy.envoymobile.ResponseHeaders; +import io.envoyproxy.envoymobile.ResponseTrailers; +import io.envoyproxy.envoymobile.Stream; +import io.envoyproxy.envoymobile.engine.AndroidJniLibrary; +import io.envoyproxy.envoymobile.engine.JniLibrary; +import java.net.MalformedURLException; +import java.net.URL; +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; +import org.junit.After; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RobolectricTestRunner; + +@RunWith(RobolectricTestRunner.class) +public class QuicTestServerTest { + + private static final String HCM_TYPE = + "type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager"; + + private static final String QUIC_UPSTREAM_TYPE = + "type.googleapis.com/envoy.extensions.transport_sockets.quic.v3.QuicUpstreamTransport"; + + private static final String config = + "static_resources:\n" + + " listeners:\n" + + " - name: base_api_listener\n" + + " address:\n" + + " socket_address: { protocol: TCP, address: 0.0.0.0, port_value: 10000 }\n" + + " api_listener:\n" + + " api_listener:\n" + + " \"@type\": " + HCM_TYPE + "\n" + + " stat_prefix: api_hcm\n" + + " route_config:\n" + + " name: api_router\n" + + " virtual_hosts:\n" + + " - name: api\n" + + " domains: [\"*\"]\n" + + " routes:\n" + + " - match: { prefix: \"/\" }\n" + + " route: { cluster: h3_remote }\n" + + " http_filters:\n" + + " - name: envoy.router\n" + + " typed_config:\n" + + " \"@type\": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router\n" + + " clusters:\n" + + " - name: h3_remote\n" + + " connect_timeout: 10s\n" + + " type: STATIC\n" + + " dns_lookup_family: V4_ONLY\n" + + " lb_policy: ROUND_ROBIN\n" + + " load_assignment:\n" + + " cluster_name: h3_remote\n" + + " endpoints:\n" + + " - lb_endpoints:\n" + + " - endpoint:\n" + + " address:\n" + + " socket_address: { address: 127.0.0.1, port_value: %s }\n" + + " typed_extension_protocol_options:\n" + + " envoy.extensions.upstreams.http.v3.HttpProtocolOptions:\n" + + + " \"@type\": type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions\n" + + " explicit_http_config:\n" + + " http3_protocol_options: {}\n" + + " common_http_protocol_options:\n" + + " idle_timeout: 1s\n" + + " transport_socket:\n" + + " name: envoy.transport_sockets.quic\n" + + " typed_config:\n" + + " \"@type\": " + QUIC_UPSTREAM_TYPE + "\n" + + " upstream_tls_context:\n" + + " sni: www.lyft.com"; + + private final Context appContext = ApplicationProvider.getApplicationContext(); + private Engine engine; + + @BeforeClass + public static void loadJniLibrary() { + AndroidJniLibrary.loadTestLibrary(); + JniLibrary.load(); + } + + @Before + public void setUpEngine() throws Exception { + QuicTestServer.startQuicTestServer(); + CountDownLatch latch = new CountDownLatch(1); + engine = new AndroidEngineBuilder( + appContext, new Custom(String.format(config, QuicTestServer.getServerPort()))) + .addLogLevel(LogLevel.OFF) + .setOnEngineRunning(() -> { + latch.countDown(); + return null; + }) + .build(); + latch.await(); // Don't launch a request before initialization has completed. + } + + @After + public void shutdownEngine() { + engine.terminate(); + QuicTestServer.shutdownQuicTestServer(); + } + + @Test + public void get_simple() throws Exception { + QuicTestServerTest.RequestScenario requestScenario = new QuicTestServerTest.RequestScenario() + .setHttpMethod(RequestMethod.GET) + .setUrl(QuicTestServer.getServerURL()); + + QuicTestServerTest.Response response = sendRequest(requestScenario); + + assertThat(response.getHeaders().getHttpStatus()).isEqualTo(200); + assertThat(response.getBodyAsString()).isEqualTo("aaaaaaaaaa"); + assertThat(response.getEnvoyError()).isNull(); + } + + private QuicTestServerTest.Response sendRequest(RequestScenario requestScenario) + throws Exception { + final CountDownLatch latch = new CountDownLatch(1); + final AtomicReference response = + new AtomicReference<>(new QuicTestServerTest.Response()); + + Stream stream = engine.streamClient() + .newStreamPrototype() + .setOnResponseHeaders((responseHeaders, endStream, ignored) -> { + response.get().setHeaders(responseHeaders); + return null; + }) + .setOnResponseData((data, endStream, ignored) -> { + response.get().addBody(data); + return null; + }) + .setOnResponseTrailers((trailers, ignored) -> { + response.get().setTrailers(trailers); + return null; + }) + .setOnError((error, ignored1, ignored2) -> { + response.get().setEnvoyError(error); + latch.countDown(); + return null; + }) + .setOnCancel((ignored1, ignored2) -> { + response.get().setCancelled(); + latch.countDown(); + return null; + }) + .setOnComplete((ignored1, ignored2) -> { + latch.countDown(); + return null; + }) + .start(Executors.newSingleThreadExecutor()) + .sendHeaders(requestScenario.getHeaders(), !requestScenario.hasBody()); + requestScenario.getBodyChunks().forEach(stream::sendData); + requestScenario.getClosingBodyChunk().ifPresent(stream::close); + + latch.await(); + response.get().throwAssertionErrorIfAny(); + return response.get(); + } + + private static class RequestScenario { + + private URL url; + private RequestMethod method = null; + private final List bodyChunks = new ArrayList<>(); + private final boolean closeBodyStream = false; + + RequestHeaders getHeaders() { + RequestHeadersBuilder requestHeadersBuilder = + new RequestHeadersBuilder(method, url.getProtocol(), url.getAuthority(), url.getPath()); + requestHeadersBuilder.add("no_trailers", "0"); + return requestHeadersBuilder.build(); + } + + List getBodyChunks() { + return closeBodyStream + ? Collections.unmodifiableList(bodyChunks.subList(0, bodyChunks.size() - 1)) + : Collections.unmodifiableList(bodyChunks); + } + + Optional getClosingBodyChunk() { + return closeBodyStream ? Optional.of(bodyChunks.get(bodyChunks.size() - 1)) + : Optional.empty(); + } + + boolean hasBody() { return !bodyChunks.isEmpty(); } + + QuicTestServerTest.RequestScenario setHttpMethod(RequestMethod requestMethod) { + this.method = requestMethod; + return this; + } + + QuicTestServerTest.RequestScenario setUrl(String url) throws MalformedURLException { + this.url = new URL(url); + return this; + } + } + + private static class Response { + + private final AtomicReference headers = new AtomicReference<>(); + private final AtomicReference trailers = new AtomicReference<>(); + private final AtomicReference envoyError = new AtomicReference<>(); + private final List bodies = new ArrayList<>(); + private final AtomicBoolean cancelled = new AtomicBoolean(false); + private final AtomicReference assertionError = new AtomicReference<>(); + + void setHeaders(ResponseHeaders headers) { + if (!this.headers.compareAndSet(null, headers)) { + assertionError.compareAndSet( + null, new AssertionError("setOnResponseHeaders called more than once.")); + } + } + + void addBody(ByteBuffer body) { bodies.add(body); } + + void setTrailers(ResponseTrailers trailers) { + if (!this.trailers.compareAndSet(null, trailers)) { + assertionError.compareAndSet( + null, new AssertionError("setOnResponseTrailers called more than once.")); + } + } + + void setEnvoyError(EnvoyError envoyError) { + if (!this.envoyError.compareAndSet(null, envoyError)) { + assertionError.compareAndSet(null, new AssertionError("setOnError called more than once.")); + } + } + + void setCancelled() { + if (!cancelled.compareAndSet(false, true)) { + assertionError.compareAndSet(null, + new AssertionError("setOnCancel called more than once.")); + } + } + + EnvoyError getEnvoyError() { return envoyError.get(); } + + ResponseHeaders getHeaders() { return headers.get(); } + + String getBodyAsString() { + int totalSize = bodies.stream().mapToInt(ByteBuffer::limit).sum(); + byte[] body = new byte[totalSize]; + int pos = 0; + for (ByteBuffer buffer : bodies) { + int bytesToRead = buffer.limit(); + buffer.get(body, pos, bytesToRead); + pos += bytesToRead; + } + return new String(body); + } + + void throwAssertionErrorIfAny() { + if (assertionError.get() != null) { + throw assertionError.get(); + } + } + } +} diff --git a/test/java/org/chromium/net/testing/BUILD b/test/java/org/chromium/net/testing/BUILD index 6209f0e41d..249d0e76c7 100644 --- a/test/java/org/chromium/net/testing/BUILD +++ b/test/java/org/chromium/net/testing/BUILD @@ -21,6 +21,7 @@ android_library( "NativeTestServer.java", "PathUtils.java", "StrictModeContext.java", + "TestFilesInstaller.java", "TestUploadDataProvider.java", "TestUrlRequestCallback.java", "UrlUtils.java", From 97c501a22a7e219e61aea9da8995364f7a4435a5 Mon Sep 17 00:00:00 2001 From: Charles Le Borgne <80125532+carloseltuerto@users.noreply.github.com> Date: Thu, 20 Jan 2022 19:33:34 +0000 Subject: [PATCH 08/21] Default timestamp to -1 for envoy_final_stream_intel (#2006) Description: Default timestamp to -1 for envoy_final_stream_intel Risk Level: small Testing: CI Docs Changes: N/A Release Notes: N/A Signed-off-by: Charles Le Borgne --- .../filters/http/platform_bridge/filter.cc | 3 +- library/common/http/client.h | 3 +- .../common/stream_info/extra_stream_info.cc | 4 +- library/common/types/c_types.h | 24 +++++----- library/swift/FinalStreamIntel.swift | 45 ++++++++++--------- 5 files changed, 41 insertions(+), 38 deletions(-) diff --git a/library/common/extensions/filters/http/platform_bridge/filter.cc b/library/common/extensions/filters/http/platform_bridge/filter.cc index d107d718c7..24cc048b14 100644 --- a/library/common/extensions/filters/http/platform_bridge/filter.cc +++ b/library/common/extensions/filters/http/platform_bridge/filter.cc @@ -192,8 +192,7 @@ Http::LocalErrorStatus PlatformBridgeFilter::onLocalReply(const LocalReplyData& envoy_final_stream_intel PlatformBridgeFilter::finalStreamIntel() { RELEASE_ASSERT(decoder_callbacks_, "StreamInfo accessed before filter callbacks are set"); // FIXME: Stream handle cannot currently be set from the filter context. - envoy_final_stream_intel final_stream_intel; - memset(&final_stream_intel, 0, sizeof(final_stream_intel)); + envoy_final_stream_intel final_stream_intel{-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 0, 0, 0}; setFinalStreamIntel(decoder_callbacks_->streamInfo(), final_stream_intel); return final_stream_intel; } diff --git a/library/common/http/client.h b/library/common/http/client.h index 623cd399b4..b07ad397df 100644 --- a/library/common/http/client.h +++ b/library/common/http/client.h @@ -280,7 +280,8 @@ class Client : public Logger::Loggable { bool explicit_flow_control_ = false; // Latest intel data retrieved from the StreamInfo. envoy_stream_intel stream_intel_{-1, -1, 0}; - envoy_final_stream_intel envoy_final_stream_intel_; + envoy_final_stream_intel envoy_final_stream_intel_{-1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, 0, 0, 0}; StreamInfo::BytesMeterSharedPtr bytes_meter_; }; diff --git a/library/common/stream_info/extra_stream_info.cc b/library/common/stream_info/extra_stream_info.cc index 50cc1122cb..0bd2f05ec7 100644 --- a/library/common/stream_info/extra_stream_info.cc +++ b/library/common/stream_info/extra_stream_info.cc @@ -6,14 +6,14 @@ namespace Envoy { namespace StreamInfo { namespace { -void setFromOptional(uint64_t& to_set, const absl::optional& time) { +void setFromOptional(int64_t& to_set, const absl::optional& time) { if (time.has_value()) { to_set = std::chrono::duration_cast(time.value().time_since_epoch()) .count(); } } -void setFromOptional(uint64_t& to_set, absl::optional time, long offset) { +void setFromOptional(int64_t& to_set, absl::optional time, long offset) { if (time.has_value()) { to_set = offset + std::chrono::duration_cast(time.value()).count(); } diff --git a/library/common/types/c_types.h b/library/common/types/c_types.h index 417a1872aa..72fac8dbca 100644 --- a/library/common/types/c_types.h +++ b/library/common/types/c_types.h @@ -157,34 +157,36 @@ typedef struct { /** * Contains internal HTTP stream metrics which sent at stream end. + * + * Note: for the signed fields, -1 means not present. */ typedef struct { // The time the request started, in ms since the epoch. - uint64_t request_start_ms; + int64_t request_start_ms; // The time the DNS resolution for this request started, in ms since the epoch. - uint64_t dns_start_ms; + int64_t dns_start_ms; // The time the DNS resolution for this request completed, in ms since the epoch. - uint64_t dns_end_ms; + int64_t dns_end_ms; // The time the upstream connection started, in ms since the epoch. // This may not be set if socket_reused is false. - uint64_t connect_start_ms; + int64_t connect_start_ms; // The time the upstream connection completed, in ms since the epoch. // This may not be set if socket_reused is false. - uint64_t connect_end_ms; + int64_t connect_end_ms; // The time the SSL handshake started, in ms since the epoch. // This may not be set if socket_reused is false. - uint64_t ssl_start_ms; + int64_t ssl_start_ms; // The time the SSL handshake completed, in ms since the epoch. // This may not be set if socket_reused is false. - uint64_t ssl_end_ms; + int64_t ssl_end_ms; // The time the first byte of the request was sent upstream, in ms since the epoch. - uint64_t sending_start_ms; + int64_t sending_start_ms; // The time the last byte of the request was sent upstream, in ms since the epoch. - uint64_t sending_end_ms; + int64_t sending_end_ms; // The time the first byte of the response was received, in ms since the epoch. - uint64_t response_start_ms; + int64_t response_start_ms; // The time the last byte of the request was received, in ms since the epoch. - uint64_t request_end_ms; + int64_t request_end_ms; // True if the upstream socket had been used previously. uint64_t socket_reused; // The number of bytes sent upstream. diff --git a/library/swift/FinalStreamIntel.swift b/library/swift/FinalStreamIntel.swift index a662c5bfe6..970a919e28 100644 --- a/library/swift/FinalStreamIntel.swift +++ b/library/swift/FinalStreamIntel.swift @@ -2,30 +2,31 @@ import Foundation /// Exposes one time HTTP stream metrics, context, and other details. +/// Note: -1 means "not present" for the fields of type Int64. @objcMembers public final class FinalStreamIntel: StreamIntel { /// The time the request started, in ms since the epoch. - public let requestStartMs: UInt64 + public let requestStartMs: Int64 /// The time the DNS resolution for this request started, in ms since the epoch. - public let dnsStartMs: UInt64 + public let dnsStartMs: Int64 /// The time the DNS resolution for this request completed, in ms since the epoch. - public let dnsEndMs: UInt64 + public let dnsEndMs: Int64 /// The time the upstream connection started, in ms since the epoch. (1) - public let connectStartMs: UInt64 + public let connectStartMs: Int64 /// The time the upstream connection completed, in ms since the epoch. (1) - public let connectEndMs: UInt64 + public let connectEndMs: Int64 /// The time the SSL handshake started, in ms since the epoch. (1) - public let sslStartMs: UInt64 + public let sslStartMs: Int64 /// The time the SSL handshake completed, in ms since the epoch. (1) - public let sslEndMs: UInt64 + public let sslEndMs: Int64 /// The time the first byte of the request was sent upstream, in ms since the epoch. - public let sendingStartMs: UInt64 + public let sendingStartMs: Int64 /// The time the last byte of the request was sent upstream, in ms since the epoch. - public let sendingEndMs: UInt64 + public let sendingEndMs: Int64 /// The time the first byte of the response was received, in ms since the epoch. - public let responseStartMs: UInt64 + public let responseStartMs: Int64 /// The time the last byte of the request was received, in ms since the epoch. - public let requestEndMs: UInt64 + public let requestEndMs: Int64 /// True if the upstream socket had been used previously. public let socketReused: Bool /// The number of bytes sent upstream. @@ -39,17 +40,17 @@ public final class FinalStreamIntel: StreamIntel { streamId: Int64, connectionId: Int64, attemptCount: UInt64, - requestStartMs: UInt64, - dnsStartMs: UInt64, - dnsEndMs: UInt64, - connectStartMs: UInt64, - connectEndMs: UInt64, - sslStartMs: UInt64, - sslEndMs: UInt64, - sendingStartMs: UInt64, - sendingEndMs: UInt64, - responseStartMs: UInt64, - requestEndMs: UInt64, + requestStartMs: Int64, + dnsStartMs: Int64, + dnsEndMs: Int64, + connectStartMs: Int64, + connectEndMs: Int64, + sslStartMs: Int64, + sslEndMs: Int64, + sendingStartMs: Int64, + sendingEndMs: Int64, + responseStartMs: Int64, + requestEndMs: Int64, socketReused: Bool, sentByteCount: UInt64, receivedByteCount: UInt64 From 3553b3cec846bb7d46d665485928ba5f46a9e2b6 Mon Sep 17 00:00:00 2001 From: Jose Ulises Nino Rivera Date: Mon, 24 Jan 2022 12:53:34 -0800 Subject: [PATCH 09/21] envoy: 519774f742 (#2015) Signed-off-by: Jose Nino --- envoy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/envoy b/envoy index a9d72603c6..519774f742 160000 --- a/envoy +++ b/envoy @@ -1 +1 @@ -Subproject commit a9d72603c68da3a10a1c0d021d01c7877e6f2a30 +Subproject commit 519774f742bb2500cf5f6a00933c9662337acbb2 From af4d8955f7e0ff0fb8095dc4d49ab9b9673b5752 Mon Sep 17 00:00:00 2001 From: Charles Le Borgne <80125532+carloseltuerto@users.noreply.github.com> Date: Tue, 25 Jan 2022 12:20:05 +0000 Subject: [PATCH 10/21] test: Solve CI flakiness for test/java/org/chromium/net/urlconnection:urlconnection_test (#2007) Description: EM has a 1024 concurrent request limit that is now respected by Cronet tests Risk Level: N/A Testing: Solves CI "android_tests / java_tests_mac" failure Docs Changes: N/A Release Notes: N/A Signed-off-by: Charles Le Borgne --- test/java/org/chromium/net/CronetUrlRequestTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/java/org/chromium/net/CronetUrlRequestTest.java b/test/java/org/chromium/net/CronetUrlRequestTest.java index 6d27c568e4..61e9249813 100644 --- a/test/java/org/chromium/net/CronetUrlRequestTest.java +++ b/test/java/org/chromium/net/CronetUrlRequestTest.java @@ -2305,8 +2305,7 @@ public void testTagging() throws Exception {} */ public void testManyRequests() throws Exception { String url = NativeTestServer.getMultiRedirectURL(); - // Jelly Bean has a 2000 limit on global references, crbug.com/922656. - final int numRequests = Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT ? 2000 : 1500; + final int numRequests = 1000; TestUrlRequestCallback callbacks[] = new TestUrlRequestCallback[numRequests]; UrlRequest requests[] = new UrlRequest[numRequests]; for (int i = 0; i < numRequests; i++) { From 247e1c9c894d867257f7e37d4920ff1a7d96af84 Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Tue, 25 Jan 2022 08:40:11 -0500 Subject: [PATCH 11/21] tooling: first pass at oncall tooling (#2014) General functionality: send daily reminders to folks listed as reviewers. Lyft folk: please let me know who else if any should be added send on-call channel updates for PRs which are out-SLO (haven't been reviewed in 2 days, and aren't tagged as waiting) Major differences from the Envoy tooling: removed all the first-pass review and api review cruft no nags for unassigned PRs - I think we'll want this at some point but I think most repo contributors have been assigning their own when the PR is ready, WDYT? Signed-off-by: Alyssa Wilk --- .github/actions/pr_notifier/pr_notifier.py | 174 +++++++++++++++++++ .github/actions/pr_notifier/requirements.in | 2 + .github/actions/pr_notifier/requirements.txt | 124 +++++++++++++ .github/workflows/pr_notifier.yml | 26 +++ 4 files changed, 326 insertions(+) create mode 100644 .github/actions/pr_notifier/pr_notifier.py create mode 100644 .github/actions/pr_notifier/requirements.in create mode 100644 .github/actions/pr_notifier/requirements.txt create mode 100644 .github/workflows/pr_notifier.yml diff --git a/.github/actions/pr_notifier/pr_notifier.py b/.github/actions/pr_notifier/pr_notifier.py new file mode 100644 index 0000000000..7efbecea70 --- /dev/null +++ b/.github/actions/pr_notifier/pr_notifier.py @@ -0,0 +1,174 @@ +# Script for collecting PRs in need of review, and informing reviewers via +# slack. +# +# By default this runs in "developer mode" which means that it collects PRs +# associated with reviewers and API reviewers, and spits them out (badly +# formatted) to the command line. +# +# .github/workflows/pr_notifier.yml runs the script with --cron_job +# which instead sends the collected PRs to the various slack channels. +# +# NOTE: Slack IDs can be found in the user's full profile from within Slack. + +from __future__ import print_function + +import argparse +import datetime +import os +import sys + +import github +from slack_sdk import WebClient +from slack_sdk.errors import SlackApiError + +REVIEWERS = { + 'alyssawilk': 'U78RP48V9', + 'Augustyniak': 'U017R1YHXGQ', + 'buildbreaker': 'UEUEP1QP4', + 'jpsim': 'U02KAPRELKA', + 'junr03': 'U79K0Q431', + 'RyanTheOptimist': 'U01SW3JC8GP', + 'goaway': 'U7TDPD3L2', + 'snowp': 'U93KTPQP6', +} + +def get_slo_hours(): + # on Monday, allow for 24h + 48h + if datetime.date.today().weekday() == 0: + return 72 + return 24 + + +# Return true if the PR has a waiting tag, false otherwise. +def is_waiting(labels): + for label in labels: + if label.name == 'waiting' or label.name == 'waiting:any': + return True + return False + + +# Generate a pr message, bolding the time if it's out-SLO +def pr_message(pr_age, pr_url, pr_title, delta_days, delta_hours): + if pr_age < datetime.timedelta(hours=get_slo_hours()): + return "<%s|%s> has been waiting %s days %s hours\n" % ( + pr_url, pr_title, delta_days, delta_hours) + else: + return "<%s|%s> has been waiting *%s days %s hours*\n" % ( + pr_url, pr_title, delta_days, delta_hours) + + +# Adds reminder lines to the appropriate assignee to review the assigned PRs +# Returns true if one of the assignees is in the primary_assignee_map, false otherwise. +def add_reminders( + assignees, assignees_and_prs, message, primary_assignee_map): + has_primary_assignee = False + for assignee_info in assignees: + assignee = assignee_info.login + if assignee in primary_assignee_map: + has_primary_assignee = True + if assignee not in assignees_and_prs.keys(): + assignees_and_prs[ + assignee] = "Hello, %s, here are your PR reminders for the day \n" % assignee + assignees_and_prs[assignee] = assignees_and_prs[assignee] + message + return has_primary_assignee + + +def track_prs(): + git = github.Github() + repo = git.get_repo('envoyproxy/envoy-mobile') + + # A dict of maintainer : outstanding_pr_string to be sent to slack + reviewers_and_prs = {} + # Out-SLO PRs to be sent to #envoy-maintainer-oncall + stalled_prs = "" + + # Snag all PRs, including drafts + for pr_info in repo.get_pulls("open", "updated", "desc"): + labels = pr_info.labels + assignees = pr_info.assignees + # If the PR is waiting, continue. + if is_waiting(labels): + continue + # Drafts are not covered by our SLO (repokitteh warns of this) + if pr_info.draft: + continue + # envoy-mobile currently doesn't triage unassigned PRs. + if not(pr_info.assignees): + continue + + # Update the time based on the time zone delta from github's + pr_age = pr_info.updated_at - datetime.timedelta(hours=4) + delta = datetime.datetime.now() - pr_age + delta_days = delta.days + delta_hours = delta.seconds // 3600 + + # If we get to this point, the review may be in SLO - nudge if it's in + # SLO, nudge in bold if not. + message = pr_message(delta, pr_info.html_url, pr_info.title, delta_days, delta_hours) + + # If the PR has been out-SLO for over a day, inform maintainers. + if delta > datetime.timedelta(hours=get_slo_hours() + 36): + stalled_prs = stalled_prs + message + + # Add a reminder to each maintainer-assigner on the PR. + add_reminders(pr_info.assignees, reviewers_and_prs, message, REVIEWERS) + + # Return the dict of {reviewers : PR notifications}, + # and stalled PRs + return reviewers_and_prs, stalled_prs + + +def post_to_assignee(client, assignees_and_messages, assignees_map): + # Post updates to individual assignees + for key in assignees_and_messages: + message = assignees_and_messages[key] + + # Only send messages if we have the slack UID + if key not in assignees_map: + continue + uid = assignees_map[key] + + # Ship messages off to slack. + try: + print(assignees_and_messages[key]) + response = client.conversations_open(users=uid, text="hello") + channel_id = response["channel"]["id"] + response = client.chat_postMessage(channel=channel_id, text=message) + except SlackApiError as e: + print("Unexpected error %s", e.response["error"]) + + +def post_to_oncall(client, out_slo_prs): + try: + response = client.chat_postMessage( + channel='#envoy-mobile-oncall', text=("*Stalled PRs*\n\n%s" % out_slo_prs)) + except SlackApiError as e: + print("Unexpected error %s", e.response["error"]) + + +if __name__ == '__main__': + parser = argparse.ArgumentParser() + parser.add_argument( + '--cron_job', + action="store_true", + help="true if this is run by the daily cron job, false if run manually by a developer") + args = parser.parse_args() + + reviewers_and_messages, stalled_prs = track_prs() + + if not args.cron_job: + print(reviewers_and_messages) + print("\n\n\n") + print(stalled_prs) + exit(0) + + SLACK_BOT_TOKEN = os.getenv('SLACK_BOT_TOKEN') + if not SLACK_BOT_TOKEN: + print( + 'Missing SLACK_BOT_TOKEN: please export token from https://api.slack.com/apps/A023NPQQ33K/oauth?' + ) + sys.exit(1) + + client = WebClient(token=SLACK_BOT_TOKEN) + post_to_oncall(client, reviewers_and_messages['unassigned'], stalled_prs) + post_to_assignee(client, reviewers_and_messages, REVIEWERS) diff --git a/.github/actions/pr_notifier/requirements.in b/.github/actions/pr_notifier/requirements.in new file mode 100644 index 0000000000..b27ccacba2 --- /dev/null +++ b/.github/actions/pr_notifier/requirements.in @@ -0,0 +1,2 @@ +pygithub +slack_sdk diff --git a/.github/actions/pr_notifier/requirements.txt b/.github/actions/pr_notifier/requirements.txt new file mode 100644 index 0000000000..c8f43486f2 --- /dev/null +++ b/.github/actions/pr_notifier/requirements.txt @@ -0,0 +1,124 @@ +# +# This file is autogenerated by pip-compile +# To update, run: +# +# pip-compile --generate-hashes .github/actions/pr_notifier/requirements.txt +# +certifi==2021.5.30 \ + --hash=sha256:2bbf76fd432960138b3ef6dda3dde0544f27cbf8546c458e60baf371917ba9ee \ + --hash=sha256:50b1e4f8446b06f41be7dd6338db18e0990601dce795c2b1686458aa7e8fa7d8 + # via requests +cffi==1.14.5 \ + --hash=sha256:005a36f41773e148deac64b08f233873a4d0c18b053d37da83f6af4d9087b813 \ + --hash=sha256:04c468b622ed31d408fea2346bec5bbffba2cc44226302a0de1ade9f5ea3d373 \ + --hash=sha256:06d7cd1abac2ffd92e65c0609661866709b4b2d82dd15f611e602b9b188b0b69 \ + --hash=sha256:06db6321b7a68b2bd6df96d08a5adadc1fa0e8f419226e25b2a5fbf6ccc7350f \ + --hash=sha256:0857f0ae312d855239a55c81ef453ee8fd24136eaba8e87a2eceba644c0d4c06 \ + --hash=sha256:0f861a89e0043afec2a51fd177a567005847973be86f709bbb044d7f42fc4e05 \ + --hash=sha256:1071534bbbf8cbb31b498d5d9db0f274f2f7a865adca4ae429e147ba40f73dea \ + --hash=sha256:158d0d15119b4b7ff6b926536763dc0714313aa59e320ddf787502c70c4d4bee \ + --hash=sha256:1bf1ac1984eaa7675ca8d5745a8cb87ef7abecb5592178406e55858d411eadc0 \ + --hash=sha256:1f436816fc868b098b0d63b8920de7d208c90a67212546d02f84fe78a9c26396 \ + --hash=sha256:24a570cd11895b60829e941f2613a4f79df1a27344cbbb82164ef2e0116f09c7 \ + --hash=sha256:24ec4ff2c5c0c8f9c6b87d5bb53555bf267e1e6f70e52e5a9740d32861d36b6f \ + --hash=sha256:2894f2df484ff56d717bead0a5c2abb6b9d2bf26d6960c4604d5c48bbc30ee73 \ + --hash=sha256:29314480e958fd8aab22e4a58b355b629c59bf5f2ac2492b61e3dc06d8c7a315 \ + --hash=sha256:293e7ea41280cb28c6fcaaa0b1aa1f533b8ce060b9e701d78511e1e6c4a1de76 \ + --hash=sha256:34eff4b97f3d982fb93e2831e6750127d1355a923ebaeeb565407b3d2f8d41a1 \ + --hash=sha256:35f27e6eb43380fa080dccf676dece30bef72e4a67617ffda586641cd4508d49 \ + --hash=sha256:3c3f39fa737542161d8b0d680df2ec249334cd70a8f420f71c9304bd83c3cbed \ + --hash=sha256:3d3dd4c9e559eb172ecf00a2a7517e97d1e96de2a5e610bd9b68cea3925b4892 \ + --hash=sha256:43e0b9d9e2c9e5d152946b9c5fe062c151614b262fda2e7b201204de0b99e482 \ + --hash=sha256:48e1c69bbacfc3d932221851b39d49e81567a4d4aac3b21258d9c24578280058 \ + --hash=sha256:51182f8927c5af975fece87b1b369f722c570fe169f9880764b1ee3bca8347b5 \ + --hash=sha256:58e3f59d583d413809d60779492342801d6e82fefb89c86a38e040c16883be53 \ + --hash=sha256:5de7970188bb46b7bf9858eb6890aad302577a5f6f75091fd7cdd3ef13ef3045 \ + --hash=sha256:65fa59693c62cf06e45ddbb822165394a288edce9e276647f0046e1ec26920f3 \ + --hash=sha256:681d07b0d1e3c462dd15585ef5e33cb021321588bebd910124ef4f4fb71aef55 \ + --hash=sha256:69e395c24fc60aad6bb4fa7e583698ea6cc684648e1ffb7fe85e3c1ca131a7d5 \ + --hash=sha256:6c97d7350133666fbb5cf4abdc1178c812cb205dc6f41d174a7b0f18fb93337e \ + --hash=sha256:6e4714cc64f474e4d6e37cfff31a814b509a35cb17de4fb1999907575684479c \ + --hash=sha256:72d8d3ef52c208ee1c7b2e341f7d71c6fd3157138abf1a95166e6165dd5d4369 \ + --hash=sha256:8ae6299f6c68de06f136f1f9e69458eae58f1dacf10af5c17353eae03aa0d827 \ + --hash=sha256:8b198cec6c72df5289c05b05b8b0969819783f9418e0409865dac47288d2a053 \ + --hash=sha256:99cd03ae7988a93dd00bcd9d0b75e1f6c426063d6f03d2f90b89e29b25b82dfa \ + --hash=sha256:9cf8022fb8d07a97c178b02327b284521c7708d7c71a9c9c355c178ac4bbd3d4 \ + --hash=sha256:9de2e279153a443c656f2defd67769e6d1e4163952b3c622dcea5b08a6405322 \ + --hash=sha256:9e93e79c2551ff263400e1e4be085a1210e12073a31c2011dbbda14bda0c6132 \ + --hash=sha256:9ff227395193126d82e60319a673a037d5de84633f11279e336f9c0f189ecc62 \ + --hash=sha256:a465da611f6fa124963b91bf432d960a555563efe4ed1cc403ba5077b15370aa \ + --hash=sha256:ad17025d226ee5beec591b52800c11680fca3df50b8b29fe51d882576e039ee0 \ + --hash=sha256:afb29c1ba2e5a3736f1c301d9d0abe3ec8b86957d04ddfa9d7a6a42b9367e396 \ + --hash=sha256:b85eb46a81787c50650f2392b9b4ef23e1f126313b9e0e9013b35c15e4288e2e \ + --hash=sha256:bb89f306e5da99f4d922728ddcd6f7fcebb3241fc40edebcb7284d7514741991 \ + --hash=sha256:cbde590d4faaa07c72bf979734738f328d239913ba3e043b1e98fe9a39f8b2b6 \ + --hash=sha256:cc5a8e069b9ebfa22e26d0e6b97d6f9781302fe7f4f2b8776c3e1daea35f1adc \ + --hash=sha256:cd2868886d547469123fadc46eac7ea5253ea7fcb139f12e1dfc2bbd406427d1 \ + --hash=sha256:d42b11d692e11b6634f7613ad8df5d6d5f8875f5d48939520d351007b3c13406 \ + --hash=sha256:df5052c5d867c1ea0b311fb7c3cd28b19df469c056f7fdcfe88c7473aa63e333 \ + --hash=sha256:f2d45f97ab6bb54753eab54fffe75aaf3de4ff2341c9daee1987ee1837636f1d \ + --hash=sha256:fd78e5fee591709f32ef6edb9a015b4aa1a5022598e36227500c8f4e02328d9c + # via pynacl +chardet==4.0.0 \ + --hash=sha256:0d6f53a15db4120f2b08c94f11e7d93d2c911ee118b6b30a04ec3ee8310179fa \ + --hash=sha256:f864054d66fd9118f2e67044ac8981a54775ec5b67aed0441892edb553d21da5 + # via requests +deprecated==1.2.13 \ + --hash=sha256:43ac5335da90c31c24ba028af536a91d41d53f9e6901ddb021bcc572ce44e38d \ + --hash=sha256:64756e3e14c8c5eea9795d93c524551432a0be75629f8f29e67ab8caf076c76d + # via pygithub +idna==2.10 \ + --hash=sha256:b307872f855b18632ce0c21c5e45be78c0ea7ae4c15c828c20788b26921eb3f6 \ + --hash=sha256:b97d804b1e9b523befed77c48dacec60e6dcb0b5391d57af6a65a312a90648c0 + # via requests +pycparser==2.20 \ + --hash=sha256:2d475327684562c3a96cc71adf7dc8c4f0565175cf86b6d7a404ff4c771f15f0 \ + --hash=sha256:7582ad22678f0fcd81102833f60ef8d0e57288b6b5fb00323d101be910e35705 + # via cffi +pygithub==1.55 \ + --hash=sha256:1bbfff9372047ff3f21d5cd8e07720f3dbfdaf6462fcaed9d815f528f1ba7283 \ + --hash=sha256:2caf0054ea079b71e539741ae56c5a95e073b81fa472ce222e81667381b9601b + # via -r requirements.in +pyjwt==2.1.0 \ + --hash=sha256:934d73fbba91b0483d3857d1aff50e96b2a892384ee2c17417ed3203f173fca1 \ + --hash=sha256:fba44e7898bbca160a2b2b501f492824fc8382485d3a6f11ba5d0c1937ce6130 + # via pygithub +pynacl==1.4.0 \ + --hash=sha256:06cbb4d9b2c4bd3c8dc0d267416aaed79906e7b33f114ddbf0911969794b1cc4 \ + --hash=sha256:11335f09060af52c97137d4ac54285bcb7df0cef29014a1a4efe64ac065434c4 \ + --hash=sha256:2fe0fc5a2480361dcaf4e6e7cea00e078fcda07ba45f811b167e3f99e8cff574 \ + --hash=sha256:30f9b96db44e09b3304f9ea95079b1b7316b2b4f3744fe3aaecccd95d547063d \ + --hash=sha256:4e10569f8cbed81cb7526ae137049759d2a8d57726d52c1a000a3ce366779634 \ + --hash=sha256:511d269ee845037b95c9781aa702f90ccc36036f95d0f31373a6a79bd8242e25 \ + --hash=sha256:537a7ccbea22905a0ab36ea58577b39d1fa9b1884869d173b5cf111f006f689f \ + --hash=sha256:54e9a2c849c742006516ad56a88f5c74bf2ce92c9f67435187c3c5953b346505 \ + --hash=sha256:757250ddb3bff1eecd7e41e65f7f833a8405fede0194319f87899690624f2122 \ + --hash=sha256:7757ae33dae81c300487591c68790dfb5145c7d03324000433d9a2c141f82af7 \ + --hash=sha256:7c6092102219f59ff29788860ccb021e80fffd953920c4a8653889c029b2d420 \ + --hash=sha256:8122ba5f2a2169ca5da936b2e5a511740ffb73979381b4229d9188f6dcb22f1f \ + --hash=sha256:9c4a7ea4fb81536c1b1f5cc44d54a296f96ae78c1ebd2311bd0b60be45a48d96 \ + --hash=sha256:c914f78da4953b33d4685e3cdc7ce63401247a21425c16a39760e282075ac4a6 \ + --hash=sha256:cd401ccbc2a249a47a3a1724c2918fcd04be1f7b54eb2a5a71ff915db0ac51c6 \ + --hash=sha256:d452a6746f0a7e11121e64625109bc4468fc3100452817001dbe018bb8b08514 \ + --hash=sha256:ea6841bc3a76fa4942ce00f3bda7d436fda21e2d91602b9e21b7ca9ecab8f3ff \ + --hash=sha256:f8851ab9041756003119368c1e6cd0b9c631f46d686b3904b18c0139f4419f80 + # via pygithub +requests==2.25.1 \ + --hash=sha256:27973dd4a904a4f13b263a19c866c13b92a39ed1c964655f025f3f8d3d75b804 \ + --hash=sha256:c210084e36a42ae6b9219e00e48287def368a26d03a048ddad7bfee44f75871e + # via pygithub +six==1.16.0 \ + --hash=sha256:1e61c37477a1626458e36f7b1d82aa5c9b094fa4802892072e49de9c60c4c926 \ + --hash=sha256:8abb2f1d86890a2dfb989f9a77cfcfd3e47c2a354b01111771326f8aa26e0254 + # via pynacl +slack_sdk==3.13.0 \ + --hash=sha256:54f2a5f7419f1ab932af9e3200f7f2f93db96e0f0eb8ad7d3b4214aa9f124641 \ + --hash=sha256:aae6ce057e286a5e7fe7a9f256e85b886eee556def8e04b82b08f699e64d7f67 + # via -r requirements.in +urllib3==1.26.6 \ + --hash=sha256:39fb8672126159acb139a7718dd10806104dec1e2f0f6c88aab05d17df10c8d4 \ + --hash=sha256:f57b4c16c62fa2760b7e3d97c35b255512fb6b59a259730f36ba32ce9f8e342f + # via requests +wrapt==1.12.1 \ + --hash=sha256:b62ffa81fb85f4332a4f609cab4ac40709470da05643a082ec1eb88e6d9b97d7 + # via deprecated diff --git a/.github/workflows/pr_notifier.yml b/.github/workflows/pr_notifier.yml new file mode 100644 index 0000000000..b08b0a1027 --- /dev/null +++ b/.github/workflows/pr_notifier.yml @@ -0,0 +1,26 @@ +on: + workflow_dispatch: + schedule: + - cron: '0 5 * * 1,2,3,4,5' + +jobs: + pr_notifier: + name: PR Notifier + runs-on: ubuntu-latest + if: github.repository_owner == 'envoyproxy' + + steps: + - uses: actions/checkout@ec3a7ce113134d7a93b817d10a8272cb61118579 + - name: Set up Python 3.8 + uses: actions/setup-python@v2 + with: + python-version: '3.8' + architecture: 'x64' + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -r ./.github/actions/pr_notifier/requirements.txt + - name: Notify about PRs + run: python ./.github/actions/pr_notifier/pr_notifier.py --cron_job + env: + SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }} From 4f1ce29f4fc576950a18d1f95adc65889eed8cf0 Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Tue, 25 Jan 2022 10:51:17 -0800 Subject: [PATCH 12/21] tls: update bundled root certificates (#2016) Description: Syncs with OpenBSD/LibreSSL. Risk Level: Moderate Testing: Local Signed-off-by: Mike Schore --- library/common/config/certificates.inc | 45 +------------------------- 1 file changed, 1 insertion(+), 44 deletions(-) diff --git a/library/common/config/certificates.inc b/library/common/config/certificates.inc index 717223fc20..b17ab2f7ef 100644 --- a/library/common/config/certificates.inc +++ b/library/common/config/certificates.inc @@ -1,5 +1,5 @@ R"certs( - # $OpenBSD: cert.pem,v 1.23 2021/06/11 11:40:35 sthen Exp $ + # $OpenBSD: cert.pem,v 1.24 2021/09/30 18:16:11 deraadt Exp $ ### /C=ES/CN=Autoridad de Certificacion Firmaprofesional CIF A62634068 === /C=ES/CN=Autoridad de Certificacion Firmaprofesional CIF A62634068 @@ -1822,49 +1822,6 @@ R"certs( gKDWHrO8Dw9TdSmq6hN35N6MgSGtBxBHEa2HPQfRdbzP82Z+ -----END CERTIFICATE----- - ### Digital Signature Trust Co. - - === /O=Digital Signature Trust Co./CN=DST Root CA X3 - Certificate: - Data: - Version: 3 (0x2) - Serial Number: - 44:af:b0:80:d6:a3:27:ba:89:30:39:86:2e:f8:40:6b - Signature Algorithm: sha1WithRSAEncryption - Validity - Not Before: Sep 30 21:12:19 2000 GMT - Not After : Sep 30 14:01:15 2021 GMT - Subject: O=Digital Signature Trust Co., CN=DST Root CA X3 - X509v3 extensions: - X509v3 Basic Constraints: critical - CA:TRUE - X509v3 Key Usage: critical - Certificate Sign, CRL Sign - X509v3 Subject Key Identifier: - C4:A7:B1:A4:7B:2C:71:FA:DB:E1:4B:90:75:FF:C4:15:60:85:89:10 - SHA1 Fingerprint=DA:C9:02:4F:54:D8:F6:DF:94:93:5F:B1:73:26:38:CA:6A:D7:7C:13 - SHA256 Fingerprint=06:87:26:03:31:A7:24:03:D9:09:F1:05:E6:9B:CF:0D:32:E1:BD:24:93:FF:C6:D9:20:6D:11:BC:D6:77:07:39 - -----BEGIN CERTIFICATE----- - MIIDSjCCAjKgAwIBAgIQRK+wgNajJ7qJMDmGLvhAazANBgkqhkiG9w0BAQUFADA/ - MSQwIgYDVQQKExtEaWdpdGFsIFNpZ25hdHVyZSBUcnVzdCBDby4xFzAVBgNVBAMT - DkRTVCBSb290IENBIFgzMB4XDTAwMDkzMDIxMTIxOVoXDTIxMDkzMDE0MDExNVow - PzEkMCIGA1UEChMbRGlnaXRhbCBTaWduYXR1cmUgVHJ1c3QgQ28uMRcwFQYDVQQD - Ew5EU1QgUm9vdCBDQSBYMzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB - AN+v6ZdQCINXtMxiZfaQguzH0yxrMMpb7NnDfcdAwRgUi+DoM3ZJKuM/IUmTrE4O - rz5Iy2Xu/NMhD2XSKtkyj4zl93ewEnu1lcCJo6m67XMuegwGMoOifooUMM0RoOEq - OLl5CjH9UL2AZd+3UWODyOKIYepLYYHsUmu5ouJLGiifSKOeDNoJjj4XLh7dIN9b - xiqKqy69cK3FCxolkHRyxXtqqzTWMIn/5WgTe1QLyNau7Fqckh49ZLOMxt+/yUFw - 7BZy1SbsOFU5Q9D8/RhcQPGX69Wam40dutolucbY38EVAjqr2m7xPi71XAicPNaD - aeQQmxkqtilX4+U9m5/wAl0CAwEAAaNCMEAwDwYDVR0TAQH/BAUwAwEB/zAOBgNV - HQ8BAf8EBAMCAQYwHQYDVR0OBBYEFMSnsaR7LHH62+FLkHX/xBVghYkQMA0GCSqG - SIb3DQEBBQUAA4IBAQCjGiybFwBcqR7uKGY3Or+Dxz9LwwmglSBd49lZRNI+DT69 - ikugdB/OEIKcdBodfpga3csTS7MgROSR6cz8faXbauX+5v3gTt23ADq1cEmv8uXr - AvHRAosZy5Q6XkjEGB5YGV8eAlrwDPGxrancWYaLbumR9YbK+rlmM6pZW87ipxZz - R8srzJmwN0jP41ZL9c8PDHIyh8bwRLtTcm1D9SZImlJnt1ir/md2cXjbDaJWFBM5 - JDGFoqgCWjBH4d1QB7wCCZAA62RjYJsWvIjJEubSfZGL+T0yjWW06XyxV3bqxbYo - Ob8VZRzI9neWagqNdwvYkQsEjgfbKbYK7p2CNTUQ - -----END CERTIFICATE----- - ### Disig a.s. === /C=SK/L=Bratislava/O=Disig a.s./CN=CA Disig Root R2 From f202ea520d4f3f241deaddf3390f9d70221ec68b Mon Sep 17 00:00:00 2001 From: Mike Schore Date: Tue, 25 Jan 2022 12:16:18 -0800 Subject: [PATCH 13/21] stream intel: update kotlin public interface to align with swift (#2013) Description: For terminal callbacks (onError, onCancel, onComplete) changes public interface to provide a single FinalStreamIntel struct that extends from the base StreamIntel available on other callbacks. Risk Level: Low Testing: Local & CI Signed-off-by: Mike Schore --- examples/java/hello_world/MainActivity.java | 2 +- examples/kotlin/hello_world/AsyncDemoFilter.kt | 5 ++--- examples/kotlin/hello_world/BufferDemoFilter.kt | 5 ++--- examples/kotlin/hello_world/DemoFilter.kt | 5 ++--- examples/kotlin/hello_world/MainActivity.kt | 2 +- .../io/envoyproxy/envoymobile/FinalStreamIntel.kt | 9 +++++++-- .../io/envoyproxy/envoymobile/StreamCallbacks.kt | 13 ++++++------- .../kotlin/io/envoyproxy/envoymobile/StreamIntel.kt | 2 +- .../io/envoyproxy/envoymobile/StreamPrototype.kt | 5 ++--- .../io/envoyproxy/envoymobile/filters/Filter.kt | 6 +++--- .../envoymobile/filters/ResponseFilter.kt | 9 +++------ .../envoymobile/grpc/GRPCStreamPrototype.kt | 4 ++-- .../integration/AndroidEnvoyExplicitFlowTest.java | 12 ++++++------ test/java/integration/AndroidEnvoyFlowTest.java | 4 ++-- .../engine/testing/QuicTestServerTest.java | 6 +++--- test/kotlin/integration/CancelStreamTest.kt | 8 ++++---- test/kotlin/integration/DrainConnectionsTest.kt | 4 ++-- test/kotlin/integration/GRPCReceiveErrorTest.kt | 10 +++++----- test/kotlin/integration/ReceiveDataTest.kt | 2 +- test/kotlin/integration/ReceiveErrorTest.kt | 10 +++++----- test/kotlin/integration/SendDataTest.kt | 2 +- test/kotlin/integration/SendHeadersTest.kt | 2 +- test/kotlin/integration/SendTrailersTest.kt | 2 +- test/kotlin/integration/StreamIdleTimeoutTest.kt | 8 ++++---- 24 files changed, 67 insertions(+), 70 deletions(-) diff --git a/examples/java/hello_world/MainActivity.java b/examples/java/hello_world/MainActivity.java index cf121e56eb..5d1ebcd4ac 100644 --- a/examples/java/hello_world/MainActivity.java +++ b/examples/java/hello_world/MainActivity.java @@ -125,7 +125,7 @@ private void makeRequest() { } return Unit.INSTANCE; }) - .setOnError((error, ignored, also_ignored) -> { + .setOnError((error, ignored) -> { String message = "failed with error after " + error.getAttemptCount() + " attempts: " + error.getMessage(); Log.d("MainActivity", message); diff --git a/examples/kotlin/hello_world/AsyncDemoFilter.kt b/examples/kotlin/hello_world/AsyncDemoFilter.kt index 72f6ab8463..a45ec5f7af 100644 --- a/examples/kotlin/hello_world/AsyncDemoFilter.kt +++ b/examples/kotlin/hello_world/AsyncDemoFilter.kt @@ -81,16 +81,15 @@ class AsyncDemoFilter : AsyncResponseFilter { @Suppress("EmptyFunctionBlock") override fun onError( error: EnvoyError, - streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel ) { } @Suppress("EmptyFunctionBlock") - override fun onCancel(streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) { + override fun onCancel(finalStreamIntel: FinalStreamIntel) { } @Suppress("EmptyFunctionBlock") - override fun onComplete(streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) { + override fun onComplete(finalStreamIntel: FinalStreamIntel) { } } diff --git a/examples/kotlin/hello_world/BufferDemoFilter.kt b/examples/kotlin/hello_world/BufferDemoFilter.kt index dee61511e5..f158d1b1e6 100644 --- a/examples/kotlin/hello_world/BufferDemoFilter.kt +++ b/examples/kotlin/hello_world/BufferDemoFilter.kt @@ -59,16 +59,15 @@ class BufferDemoFilter : ResponseFilter { @Suppress("EmptyFunctionBlock") override fun onError( error: EnvoyError, - streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel ) { } @Suppress("EmptyFunctionBlock") - override fun onCancel(streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) { + override fun onCancel(finalStreamIntel: FinalStreamIntel) { } @Suppress("EmptyFunctionBlock") - override fun onComplete(streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) { + override fun onComplete(finalStreamIntel: FinalStreamIntel) { } } diff --git a/examples/kotlin/hello_world/DemoFilter.kt b/examples/kotlin/hello_world/DemoFilter.kt index f9c005f997..2f231fd130 100644 --- a/examples/kotlin/hello_world/DemoFilter.kt +++ b/examples/kotlin/hello_world/DemoFilter.kt @@ -43,17 +43,16 @@ class DemoFilter : ResponseFilter { override fun onError( error: EnvoyError, - streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel ) { Log.d("DemoFilter", "On error!") } - override fun onCancel(streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) { + override fun onCancel(finalStreamIntel: FinalStreamIntel) { Log.d("DemoFilter", "On cancel!") } @Suppress("EmptyFunctionBlock") - override fun onComplete(streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) { + override fun onComplete(finalStreamIntel: FinalStreamIntel) { } } diff --git a/examples/kotlin/hello_world/MainActivity.kt b/examples/kotlin/hello_world/MainActivity.kt index 7fc060ee25..d58c7b40c1 100644 --- a/examples/kotlin/hello_world/MainActivity.kt +++ b/examples/kotlin/hello_world/MainActivity.kt @@ -135,7 +135,7 @@ class MainActivity : Activity() { recyclerView.post { viewAdapter.add(Failure(message)) } } } - .setOnError { error, _, _ -> + .setOnError { error, _ -> val attemptCount = error.attemptCount ?: -1 val message = "failed with error after $attemptCount attempts: ${error.message}" Log.d("MainActivity", message) diff --git a/library/kotlin/io/envoyproxy/envoymobile/FinalStreamIntel.kt b/library/kotlin/io/envoyproxy/envoymobile/FinalStreamIntel.kt index 5375146578..9ceb9db423 100644 --- a/library/kotlin/io/envoyproxy/envoymobile/FinalStreamIntel.kt +++ b/library/kotlin/io/envoyproxy/envoymobile/FinalStreamIntel.kt @@ -1,6 +1,7 @@ package io.envoyproxy.envoymobile import io.envoyproxy.envoymobile.engine.types.EnvoyFinalStreamIntel +import io.envoyproxy.envoymobile.engine.types.EnvoyStreamIntel /** * Exposes one time HTTP stream metrics, context, and other details. @@ -29,6 +30,9 @@ import io.envoyproxy.envoymobile.engine.types.EnvoyFinalStreamIntel */ @Suppress("LongParameterList") class FinalStreamIntel constructor( + streamId: Long, + connectionId: Long, + attemptCount: Long, val requestStartMs: Long, val dnsStartMs: Long, val dnsEndMs: Long, @@ -43,8 +47,9 @@ class FinalStreamIntel constructor( val socketReused: Boolean, val sentByteCount: Long, val receivedByteCount: Long -) { - constructor(base: EnvoyFinalStreamIntel) : this( +) : StreamIntel(streamId, connectionId, attemptCount) { + constructor(superBase: EnvoyStreamIntel, base: EnvoyFinalStreamIntel) : this( + superBase.streamId, superBase.connectionId, superBase.attemptCount, base.requestStartMs, base.dnsStartMs, base.dnsEndMs, base.connectStartMs, base.connectEndMs, base.sslStartMs, diff --git a/library/kotlin/io/envoyproxy/envoymobile/StreamCallbacks.kt b/library/kotlin/io/envoyproxy/envoymobile/StreamCallbacks.kt index 43e3251a9d..9bd34c3579 100644 --- a/library/kotlin/io/envoyproxy/envoymobile/StreamCallbacks.kt +++ b/library/kotlin/io/envoyproxy/envoymobile/StreamCallbacks.kt @@ -18,12 +18,12 @@ internal class StreamCallbacks { )? = null var onData: ((data: ByteBuffer, endStream: Boolean, streamIntel: StreamIntel) -> Unit)? = null var onTrailers: ((trailers: ResponseTrailers, streamIntel: StreamIntel) -> Unit)? = null - var onCancel: ((streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) -> Unit)? = null + var onCancel: ((finalStreamIntel: FinalStreamIntel) -> Unit)? = null var onError: ( - (error: EnvoyError, streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) -> Unit + (error: EnvoyError, finalStreamIntel: FinalStreamIntel) -> Unit )? = null var onSendWindowAvailable: ((streamIntel: StreamIntel) -> Unit)? = null - var onComplete: ((streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) -> Unit)? = null + var onComplete: ((finalStreamIntel: FinalStreamIntel) -> Unit)? = null } /** @@ -63,13 +63,12 @@ internal class EnvoyHTTPCallbacksAdapter( ) { callbacks.onError?.invoke( EnvoyError(errorCode, message, attemptCount), - StreamIntel(streamIntel), - FinalStreamIntel(finalStreamIntel) + FinalStreamIntel(streamIntel, finalStreamIntel) ) } override fun onCancel(streamIntel: EnvoyStreamIntel, finalStreamIntel: EnvoyFinalStreamIntel) { - callbacks.onCancel?.invoke(StreamIntel(streamIntel), FinalStreamIntel(finalStreamIntel)) + callbacks.onCancel?.invoke(FinalStreamIntel(streamIntel, finalStreamIntel)) } override fun onSendWindowAvailable(streamIntel: EnvoyStreamIntel) { @@ -77,6 +76,6 @@ internal class EnvoyHTTPCallbacksAdapter( } override fun onComplete(streamIntel: EnvoyStreamIntel, finalStreamIntel: EnvoyFinalStreamIntel) { - callbacks.onComplete?.invoke(StreamIntel(streamIntel), FinalStreamIntel(finalStreamIntel)) + callbacks.onComplete?.invoke(FinalStreamIntel(streamIntel, finalStreamIntel)) } } diff --git a/library/kotlin/io/envoyproxy/envoymobile/StreamIntel.kt b/library/kotlin/io/envoyproxy/envoymobile/StreamIntel.kt index f5c78549ee..29238d361b 100644 --- a/library/kotlin/io/envoyproxy/envoymobile/StreamIntel.kt +++ b/library/kotlin/io/envoyproxy/envoymobile/StreamIntel.kt @@ -9,7 +9,7 @@ import io.envoyproxy.envoymobile.engine.types.EnvoyStreamIntel * @param attemptCount The number of internal attempts to carry out a request/operation. 0 if * not set. */ -class StreamIntel constructor( +open class StreamIntel constructor( val streamId: Long, val connectionId: Long, val attemptCount: Long diff --git a/library/kotlin/io/envoyproxy/envoymobile/StreamPrototype.kt b/library/kotlin/io/envoyproxy/envoymobile/StreamPrototype.kt index cfca4cdb93..748351905a 100644 --- a/library/kotlin/io/envoyproxy/envoymobile/StreamPrototype.kt +++ b/library/kotlin/io/envoyproxy/envoymobile/StreamPrototype.kt @@ -113,7 +113,6 @@ open class StreamPrototype(private val engine: EnvoyEngine) { fun setOnError( closure: ( error: EnvoyError, - streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel ) -> Unit ): StreamPrototype { @@ -129,7 +128,7 @@ open class StreamPrototype(private val engine: EnvoyEngine) { * @return This stream, for chaining syntax. */ fun setOnComplete( - closure: (streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) -> Unit + closure: (finalStreamIntel: FinalStreamIntel) -> Unit ): StreamPrototype { callbacks.onComplete = closure return this @@ -143,7 +142,7 @@ open class StreamPrototype(private val engine: EnvoyEngine) { * @return This stream, for chaining syntax. */ fun setOnCancel( - closure: (streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) -> Unit + closure: (finalStreamIntel: FinalStreamIntel) -> Unit ): StreamPrototype { callbacks.onCancel = closure return this diff --git a/library/kotlin/io/envoyproxy/envoymobile/filters/Filter.kt b/library/kotlin/io/envoyproxy/envoymobile/filters/Filter.kt index 1741b85f2b..d83845ff94 100644 --- a/library/kotlin/io/envoyproxy/envoymobile/filters/Filter.kt +++ b/library/kotlin/io/envoyproxy/envoymobile/filters/Filter.kt @@ -102,19 +102,19 @@ internal class EnvoyHTTPFilterAdapter( override fun onError(errorCode: Int, message: String, attemptCount: Int, streamIntel: EnvoyStreamIntel, finalStreamIntel: EnvoyFinalStreamIntel) { (filter as? ResponseFilter)?.let { responseFilter -> - responseFilter.onError(EnvoyError(errorCode, message, attemptCount), StreamIntel(streamIntel), FinalStreamIntel(finalStreamIntel)) + responseFilter.onError(EnvoyError(errorCode, message, attemptCount), FinalStreamIntel(streamIntel, finalStreamIntel)) } } override fun onCancel(streamIntel: EnvoyStreamIntel, finalStreamIntel: EnvoyFinalStreamIntel) { (filter as? ResponseFilter)?.let { responseFilter -> - responseFilter.onCancel(StreamIntel(streamIntel), FinalStreamIntel(finalStreamIntel)) + responseFilter.onCancel(FinalStreamIntel(streamIntel, finalStreamIntel)) } } override fun onComplete(streamIntel: EnvoyStreamIntel, finalStreamIntel: EnvoyFinalStreamIntel) { (filter as? ResponseFilter)?.let { responseFilter -> - responseFilter.onComplete(StreamIntel(streamIntel), FinalStreamIntel(finalStreamIntel)) + responseFilter.onComplete(FinalStreamIntel(streamIntel, finalStreamIntel)) } } diff --git a/library/kotlin/io/envoyproxy/envoymobile/filters/ResponseFilter.kt b/library/kotlin/io/envoyproxy/envoymobile/filters/ResponseFilter.kt index 41991cb44a..fa7ce9a5ee 100644 --- a/library/kotlin/io/envoyproxy/envoymobile/filters/ResponseFilter.kt +++ b/library/kotlin/io/envoyproxy/envoymobile/filters/ResponseFilter.kt @@ -55,10 +55,9 @@ interface ResponseFilter : Filter { * `stopIteration{...}`. * * @param error: The error that occurred within Envoy. - * @param streamIntel: Internal HTTP stream metrics, context, and other details. * @param finalStreamIntel: Final internal HTTP stream metrics, context, and other details. */ - fun onError(error: EnvoyError, streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) + fun onError(error: EnvoyError, finalStreamIntel: FinalStreamIntel) /** * Called at most once when the client cancels the stream. @@ -67,10 +66,9 @@ interface ResponseFilter : Filter { * This should be considered a terminal state, and invalidates any previous attempts to * `stopIteration{...}`. * - * @param streamIntel: Internal HTTP stream metrics, context, and other details. * @param finalStreamIntel: Final internal HTTP stream metrics, context, and other details. */ - fun onCancel(streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) + fun onCancel(finalStreamIntel: FinalStreamIntel) /** * Called at most once when the stream completes gracefully. @@ -79,8 +77,7 @@ interface ResponseFilter : Filter { * This should be considered a terminal state, and invalidates any previous attempts to * `stopIteration{...}`. * - * @param streamIntel: Internal HTTP stream metrics, context, and other details. * @param finalStreamIntel: Final internal HTTP stream metrics, context, and other details. */ - fun onComplete(streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) + fun onComplete(finalStreamIntel: FinalStreamIntel) } diff --git a/library/kotlin/io/envoyproxy/envoymobile/grpc/GRPCStreamPrototype.kt b/library/kotlin/io/envoyproxy/envoymobile/grpc/GRPCStreamPrototype.kt index 6192c7d4d3..7b35796c52 100644 --- a/library/kotlin/io/envoyproxy/envoymobile/grpc/GRPCStreamPrototype.kt +++ b/library/kotlin/io/envoyproxy/envoymobile/grpc/GRPCStreamPrototype.kt @@ -89,7 +89,7 @@ class GRPCStreamPrototype( * @return This stream, for chaining syntax. */ fun setOnError( - closure: (error: EnvoyError, streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) -> Unit + closure: (error: EnvoyError, finalStreamIntel: FinalStreamIntel) -> Unit ): GRPCStreamPrototype { underlyingStream.setOnError(closure) return this @@ -103,7 +103,7 @@ class GRPCStreamPrototype( * @return This stream, for chaining syntax. */ fun setOnCancel( - closure: (streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) -> Unit + closure: (finalStreamIntel: FinalStreamIntel) -> Unit ): GRPCStreamPrototype { underlyingStream.setOnCancel(closure) return this diff --git a/test/java/integration/AndroidEnvoyExplicitFlowTest.java b/test/java/integration/AndroidEnvoyExplicitFlowTest.java index fc8cd5c4f1..09f2e50675 100644 --- a/test/java/integration/AndroidEnvoyExplicitFlowTest.java +++ b/test/java/integration/AndroidEnvoyExplicitFlowTest.java @@ -500,22 +500,22 @@ private Response sendRequest(RequestScenario requestScenario) throws Exception { response.get().addStreamIntel(streamIntel); return null; }) - .setOnError((error, streamIntel, finalStreamIntel) -> { + .setOnError((error, finalStreamIntel) -> { response.get().setEnvoyError(error); - response.get().addStreamIntel(streamIntel); + response.get().addStreamIntel(finalStreamIntel); response.get().setFinalStreamIntel(finalStreamIntel); latch.countDown(); return null; }) - .setOnCancel((streamIntel, finalStreamIntel) -> { + .setOnCancel((finalStreamIntel) -> { response.get().setCancelled(); - response.get().addStreamIntel(streamIntel); + response.get().addStreamIntel(finalStreamIntel); response.get().setFinalStreamIntel(finalStreamIntel); latch.countDown(); return null; }) - .setOnComplete((streamIntel, finalStreamIntel) -> { - response.get().addStreamIntel(streamIntel); + .setOnComplete((finalStreamIntel) -> { + response.get().addStreamIntel(finalStreamIntel); response.get().setFinalStreamIntel(finalStreamIntel); latch.countDown(); return null; diff --git a/test/java/integration/AndroidEnvoyFlowTest.java b/test/java/integration/AndroidEnvoyFlowTest.java index a1a57e8904..7d1bf250c9 100644 --- a/test/java/integration/AndroidEnvoyFlowTest.java +++ b/test/java/integration/AndroidEnvoyFlowTest.java @@ -302,12 +302,12 @@ private Response sendRequest(RequestScenario requestScenario) throws Exception { latch.countDown(); return null; }) - .setOnError((error, ignored, also_ignored) -> { + .setOnError((error, ignored) -> { response.get().setEnvoyError(error); latch.countDown(); return null; }) - .setOnCancel((ignored, also_ignored) -> { + .setOnCancel((ignored) -> { response.get().setCancelled(); latch.countDown(); return null; diff --git a/test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServerTest.java b/test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServerTest.java index 982b4f5a21..4a50ef8414 100644 --- a/test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServerTest.java +++ b/test/java/io/envoyproxy/envoymobile/engine/testing/QuicTestServerTest.java @@ -157,17 +157,17 @@ private QuicTestServerTest.Response sendRequest(RequestScenario requestScenario) response.get().setTrailers(trailers); return null; }) - .setOnError((error, ignored1, ignored2) -> { + .setOnError((error, ignored) -> { response.get().setEnvoyError(error); latch.countDown(); return null; }) - .setOnCancel((ignored1, ignored2) -> { + .setOnCancel((ignored) -> { response.get().setCancelled(); latch.countDown(); return null; }) - .setOnComplete((ignored1, ignored2) -> { + .setOnComplete((ignored) -> { latch.countDown(); return null; }) diff --git a/test/kotlin/integration/CancelStreamTest.kt b/test/kotlin/integration/CancelStreamTest.kt index 31321010fd..9964f4770e 100644 --- a/test/kotlin/integration/CancelStreamTest.kt +++ b/test/kotlin/integration/CancelStreamTest.kt @@ -109,10 +109,10 @@ class CancelStreamTest { return FilterTrailersStatus.Continue(trailers) } - override fun onError(error: EnvoyError, streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) {} - override fun onComplete(streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) {} + override fun onError(error: EnvoyError, finalStreamIntel: FinalStreamIntel) {} + override fun onComplete(finalStreamIntel: FinalStreamIntel) {} - override fun onCancel(streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) { + override fun onCancel(finalStreamIntel: FinalStreamIntel) { latch.countDown() } } @@ -139,7 +139,7 @@ class CancelStreamTest { .build() client.newStreamPrototype() - .setOnCancel { _, _ -> + .setOnCancel { _ -> runExpectation.countDown() } .start(Executors.newSingleThreadExecutor()) diff --git a/test/kotlin/integration/DrainConnectionsTest.kt b/test/kotlin/integration/DrainConnectionsTest.kt index f43c04a2a4..597c659120 100644 --- a/test/kotlin/integration/DrainConnectionsTest.kt +++ b/test/kotlin/integration/DrainConnectionsTest.kt @@ -85,7 +85,7 @@ class DrainConnectionsTest { resultEndStream1 = endStream headersExpectation.countDown() } - .setOnError { _, _, _ -> fail("Unexpected error") } + .setOnError { _, _ -> fail("Unexpected error") } .start() .sendHeaders(requestHeaders, true) @@ -101,7 +101,7 @@ class DrainConnectionsTest { resultEndStream2 = endStream headersExpectation.countDown() } - .setOnError { _, _, _ -> fail("Unexpected error") } + .setOnError { _, _ -> fail("Unexpected error") } .start() .sendHeaders(requestHeaders, true) diff --git a/test/kotlin/integration/GRPCReceiveErrorTest.kt b/test/kotlin/integration/GRPCReceiveErrorTest.kt index e83c87382f..8da673736f 100644 --- a/test/kotlin/integration/GRPCReceiveErrorTest.kt +++ b/test/kotlin/integration/GRPCReceiveErrorTest.kt @@ -96,12 +96,12 @@ class GRPCReceiveErrorTest { return FilterTrailersStatus.Continue(trailers) } - override fun onError(error: EnvoyError, streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) { + override fun onError(error: EnvoyError, finalStreamIntel: FinalStreamIntel) { receivedError.countDown() } - override fun onComplete(streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) {} + override fun onComplete(finalStreamIntel: FinalStreamIntel) {} - override fun onCancel(streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) { + override fun onCancel(finalStreamIntel: FinalStreamIntel) { notCancelled.countDown() } } @@ -126,10 +126,10 @@ class GRPCReceiveErrorTest { .newGRPCStreamPrototype() .setOnResponseHeaders { _, _, _ -> } .setOnResponseMessage { _, _ -> } - .setOnError { _, _, _ -> + .setOnError { _, _ -> callbackReceivedError.countDown() } - .setOnCancel { _, _ -> + .setOnCancel { _ -> fail("Unexpected call to onCancel response callback") } .start() diff --git a/test/kotlin/integration/ReceiveDataTest.kt b/test/kotlin/integration/ReceiveDataTest.kt index 36702adbf5..4059e14587 100644 --- a/test/kotlin/integration/ReceiveDataTest.kt +++ b/test/kotlin/integration/ReceiveDataTest.kt @@ -94,7 +94,7 @@ class ReceiveDataTest { body = data dataExpectation.countDown() } - .setOnError { _, _, _ -> fail("Unexpected error") } + .setOnError { _, _ -> fail("Unexpected error") } .start() .sendHeaders(requestHeaders, true) diff --git a/test/kotlin/integration/ReceiveErrorTest.kt b/test/kotlin/integration/ReceiveErrorTest.kt index b7f61b239f..0e71279f51 100644 --- a/test/kotlin/integration/ReceiveErrorTest.kt +++ b/test/kotlin/integration/ReceiveErrorTest.kt @@ -93,12 +93,12 @@ class ReceiveErrorTest { return FilterTrailersStatus.Continue(trailers) } - override fun onError(error: EnvoyError, streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) { + override fun onError(error: EnvoyError, finalStreamIntel: FinalStreamIntel) { receivedError.countDown() } - override fun onComplete(streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) {} + override fun onComplete(finalStreamIntel: FinalStreamIntel) {} - override fun onCancel(streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) { + override fun onCancel(finalStreamIntel: FinalStreamIntel) { notCancelled.countDown() } } @@ -127,11 +127,11 @@ class ReceiveErrorTest { .setOnResponseData { _, _, _ -> fail("Data received instead of expected error") } // The unmatched expectation will cause a local reply which gets translated in Envoy Mobile to // an error. - .setOnError { error, _, _ -> + .setOnError { error, _ -> errorCode = error.errorCode callbackReceivedError.countDown() } - .setOnCancel { _, _ -> + .setOnCancel { _ -> fail("Unexpected call to onCancel response callback") } .start() diff --git a/test/kotlin/integration/SendDataTest.kt b/test/kotlin/integration/SendDataTest.kt index 1fe9075a8a..925e9f3bea 100644 --- a/test/kotlin/integration/SendDataTest.kt +++ b/test/kotlin/integration/SendDataTest.kt @@ -93,7 +93,7 @@ class SendDataTest { responseHeadersEndStream = endStream expectation.countDown() } - .setOnError { _, _, _ -> + .setOnError { _, _ -> fail("Unexpected error") } .start() diff --git a/test/kotlin/integration/SendHeadersTest.kt b/test/kotlin/integration/SendHeadersTest.kt index 823a524d7a..20cdb99f01 100644 --- a/test/kotlin/integration/SendHeadersTest.kt +++ b/test/kotlin/integration/SendHeadersTest.kt @@ -85,7 +85,7 @@ class SendHeadersTest { resultEndStream = endStream headersExpectation.countDown() } - .setOnError { _, _, _ -> fail("Unexpected error") } + .setOnError { _, _ -> fail("Unexpected error") } .start() .sendHeaders(requestHeaders, true) diff --git a/test/kotlin/integration/SendTrailersTest.kt b/test/kotlin/integration/SendTrailersTest.kt index 1391ced880..1392583e19 100644 --- a/test/kotlin/integration/SendTrailersTest.kt +++ b/test/kotlin/integration/SendTrailersTest.kt @@ -98,7 +98,7 @@ class SendTrailersTest { responseStatus = headers.httpStatus expectation.countDown() } - .setOnError { _, _, _ -> + .setOnError { _, _ -> fail("Unexpected error") } .start() diff --git a/test/kotlin/integration/StreamIdleTimeoutTest.kt b/test/kotlin/integration/StreamIdleTimeoutTest.kt index 8d1117961f..4b787242dc 100644 --- a/test/kotlin/integration/StreamIdleTimeoutTest.kt +++ b/test/kotlin/integration/StreamIdleTimeoutTest.kt @@ -132,13 +132,13 @@ class CancelStreamTest { return FilterTrailersStatus.StopIteration() } - override fun onError(error: EnvoyError, streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) { + override fun onError(error: EnvoyError, finalStreamIntel: FinalStreamIntel) { assertThat(error.errorCode).isEqualTo(4) latch.countDown() } - override fun onComplete(streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) {} + override fun onComplete(finalStreamIntel: FinalStreamIntel) {} - override fun onCancel(streamIntel: StreamIntel, finalStreamIntel: FinalStreamIntel) { + override fun onCancel(finalStreamIntel: FinalStreamIntel) { fail("Unexpected call to onCancel filter callback") } } @@ -165,7 +165,7 @@ class CancelStreamTest { .build() client.newStreamPrototype() - .setOnError { error, _, _ -> + .setOnError { error, _ -> assertThat(error.errorCode).isEqualTo(4) callbackExpectation.countDown() } From c99f784d110eb36166bb8d0772eb0466ab11b2ce Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Wed, 26 Jan 2022 12:17:18 -0500 Subject: [PATCH 14/21] fix (#2025) Signed-off-by: Alyssa Wilk --- .github/actions/pr_notifier/pr_notifier.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/pr_notifier/pr_notifier.py b/.github/actions/pr_notifier/pr_notifier.py index 7efbecea70..2662fe9608 100644 --- a/.github/actions/pr_notifier/pr_notifier.py +++ b/.github/actions/pr_notifier/pr_notifier.py @@ -170,5 +170,5 @@ def post_to_oncall(client, out_slo_prs): sys.exit(1) client = WebClient(token=SLACK_BOT_TOKEN) - post_to_oncall(client, reviewers_and_messages['unassigned'], stalled_prs) + post_to_oncall(client, stalled_prs) post_to_assignee(client, reviewers_and_messages, REVIEWERS) From cef4ec0a29d3adf09a0df68da3d2aa8c988a7335 Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Wed, 26 Jan 2022 14:29:51 -0500 Subject: [PATCH 15/21] final_intel: adding response flags (#2009) Risk Level: low Testing: Docs Changes: inline Part of #1594 Signed-off-by: Alyssa Wilk --- docs/root/intro/version_history.rst | 1 + .../extensions/filters/http/platform_bridge/filter.cc | 3 ++- library/common/http/client.h | 4 ++-- library/common/jni/jni_utility.cc | 3 ++- library/common/stream_info/extra_stream_info.cc | 1 + library/common/types/c_types.h | 4 ++++ .../envoymobile/engine/EnvoyFinalStreamIntelImpl.java | 6 ++++++ .../envoymobile/engine/types/EnvoyFinalStreamIntel.java | 6 ++++++ .../kotlin/io/envoyproxy/envoymobile/FinalStreamIntel.kt | 6 ++++-- .../kotlin/io/envoyproxy/envoymobile/mocks/MockStream.kt | 1 + library/swift/FinalStreamIntel.swift | 9 +++++++-- 11 files changed, 36 insertions(+), 8 deletions(-) diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index 0547e8bc7b..bbfbe8a55d 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -7,6 +7,7 @@ Pending Release Bugfixes: Features: +- API: added Envoy's response flags to final stream intel (:issue:`#2009 <2009>`) 0.4.5 (January 13, 2022) ======================== diff --git a/library/common/extensions/filters/http/platform_bridge/filter.cc b/library/common/extensions/filters/http/platform_bridge/filter.cc index 24cc048b14..3a0742a26d 100644 --- a/library/common/extensions/filters/http/platform_bridge/filter.cc +++ b/library/common/extensions/filters/http/platform_bridge/filter.cc @@ -192,7 +192,8 @@ Http::LocalErrorStatus PlatformBridgeFilter::onLocalReply(const LocalReplyData& envoy_final_stream_intel PlatformBridgeFilter::finalStreamIntel() { RELEASE_ASSERT(decoder_callbacks_, "StreamInfo accessed before filter callbacks are set"); // FIXME: Stream handle cannot currently be set from the filter context. - envoy_final_stream_intel final_stream_intel{-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 0, 0, 0}; + envoy_final_stream_intel final_stream_intel{-1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, 0, 0, 0, 0}; setFinalStreamIntel(decoder_callbacks_->streamInfo(), final_stream_intel); return final_stream_intel; } diff --git a/library/common/http/client.h b/library/common/http/client.h index b07ad397df..0246333a10 100644 --- a/library/common/http/client.h +++ b/library/common/http/client.h @@ -280,8 +280,8 @@ class Client : public Logger::Loggable { bool explicit_flow_control_ = false; // Latest intel data retrieved from the StreamInfo. envoy_stream_intel stream_intel_{-1, -1, 0}; - envoy_final_stream_intel envoy_final_stream_intel_{-1, -1, -1, -1, -1, -1, -1, - -1, -1, -1, -1, 0, 0, 0}; + envoy_final_stream_intel envoy_final_stream_intel_{-1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, 0, 0, 0, 0}; StreamInfo::BytesMeterSharedPtr bytes_meter_; }; diff --git a/library/common/jni/jni_utility.cc b/library/common/jni/jni_utility.cc index 7362673d59..1cd4eaba0c 100644 --- a/library/common/jni/jni_utility.cc +++ b/library/common/jni/jni_utility.cc @@ -99,7 +99,7 @@ jlongArray native_stream_intel_to_array(JNIEnv* env, envoy_stream_intel stream_i jlongArray native_final_stream_intel_to_array(JNIEnv* env, envoy_final_stream_intel final_stream_intel) { - jlongArray j_array = env->NewLongArray(14); + jlongArray j_array = env->NewLongArray(15); jlong* critical_array = static_cast(env->GetPrimitiveArrayCritical(j_array, nullptr)); RELEASE_ASSERT(critical_array != nullptr, "unable to allocate memory in jni_utility"); @@ -117,6 +117,7 @@ jlongArray native_final_stream_intel_to_array(JNIEnv* env, critical_array[11] = static_cast(final_stream_intel.socket_reused); critical_array[12] = static_cast(final_stream_intel.sent_byte_count); critical_array[13] = static_cast(final_stream_intel.received_byte_count); + critical_array[14] = static_cast(final_stream_intel.response_flags); // Here '0' (for which there is no named constant) indicates we want to commit the changes back // to the JVM and free the c array, where applicable. diff --git a/library/common/stream_info/extra_stream_info.cc b/library/common/stream_info/extra_stream_info.cc index 0bd2f05ec7..7404b47910 100644 --- a/library/common/stream_info/extra_stream_info.cc +++ b/library/common/stream_info/extra_stream_info.cc @@ -52,6 +52,7 @@ void setFinalStreamIntel(StreamInfo& stream_info, envoy_final_stream_intel& fina final_intel.sent_byte_count = stream_info.getUpstreamBytesMeter()->wireBytesSent(); final_intel.received_byte_count = stream_info.getUpstreamBytesMeter()->wireBytesReceived(); } + final_intel.response_flags = stream_info.responseFlags(); } } // namespace StreamInfo diff --git a/library/common/types/c_types.h b/library/common/types/c_types.h index 72fac8dbca..d664795885 100644 --- a/library/common/types/c_types.h +++ b/library/common/types/c_types.h @@ -193,6 +193,10 @@ typedef struct { uint64_t sent_byte_count; // The number of bytes received from upstream. uint64_t received_byte_count; + // The final response flags for the stream. See + // https://github.com/envoyproxy/envoy/blob/main/envoy/stream_info/stream_info.h + // for the ResponseFlag enum. + uint64_t response_flags; } envoy_final_stream_intel; #ifdef __cplusplus diff --git a/library/java/io/envoyproxy/envoymobile/engine/EnvoyFinalStreamIntelImpl.java b/library/java/io/envoyproxy/envoymobile/engine/EnvoyFinalStreamIntelImpl.java index 050cede043..f10f67a6ed 100644 --- a/library/java/io/envoyproxy/envoymobile/engine/EnvoyFinalStreamIntelImpl.java +++ b/library/java/io/envoyproxy/envoymobile/engine/EnvoyFinalStreamIntelImpl.java @@ -17,6 +17,7 @@ class EnvoyFinalStreamIntelImpl implements EnvoyFinalStreamIntel { private boolean socketReused; private long sentByteCount; private long receivedByteCount; + private long responseFlags; EnvoyFinalStreamIntelImpl(long[] values) { requestStartMs = values[0]; @@ -33,6 +34,7 @@ class EnvoyFinalStreamIntelImpl implements EnvoyFinalStreamIntel { socketReused = values[11] != 0; sentByteCount = values[12]; receivedByteCount = values[13]; + responseFlags = values[14]; } @Override @@ -91,4 +93,8 @@ public long getSentByteCount() { public long getReceivedByteCount() { return receivedByteCount; } + @Override + public long getResponseFlags() { + return responseFlags; + } } diff --git a/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyFinalStreamIntel.java b/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyFinalStreamIntel.java index 4936176a56..15c9f8795c 100644 --- a/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyFinalStreamIntel.java +++ b/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyFinalStreamIntel.java @@ -64,4 +64,10 @@ public interface EnvoyFinalStreamIntel { * The number of bytes received from upstream. */ public long getReceivedByteCount(); + /* + * The response flags for the stream. See + * https://github.com/envoyproxy/envoy/blob/main/envoy/stream_info/stream_info.h#L39 + * for values. + */ + public long getResponseFlags(); } diff --git a/library/kotlin/io/envoyproxy/envoymobile/FinalStreamIntel.kt b/library/kotlin/io/envoyproxy/envoymobile/FinalStreamIntel.kt index 9ceb9db423..deb9c7cffb 100644 --- a/library/kotlin/io/envoyproxy/envoymobile/FinalStreamIntel.kt +++ b/library/kotlin/io/envoyproxy/envoymobile/FinalStreamIntel.kt @@ -27,6 +27,7 @@ import io.envoyproxy.envoymobile.engine.types.EnvoyStreamIntel * @param socket_reused True if the upstream socket had been used previously. * @param sentByteCount The number of bytes sent upstream. * @param receivedByteCount The number of bytes received from upstream. + * @param responseFlags The response flags for the stream. */ @Suppress("LongParameterList") class FinalStreamIntel constructor( @@ -46,7 +47,8 @@ class FinalStreamIntel constructor( val requestEndMs: Long, val socketReused: Boolean, val sentByteCount: Long, - val receivedByteCount: Long + val receivedByteCount: Long, + val responseFlags: Long ) : StreamIntel(streamId, connectionId, attemptCount) { constructor(superBase: EnvoyStreamIntel, base: EnvoyFinalStreamIntel) : this( superBase.streamId, superBase.connectionId, superBase.attemptCount, @@ -57,6 +59,6 @@ class FinalStreamIntel constructor( base.sendingEndMs, base.responseStartMs, base.requestEndMs, base.socketReused, base.sentByteCount, - base.receivedByteCount + base.receivedByteCount, base.responseFlags ) } diff --git a/library/kotlin/io/envoyproxy/envoymobile/mocks/MockStream.kt b/library/kotlin/io/envoyproxy/envoymobile/mocks/MockStream.kt index 7f326d22ab..67cf7990bb 100644 --- a/library/kotlin/io/envoyproxy/envoymobile/mocks/MockStream.kt +++ b/library/kotlin/io/envoyproxy/envoymobile/mocks/MockStream.kt @@ -32,6 +32,7 @@ class MockStream internal constructor(underlyingStream: MockEnvoyHTTPStream) : S override fun getSocketReused(): Boolean { return false } override fun getSentByteCount(): Long { return 0 } override fun getReceivedByteCount(): Long { return 0 } + override fun getResponseFlags(): Long { return 0 } } /** * Closure that will be called when request headers are sent. diff --git a/library/swift/FinalStreamIntel.swift b/library/swift/FinalStreamIntel.swift index 970a919e28..bcf3fdf354 100644 --- a/library/swift/FinalStreamIntel.swift +++ b/library/swift/FinalStreamIntel.swift @@ -33,6 +33,8 @@ public final class FinalStreamIntel: StreamIntel { public let sentByteCount: UInt64 /// The number of bytes received from upstream. public let receivedByteCount: UInt64 + /// The response flags for the upstream stream. + public let responseFlags: UInt64 // NOTE(1): These fields may not be set if socket_reused is false. @@ -53,7 +55,8 @@ public final class FinalStreamIntel: StreamIntel { requestEndMs: Int64, socketReused: Bool, sentByteCount: UInt64, - receivedByteCount: UInt64 + receivedByteCount: UInt64, + responseFlags: UInt64 ) { self.requestStartMs = requestStartMs self.dnsStartMs = dnsStartMs @@ -69,6 +72,7 @@ public final class FinalStreamIntel: StreamIntel { self.socketReused = socketReused self.sentByteCount = sentByteCount self.receivedByteCount = receivedByteCount + self.responseFlags = responseFlags super.init(streamId: streamId, connectionId: connectionId, attemptCount: attemptCount) } } @@ -92,7 +96,8 @@ extension FinalStreamIntel { requestEndMs: cFinalIntel.request_end_ms, socketReused: cFinalIntel.socket_reused != 0, sentByteCount: cFinalIntel.sent_byte_count, - receivedByteCount: cFinalIntel.received_byte_count + receivedByteCount: cFinalIntel.received_byte_count, + responseFlags: cFinalIntel.response_flags ) } } From 957bf7e58c4ba0c3d324576ea356c42f0a084349 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Thu, 27 Jan 2022 13:11:37 -0500 Subject: [PATCH 16/21] Run Kotlin macOS tests without EngFlow (#2018) These jobs fail when run with remote execution but pass when run directly on GitHub Actions agents. Signed-off-by: JP Simard --- .github/workflows/android_tests.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/android_tests.yml b/.github/workflows/android_tests.yml index 6b16e0e5af..fd2a732499 100644 --- a/.github/workflows/android_tests.yml +++ b/.github/workflows/android_tests.yml @@ -45,8 +45,6 @@ jobs: ./bazelw test \ --test_output=all \ --build_tests_only \ - --config=remote-ci-macos \ - --remote_header="Authorization=Bearer $GITHUB_TOKEN" \ //test/kotlin/io/... javatestsmac: name: java_tests_mac From 0fbe18a5cd961f609aff820a3753d4fd6c9f615e Mon Sep 17 00:00:00 2001 From: Charles Le Borgne <80125532+carloseltuerto@users.noreply.github.com> Date: Fri, 28 Jan 2022 00:16:48 +0000 Subject: [PATCH 17/21] Enable the "received byte count" feature. (#2004) Description: Report stream_intel_.received_byte_count to the Java Layer. Risk Level: Small Testing: Tests added for C++, tests enabled in Java, CI Docs Changes: N/A Release Notes: N/A Fixes: 1426 Signed-off-by: Charles Le Borgne --- .../filters/http/platform_bridge/filter.cc | 2 +- library/common/http/client.cc | 30 +++++--- library/common/http/client.h | 2 +- library/common/jni/jni_utility.cc | 3 +- library/common/types/c_types.h | 6 ++ .../engine/EnvoyStreamIntelImpl.java | 7 ++ .../engine/types/EnvoyFinalStreamIntel.java | 2 + .../engine/types/EnvoyStreamIntel.java | 11 +++ .../chromium/net/impl/CronetUrlRequest.java | 75 ++++++++++++++----- .../net/impl/CronetUrlRequestContext.java | 4 +- .../envoymobile/mocks/MockStream.kt | 1 + .../integration/client_integration_test.cc | 12 ++- .../chromium/net/CronetUrlRequestTest.java | 23 +++--- .../chromium/net/testing/CronetTestRule.java | 5 +- 14 files changed, 132 insertions(+), 51 deletions(-) diff --git a/library/common/extensions/filters/http/platform_bridge/filter.cc b/library/common/extensions/filters/http/platform_bridge/filter.cc index 3a0742a26d..cae954db13 100644 --- a/library/common/extensions/filters/http/platform_bridge/filter.cc +++ b/library/common/extensions/filters/http/platform_bridge/filter.cc @@ -202,7 +202,7 @@ envoy_stream_intel PlatformBridgeFilter::streamIntel() { RELEASE_ASSERT(decoder_callbacks_, "StreamInfo accessed before filter callbacks are set"); auto& info = decoder_callbacks_->streamInfo(); // FIXME: Stream handle cannot currently be set from the filter context. - envoy_stream_intel stream_intel{-1, -1, 0}; + envoy_stream_intel stream_intel{-1, -1, 0, 0}; if (info.upstreamInfo()) { stream_intel.connection_id = info.upstreamInfo()->upstreamConnectionId().value_or(-1); } diff --git a/library/common/http/client.cc b/library/common/http/client.cc index 0095bd57c9..d18262cf82 100644 --- a/library/common/http/client.cc +++ b/library/common/http/client.cc @@ -41,23 +41,27 @@ void Client::DirectStreamCallbacks::encodeHeaders(const ResponseHeaderMap& heade ASSERT(http_client_.getStream(direct_stream_.stream_handle_, GetStreamFilters::ALLOW_FOR_ALL_STREAMS)); - direct_stream_.saveLatestStreamIntel(); - if (end_stream) { - closeStream(); - } + // Capture some metadata before potentially closing the stream. absl::string_view alpn = ""; - uint64_t response_status = Utility::getResponseStatus(headers); - if (direct_stream_.request_decoder_ && - direct_stream_.request_decoder_->streamInfo().upstreamInfo() && - direct_stream_.request_decoder_->streamInfo().upstreamInfo()->upstreamSslConnection()) { - alpn = direct_stream_.request_decoder_->streamInfo() - .upstreamInfo() - ->upstreamSslConnection() - ->alpn(); + if (direct_stream_.request_decoder_) { + direct_stream_.saveLatestStreamIntel(); + const auto& info = direct_stream_.request_decoder_->streamInfo(); + // Set the initial number of bytes consumed for the non terminal callbacks. + direct_stream_.stream_intel_.consumed_bytes_from_response = + info.getUpstreamBytesMeter() ? info.getUpstreamBytesMeter()->headerBytesReceived() : 0; + // Capture the alpn if available. + if (info.upstreamInfo() && info.upstreamInfo()->upstreamSslConnection()) { + alpn = info.upstreamInfo()->upstreamSslConnection()->alpn(); + } + } + + if (end_stream) { + closeStream(); } // Track success for later bookkeeping (stream could still be reset). + uint64_t response_status = Utility::getResponseStatus(headers); success_ = CodeUtility::is2xx(response_status); ENVOY_LOG(debug, "[S{}] dispatching to platform response headers for stream (end_stream={}):\n{}", @@ -123,6 +127,8 @@ void Client::DirectStreamCallbacks::sendDataToBridge(Buffer::Instance& data, boo // Cap by bytes_to_send_ if and only if applying explicit flow control. uint32_t bytes_to_send = calculateBytesToSend(data, bytes_to_send_); + // Update the number of bytes consumed by this non terminal callback. + direct_stream_.stream_intel_.consumed_bytes_from_response += bytes_to_send; // Only send end stream if all data is being sent. bool send_end_stream = end_stream && (bytes_to_send == data.length()); diff --git a/library/common/http/client.h b/library/common/http/client.h index 0246333a10..e66c065628 100644 --- a/library/common/http/client.h +++ b/library/common/http/client.h @@ -279,7 +279,7 @@ class Client : public Logger::Loggable { // read faster than the mobile caller can process it. bool explicit_flow_control_ = false; // Latest intel data retrieved from the StreamInfo. - envoy_stream_intel stream_intel_{-1, -1, 0}; + envoy_stream_intel stream_intel_{-1, -1, 0, 0}; envoy_final_stream_intel envoy_final_stream_intel_{-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 0, 0, 0, 0}; StreamInfo::BytesMeterSharedPtr bytes_meter_; diff --git a/library/common/jni/jni_utility.cc b/library/common/jni/jni_utility.cc index 1cd4eaba0c..712269fb18 100644 --- a/library/common/jni/jni_utility.cc +++ b/library/common/jni/jni_utility.cc @@ -85,12 +85,13 @@ jbyteArray native_data_to_array(JNIEnv* env, envoy_data data) { } jlongArray native_stream_intel_to_array(JNIEnv* env, envoy_stream_intel stream_intel) { - jlongArray j_array = env->NewLongArray(3); + jlongArray j_array = env->NewLongArray(4); jlong* critical_array = static_cast(env->GetPrimitiveArrayCritical(j_array, nullptr)); RELEASE_ASSERT(critical_array != nullptr, "unable to allocate memory in jni_utility"); critical_array[0] = static_cast(stream_intel.stream_id); critical_array[1] = static_cast(stream_intel.connection_id); critical_array[2] = static_cast(stream_intel.attempt_count); + critical_array[3] = static_cast(stream_intel.consumed_bytes_from_response); // Here '0' (for which there is no named constant) indicates we want to commit the changes back // to the JVM and free the c array, where applicable. env->ReleasePrimitiveArrayCritical(j_array, critical_array, 0); diff --git a/library/common/types/c_types.h b/library/common/types/c_types.h index d664795885..11b0d9e735 100644 --- a/library/common/types/c_types.h +++ b/library/common/types/c_types.h @@ -153,6 +153,12 @@ typedef struct { int64_t connection_id; // The number of internal attempts to carry out a request/operation. 0 if not present. uint64_t attempt_count; + // Number of bytes consumed by the non terminal callbacks out of the response. + // NOTE: on terminal callbacks (on_complete, on_error_, on_cancel), this value will not be equal + // to envoy_final_stream_intel.received_byte_count. The latter represents the real number + // of bytes received before decompression. consumed_bytes_from_response omits the number + // number of bytes related to the Status Line, and is after decompression. + uint64_t consumed_bytes_from_response; } envoy_stream_intel; /** diff --git a/library/java/io/envoyproxy/envoymobile/engine/EnvoyStreamIntelImpl.java b/library/java/io/envoyproxy/envoymobile/engine/EnvoyStreamIntelImpl.java index 3e6f35a5d8..d96d311bc2 100644 --- a/library/java/io/envoyproxy/envoymobile/engine/EnvoyStreamIntelImpl.java +++ b/library/java/io/envoyproxy/envoymobile/engine/EnvoyStreamIntelImpl.java @@ -6,11 +6,13 @@ class EnvoyStreamIntelImpl implements EnvoyStreamIntel { private long streamId; private long connectionId; private long attemptCount; + private long consumedBytesFromResponse; EnvoyStreamIntelImpl(long[] values) { streamId = values[0]; connectionId = values[1]; attemptCount = values[2]; + consumedBytesFromResponse = values[3]; } @Override @@ -27,4 +29,9 @@ public long getConnectionId() { public long getAttemptCount() { return attemptCount; } + + @Override + public long getConsumedBytesFromResponse() { + return consumedBytesFromResponse; + } } diff --git a/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyFinalStreamIntel.java b/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyFinalStreamIntel.java index 15c9f8795c..50c3f621bf 100644 --- a/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyFinalStreamIntel.java +++ b/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyFinalStreamIntel.java @@ -2,6 +2,8 @@ /** * Exposes internal HTTP stream metrics, context, and other details sent once on stream end. + * + * Note: a value of -1 means "not present" for any field where the name is suffixed with "Ms". */ public interface EnvoyFinalStreamIntel { /* diff --git a/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyStreamIntel.java b/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyStreamIntel.java index 5a8a305350..71f119bf71 100644 --- a/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyStreamIntel.java +++ b/library/java/io/envoyproxy/envoymobile/engine/types/EnvoyStreamIntel.java @@ -4,6 +4,7 @@ * Exposes internal HTTP stream metrics, context, and other details. */ public interface EnvoyStreamIntel { + /** * An internal identifier for the stream. */ @@ -18,4 +19,14 @@ public interface EnvoyStreamIntel { * The number of internal attempts to carry out a request/operation. */ public long getAttemptCount(); + + /** + * The number of bytes consumed by the non terminal callbacks, from the response. + * + *

>NOTE: on terminal callbacks (on_complete, on_error_, on_cancel), this value will not be + * equal to {@link EnvoyFinalStreamIntel#getReceivedByteCount()}. The latter represents the real + * number of bytes received before decompression. getConsumedBytesFromResponse() omits the number + * number of bytes related to the Status Line, and is after decompression. + */ + public long getConsumedBytesFromResponse(); } diff --git a/library/java/org/chromium/net/impl/CronetUrlRequest.java b/library/java/org/chromium/net/impl/CronetUrlRequest.java index 4190214389..787e51c857 100644 --- a/library/java/org/chromium/net/impl/CronetUrlRequest.java +++ b/library/java/org/chromium/net/impl/CronetUrlRequest.java @@ -16,6 +16,7 @@ import java.util.AbstractMap; import java.util.AbstractMap.SimpleEntry; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; @@ -29,6 +30,7 @@ import org.chromium.net.CallbackException; import org.chromium.net.CronetException; import org.chromium.net.InlineExecutionProhibitedException; +import org.chromium.net.RequestFinishedInfo; import org.chromium.net.UploadDataProvider; /** UrlRequest, backed by Envoy-Mobile. */ @@ -88,7 +90,7 @@ public final class CronetUrlRequest extends UrlRequestBase { private final String mUserAgent; private final HeadersList mRequestHeaders = new HeadersList(); - private final List mUrlChain = new ArrayList<>(); + private final Collection mRequestAnnotations; private final CronetUrlRequestContext mRequestContext; private final AtomicBoolean mWaitingOnRedirect = new AtomicBoolean(false); private final AtomicBoolean mWaitingOnRead = new AtomicBoolean(false); @@ -111,11 +113,16 @@ public final class CronetUrlRequest extends UrlRequestBase { /* These don't change with redirects */ private String mInitialMethod; - private CronetUploadDataStream mUploadDataStream; private final Executor mUserExecutor; private final VersionSafeCallbacks.UrlRequestCallback mCallback; + private final String mInitialUrl; + private final VersionSafeCallbacks.RequestFinishedInfoListener mRequestFinishedListener; private final ConditionVariable mStartBlock = new ConditionVariable(); + private CronetUploadDataStream mUploadDataStream; + + private volatile CronetException mException; + /** * Holds a subset of StatusValues - {@link State#STARTED} can represent {@link * Status#SENDING_REQUEST} or {@link Status#WAITING_FOR_RESPONSE}. While the distinction isn't @@ -127,13 +134,16 @@ public final class CronetUrlRequest extends UrlRequestBase { * only used with the STARTED state, so it's inconsequential. */ @StatusValues private volatile int mAdditionalStatusDetails = Status.INVALID; - private final AtomicReference mError = new AtomicReference<>(); /* These change with redirects. */ private final AtomicReference mStream = new AtomicReference<>(); + private final List mUrlChain = new ArrayList<>(); + private EnvoyFinalStreamIntel mEnvoyFinalStreamIntel; + private long mBytesReceivedFromRedirects = 0; + private long mBytesReceivedFromLastRedirect = 0; private CronvoyHttpCallbacks mCronvoyCallbacks; private String mCurrentUrl; - private UrlResponseInfoImpl mUrlResponseInfo; + private volatile UrlResponseInfoImpl mUrlResponseInfo; private String mPendingRedirectUrl; /** @@ -142,8 +152,9 @@ public final class CronetUrlRequest extends UrlRequestBase { */ CronetUrlRequest(CronetUrlRequestContext cronvoyEngine, Callback callback, Executor executor, String url, String userAgent, boolean allowDirectExecutor, - boolean trafficStatsTagSet, int trafficStatsTag, boolean trafficStatsUidSet, - int trafficStatsUid) { + Collection connectionAnnotations, boolean trafficStatsTagSet, + int trafficStatsTag, boolean trafficStatsUidSet, int trafficStatsUid, + RequestFinishedInfo.Listener requestFinishedListener) { if (url == null) { throw new NullPointerException("URL is required"); } @@ -154,11 +165,17 @@ public final class CronetUrlRequest extends UrlRequestBase { throw new NullPointerException("Executor is required"); } mCallback = new VersionSafeCallbacks.UrlRequestCallback(callback); + mRequestFinishedListener = + requestFinishedListener != null + ? new VersionSafeCallbacks.RequestFinishedInfoListener(requestFinishedListener) + : null; mRequestContext = cronvoyEngine; mAllowDirectExecutor = allowDirectExecutor; mUserExecutor = executor; + mInitialUrl = url; mCurrentUrl = url; mUserAgent = userAgent; + mRequestAnnotations = connectionAnnotations; } @Override @@ -305,8 +322,8 @@ public boolean isDone() { @Override public void getStatus(StatusListener listener) { + @StatusValues int extraStatus = mAdditionalStatusDetails; @State int state = mState.get(); - int extraStatus = mAdditionalStatusDetails; @StatusValues final int status; switch (state) { @@ -401,7 +418,6 @@ private static int determineNextErrorState(boolean streamEnded, @State int origi } private void enterErrorState(CronetException error) { - mError.compareAndSet(null, error); @State int originalState; @State int updatedState; do { @@ -414,6 +430,7 @@ private void enterErrorState(CronetException error) { if (isTerminalState(originalState)) { return; } + mException = error; fireCloseUploadDataProvider(); if (updatedState == State.ERROR_PENDING_CANCEL) { CronvoyHttpCallbacks cronvoyCallbacks = this.mCronvoyCallbacks; @@ -469,12 +486,17 @@ private void fireCloseUploadDataProvider() { } } + // This method is only called when in STARTED state. This means a "cancel" request won't be + // executed immediately - that quite important here, otherwise this would lead to unfortunate + // race conditions. A "cancel" request will then be honnored on the first callback. private void fireOpenConnection() { if (mInitialMethod == null) { mInitialMethod = "GET"; } + mUrlResponseInfo = null; + mEnvoyFinalStreamIntel = null; + mBytesReceivedFromRedirects += mBytesReceivedFromLastRedirect; mAdditionalStatusDetails = Status.CONNECTING; - mUrlResponseInfo = new UrlResponseInfoImpl(); mUrlChain.add(mCurrentUrl); Map> envoyRequestHeaders = buildEnvoyRequestHeaders(mInitialMethod, mRequestHeaders, mUploadDataStream, mUserAgent, @@ -568,7 +590,7 @@ public void run() { try { mCallback.onCanceled(CronetUrlRequest.this, mUrlResponseInfo); } catch (Exception exception) { - Log.e(TAG, "Exception in onCanceled method", exception); + Log.e(CronetUrlRequestContext.LOG_TAG, "Exception in onCanceled method", exception); } } }; @@ -582,7 +604,7 @@ public void run() { try { mCallback.onSucceeded(CronetUrlRequest.this, mUrlResponseInfo); } catch (Exception exception) { - Log.e(TAG, "Exception in onSucceeded method", exception); + Log.e(CronetUrlRequestContext.LOG_TAG, "Exception in onSucceeded method", exception); } } }; @@ -594,9 +616,9 @@ void onFailed() { @Override public void run() { try { - mCallback.onFailed(CronetUrlRequest.this, mUrlResponseInfo, mError.get()); + mCallback.onFailed(CronetUrlRequest.this, mUrlResponseInfo, mException); } catch (Exception exception) { - Log.e(TAG, "Exception in onFailed method", exception); + Log.e(CronetUrlRequestContext.LOG_TAG, "Exception in onFailed method", exception); } } }; @@ -631,6 +653,19 @@ private boolean streamEnded() { return cronvoyCallbacks != null && cronvoyCallbacks.mEndStream; } + private void recordEnvoyFinalStreamIntel(EnvoyFinalStreamIntel envoyFinalStreamIntel) { + mEnvoyFinalStreamIntel = envoyFinalStreamIntel; + if (mUrlResponseInfo != null) { // Null if cancelled before receiving a Response. + mUrlResponseInfo.setReceivedByteCount(envoyFinalStreamIntel.getReceivedByteCount() + + mBytesReceivedFromRedirects); + } + } + + private void recordEnvoyStreamIntel(EnvoyStreamIntel envoyStreamIntel) { + mUrlResponseInfo.setReceivedByteCount(envoyStreamIntel.getConsumedBytesFromResponse() + + mBytesReceivedFromRedirects); + } + private static class HeadersList extends ArrayList> {} private static class DirectExecutor implements Executor { @@ -666,9 +701,8 @@ public Executor getExecutor() { @Override public void onHeaders(Map> headers, boolean endStream, EnvoyStreamIntel streamIntel) { - if (isAbandoned()) { - return; - } + mUrlResponseInfo = new UrlResponseInfoImpl(); + recordEnvoyStreamIntel(streamIntel); mEndStream = endStream; List statuses = headers.get(":status"); final int responseCode = @@ -698,6 +732,7 @@ public void onHeaders(Map> headers, boolean endStream, } if (locationField != null) { + mBytesReceivedFromLastRedirect = streamIntel.getConsumedBytesFromResponse(); cancel(); // Abort the the original request - we are being redirected. } @@ -734,6 +769,7 @@ public void onData(ByteBuffer data, boolean endStream, EnvoyStreamIntel streamIn if (isAbandoned()) { return; } + recordEnvoyStreamIntel(streamIntel); mEndStream = endStream; @State int originalState; @State int updatedState; @@ -789,6 +825,7 @@ public void onError(int errorCode, String message, int attemptCount, if (isAbandoned()) { return; } + recordEnvoyFinalStreamIntel(finalStreamIntel); mEndStream = true; @State int originalState; @State int updatedState; @@ -811,6 +848,7 @@ public void onCancel(EnvoyStreamIntel streamIntel, EnvoyFinalStreamIntel finalSt if (isAbandoned()) { return; } + recordEnvoyFinalStreamIntel(finalStreamIntel); mEndStream = true; @State int originalState; @State int updatedState; @@ -852,7 +890,7 @@ public void onComplete(EnvoyStreamIntel streamIntel, EnvoyFinalStreamIntel final if (isAbandoned()) { return; } - mUrlResponseInfo.setReceivedByteCount(finalStreamIntel.getSentByteCount()); + recordEnvoyFinalStreamIntel(finalStreamIntel); if (successReady(SucceededState.ON_COMPLETE_RECEIVED)) { onSucceeded(); } @@ -900,7 +938,7 @@ void readData(int size) { */ void cancel() { EnvoyHTTPStream stream = mStream.get(); - if (isAbandoned() || mEndStream) { + if (this != mCronvoyCallbacks || mEndStream) { return; } @CancelState int oldState = mCancelState.getAndSet(CancelState.CANCELLED); @@ -936,6 +974,7 @@ private void setUrlResponseInfo(Map> responseHeaders, int r // TODO(https://github.com/envoyproxy/envoy-mobile/issues/1426) set receivedByteCount // TODO(https://github.com/envoyproxy/envoy-mobile/issues/1622) support proxy // TODO(https://github.com/envoyproxy/envoy-mobile/issues/1546) negotiated protocol + // TODO(https://github.com/envoyproxy/envoy-mobile/issues/1578) http caching mUrlResponseInfo.setResponseValues( new ArrayList<>(mUrlChain), responseCode, HttpReason.getReason(responseCode), Collections.unmodifiableList(headerList), false, selectedTransport, ":0"); diff --git a/library/java/org/chromium/net/impl/CronetUrlRequestContext.java b/library/java/org/chromium/net/impl/CronetUrlRequestContext.java index d12b41a955..00e9f3a226 100644 --- a/library/java/org/chromium/net/impl/CronetUrlRequestContext.java +++ b/library/java/org/chromium/net/impl/CronetUrlRequestContext.java @@ -126,8 +126,8 @@ void setTaskToExecuteWhenInitializationIsCompleted(Runnable runnable) { int trafficStatsUid, RequestFinishedInfo.Listener requestFinishedListener, int idempotency) { return new CronetUrlRequest(this, callback, executor, url, mUserAgent, allowDirectExecutor, - trafficStatsTagSet, trafficStatsTag, trafficStatsUidSet, - trafficStatsUid); + connectionAnnotations, trafficStatsTagSet, trafficStatsTag, + trafficStatsUidSet, trafficStatsUid, requestFinishedListener); } @Override diff --git a/library/kotlin/io/envoyproxy/envoymobile/mocks/MockStream.kt b/library/kotlin/io/envoyproxy/envoymobile/mocks/MockStream.kt index 67cf7990bb..d85673347f 100644 --- a/library/kotlin/io/envoyproxy/envoymobile/mocks/MockStream.kt +++ b/library/kotlin/io/envoyproxy/envoymobile/mocks/MockStream.kt @@ -15,6 +15,7 @@ class MockStream internal constructor(underlyingStream: MockEnvoyHTTPStream) : S override fun getStreamId(): Long { return 0 } override fun getConnectionId(): Long { return 0 } override fun getAttemptCount(): Long { return 0 } + override fun getConsumedBytesFromResponse(): Long { return 0 } } private val mockFinalStreamIntel = object : EnvoyFinalStreamIntel { diff --git a/test/common/integration/client_integration_test.cc b/test/common/integration/client_integration_test.cc index 6e104cd2bb..0425f915a9 100644 --- a/test/common/integration/client_integration_test.cc +++ b/test/common/integration/client_integration_test.cc @@ -36,6 +36,8 @@ typedef struct { uint32_t on_complete_calls; uint32_t on_error_calls; uint32_t on_cancel_calls; + uint64_t on_header_consumed_bytes_from_response; + uint64_t on_complete_received_byte_count; std::string status; ConditionalInitializer* terminal_callback; } callbacks_called; @@ -62,12 +64,13 @@ class ClientIntegrationTest : public BaseIntegrationTest, }); bridge_callbacks_.context = &cc_; - bridge_callbacks_.on_headers = [](envoy_headers c_headers, bool, envoy_stream_intel, + bridge_callbacks_.on_headers = [](envoy_headers c_headers, bool, envoy_stream_intel intel, void* context) -> void* { Http::ResponseHeaderMapPtr response_headers = toResponseHeaders(c_headers); callbacks_called* cc_ = static_cast(context); cc_->on_headers_calls++; cc_->status = response_headers->Status()->value().getStringView(); + cc_->on_header_consumed_bytes_from_response = intel.consumed_bytes_from_response; return nullptr; }; bridge_callbacks_.on_data = [](envoy_data c_data, bool, envoy_stream_intel, @@ -77,9 +80,10 @@ class ClientIntegrationTest : public BaseIntegrationTest, release_envoy_data(c_data); return nullptr; }; - bridge_callbacks_.on_complete = [](envoy_stream_intel, envoy_final_stream_intel, + bridge_callbacks_.on_complete = [](envoy_stream_intel, envoy_final_stream_intel final_intel, void* context) -> void* { callbacks_called* cc_ = static_cast(context); + cc_->on_complete_received_byte_count = final_intel.received_byte_count; cc_->on_complete_calls++; cc_->terminal_callback->setReady(); return nullptr; @@ -143,7 +147,7 @@ name: api_listener Http::ClientPtr http_client_{}; envoy_http_callbacks bridge_callbacks_; ConditionalInitializer terminal_callback_; - callbacks_called cc_ = {0, 0, 0, 0, 0, "", &terminal_callback_}; + callbacks_called cc_ = {0, 0, 0, 0, 0, 0, 0, "", &terminal_callback_}; }; INSTANTIATE_TEST_SUITE_P(IpVersions, ClientIntegrationTest, @@ -206,6 +210,8 @@ TEST_P(ClientIntegrationTest, Basic) { ASSERT_EQ(cc_.status, "200"); ASSERT_EQ(cc_.on_data_calls, 2); ASSERT_EQ(cc_.on_complete_calls, 1); + ASSERT_EQ(cc_.on_header_consumed_bytes_from_response, 27); + ASSERT_EQ(cc_.on_complete_received_byte_count, 67); // stream_success gets charged for 2xx status codes. test_server_->waitForCounterEq("http.client.stream_success", 1); diff --git a/test/java/org/chromium/net/CronetUrlRequestTest.java b/test/java/org/chromium/net/CronetUrlRequestTest.java index 61e9249813..414d19d96a 100644 --- a/test/java/org/chromium/net/CronetUrlRequestTest.java +++ b/test/java/org/chromium/net/CronetUrlRequestTest.java @@ -220,7 +220,7 @@ public void testRedirectAsync() throws Exception { "header-value"); UrlResponseInfo expected = createUrlResponseInfo( - new String[] {NativeTestServer.getRedirectURL()}, "Found", 302, 73, "Content-Length", "92", + new String[] {NativeTestServer.getRedirectURL()}, "Found", 302, 72, "Content-Length", "92", "Location", "/success.txt", "redirect-header", "header-value"); mTestRule.assertResponseEquals(expected, callback.mRedirectResponseInfoList.get(0)); @@ -266,9 +266,10 @@ public void testRedirectAsync() throws Exception { assertEquals(ResponseStep.ON_SUCCEEDED, callback.mResponseStep); assertEquals(NativeTestServer.SUCCESS_BODY, callback.mResponseAsString); + // Original bytesReceived: 258 UrlResponseInfo urlResponseInfo = createUrlResponseInfo( new String[] {NativeTestServer.getRedirectURL(), NativeTestServer.getSuccessURL()}, "OK", - 200, 258, "Content-Length", "20", "Content-Type", "text/plain", + 200, 284, "Content-Length", "20", "Content-Type", "text/plain", "Access-Control-Allow-Origin", "*", "header-name", "header-value", "multi-header-name", "header-value1", "multi-header-name", "header-value2"); @@ -666,17 +667,19 @@ public void testMockMultiRedirect() throws Exception { assertEquals(2, callback.mRedirectResponseInfoList.size()); // Check first redirect (multiredirect.html -> redirect.html) + // Original receivedBytes: 76 UrlResponseInfo firstExpectedResponseInfo = createUrlResponseInfo( - new String[] {NativeTestServer.getMultiRedirectURL()}, "Found", 302, 76, "Content-Length", + new String[] {NativeTestServer.getMultiRedirectURL()}, "Found", 302, 75, "Content-Length", "92", "Location", "/redirect.html", "redirect-header0", "header-value"); UrlResponseInfo firstRedirectResponseInfo = callback.mRedirectResponseInfoList.get(0); mTestRule.assertResponseEquals(firstExpectedResponseInfo, firstRedirectResponseInfo); // Check second redirect (redirect.html -> success.txt) + // Original receivedBytes: 334 UrlResponseInfo secondExpectedResponseInfo = createUrlResponseInfo( new String[] {NativeTestServer.getMultiRedirectURL(), NativeTestServer.getRedirectURL(), NativeTestServer.getSuccessURL()}, - "OK", 200, 334, "Content-Length", "20", "Content-Type", "text/plain", + "OK", 200, 359, "Content-Length", "20", "Content-Type", "text/plain", "Access-Control-Allow-Origin", "*", "header-name", "header-value", "multi-header-name", "header-value1", "multi-header-name", "header-value2"); @@ -693,7 +696,7 @@ public void testMockNotFound() throws Exception { TestUrlRequestCallback callback = startAndWaitForComplete(NativeTestServer.getNotFoundURL()); UrlResponseInfo expected = createUrlResponseInfo(new String[] {NativeTestServer.getNotFoundURL()}, "Not Found", 404, - 140, "Content-Length", "96"); + 142, "Content-Length", "96"); mTestRule.assertResponseEquals(expected, callback.mResponseInfo); assertTrue(callback.mHttpResponseDataLength != 0); assertEquals(0, callback.mRedirectCount); @@ -2137,26 +2140,26 @@ public void testLegacyOnFailedCallback() throws Exception { final ConditionVariable done = new ConditionVariable(); UrlRequest.Callback callback = new UrlRequest.Callback() { @Override - public void onRedirectReceived(UrlRequest request, UrlResponseInfo info, - String newLocationUrl) { + public void onSucceeded(UrlRequest request, UrlResponseInfo info) { failedExpectation.set(true); fail(); } @Override - public void onResponseStarted(UrlRequest request, UrlResponseInfo info) { + public void onRedirectReceived(UrlRequest request, UrlResponseInfo info, + String newLocationUrl) { failedExpectation.set(true); fail(); } @Override - public void onReadCompleted(UrlRequest request, UrlResponseInfo info, ByteBuffer byteBuffer) { + public void onResponseStarted(UrlRequest request, UrlResponseInfo info) { failedExpectation.set(true); fail(); } @Override - public void onSucceeded(UrlRequest request, UrlResponseInfo info) { + public void onReadCompleted(UrlRequest request, UrlResponseInfo info, ByteBuffer byteBuffer) { failedExpectation.set(true); fail(); } diff --git a/test/java/org/chromium/net/testing/CronetTestRule.java b/test/java/org/chromium/net/testing/CronetTestRule.java index c25e46a45d..b15eece987 100644 --- a/test/java/org/chromium/net/testing/CronetTestRule.java +++ b/test/java/org/chromium/net/testing/CronetTestRule.java @@ -68,7 +68,7 @@ public static class CronetTestFramework { private static ExperimentalCronetEngine createEngine(Context context) { ExperimentalCronetEngine.Builder builder = new ExperimentalCronetEngine.Builder(context); - ((CronetEngineBuilderImpl)builder.getBuilderDelegate()).setLogLevel("info"); + ((CronetEngineBuilderImpl)builder.getBuilderDelegate()).setLogLevel("warning"); return builder.enableQuic(true).build(); } @@ -253,8 +253,7 @@ public void assertResponseEquals(UrlResponseInfo expected, UrlResponseInfo actua assertEquals(expected.getUrl(), actual.getUrl()); // Transferred bytes and proxy server are not supported in pure java if (!testingJavaImpl()) { - // TODO("https://github.com/envoyproxy/envoy-mobile/issues/1426"): uncomment the assert - // assertEquals(expected.getReceivedByteCount(), actual.getReceivedByteCount()); + assertEquals(expected.getReceivedByteCount(), actual.getReceivedByteCount()); assertEquals(expected.getProxyServer(), actual.getProxyServer()); // This is a place where behavior intentionally differs between native and java assertEquals(expected.getNegotiatedProtocol(), actual.getNegotiatedProtocol()); From 696dc2d6d8ebd59deb9c40edb57697a52298fce8 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 1 Feb 2022 14:10:39 -0500 Subject: [PATCH 18/21] [json] Remove com_github_nlohmann_json override & patch (#2030) The patch is no longer needed as it was similarly patched upstream in nlohmann/json#3101. That patch was then shipped in v3.10.5, which is the version used in Envoy now. Signed-off-by: JP Simard --- bazel/envoy_mobile_repositories.bzl | 12 -------- bazel/json.patch | 48 ----------------------------- 2 files changed, 60 deletions(-) delete mode 100644 bazel/json.patch diff --git a/bazel/envoy_mobile_repositories.bzl b/bazel/envoy_mobile_repositories.bzl index bc5897e029..3681f54e5b 100644 --- a/bazel/envoy_mobile_repositories.bzl +++ b/bazel/envoy_mobile_repositories.bzl @@ -69,18 +69,6 @@ def upstream_envoy_overrides(): urls = ["https://github.com/bazelbuild/rules_python/archive/6f37aa9966f53e063c41b7509a386d53a9f156c3.tar.gz"], ) - http_archive( - name = "com_github_nlohmann_json", - # 3.10.4 introduced incompatible changes with Envoy Mobile. Until Envoy Mobile updates it's - # minimum iOS version to 13+ this dependency needs to be patched. - patches = ["@envoy_mobile//bazel:json.patch"], - patch_args = ["-p1"], - sha256 = "1155fd1a83049767360e9a120c43c578145db3204d2b309eba49fbbedd0f4ed3", - strip_prefix = "json-3.10.4", - urls = ["https://github.com/nlohmann/json/archive/v3.10.4.tar.gz"], - build_file = "@envoy//bazel/external:json.BUILD", - ) - def swift_repos(): http_archive( name = "build_bazel_rules_apple", diff --git a/bazel/json.patch b/bazel/json.patch deleted file mode 100644 index e09a72875a..0000000000 --- a/bazel/json.patch +++ /dev/null @@ -1,48 +0,0 @@ -diff --git a/include/nlohmann/detail/conversions/from_json.hpp b/include/nlohmann/detail/conversions/from_json.hpp -index 71e32aaf7..6a4575ecb 100644 ---- a/include/nlohmann/detail/conversions/from_json.hpp -+++ b/include/nlohmann/detail/conversions/from_json.hpp -@@ -448,7 +448,7 @@ void from_json(const BasicJsonType& j, std::unordered_map= 130000)) - template - void from_json(const BasicJsonType& j, std::filesystem::path& p) - { -diff --git a/include/nlohmann/detail/conversions/to_json.hpp b/include/nlohmann/detail/conversions/to_json.hpp -index 79fdb3233..6a4aa7ff3 100644 ---- a/include/nlohmann/detail/conversions/to_json.hpp -+++ b/include/nlohmann/detail/conversions/to_json.hpp -@@ -391,7 +391,7 @@ void to_json(BasicJsonType& j, const T& t) - to_json_tuple_impl(j, t, make_index_sequence::value> {}); - } - --#ifdef JSON_HAS_CPP_17 -+#if defined(JSON_HAS_CPP_17) && (defined(__APPLE__) && (defined(__ENVIRONMENT_IPHONE_OS_VERSION_MIN_REQUIRED__) && __ENVIRONMENT_IPHONE_OS_VERSION_MIN_REQUIRED__ >= 130000)) - template - void to_json(BasicJsonType& j, const std::filesystem::path& p) - { -diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp -index 87475ab31..c999784a2 100644 ---- a/single_include/nlohmann/json.hpp -+++ b/single_include/nlohmann/json.hpp -@@ -4379,7 +4379,7 @@ void from_json(const BasicJsonType& j, std::unordered_map= 130000)) - template - void from_json(const BasicJsonType& j, std::filesystem::path& p) - { -@@ -5002,7 +5002,7 @@ void to_json(BasicJsonType& j, const T& t) - to_json_tuple_impl(j, t, make_index_sequence::value> {}); - } - --#ifdef JSON_HAS_CPP_17 -+#if defined(JSON_HAS_CPP_17) && (defined(__APPLE__) && (defined(__ENVIRONMENT_IPHONE_OS_VERSION_MIN_REQUIRED__) && __ENVIRONMENT_IPHONE_OS_VERSION_MIN_REQUIRED__ >= 130000)) - template - void to_json(BasicJsonType& j, const std::filesystem::path& p) - { From 5ea738a119c8186090a74ca5a3da71d6aa87435e Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 1 Feb 2022 17:13:00 -0500 Subject: [PATCH 19/21] [deps] Update rules_android to a stable release URL (#2032) Previous URL started returning a zip archive with a different checksum: https://github.com/bazelbuild/rules_android/issues/43 Signed-off-by: JP Simard --- bazel/envoy_mobile_repositories.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bazel/envoy_mobile_repositories.bzl b/bazel/envoy_mobile_repositories.bzl index 3681f54e5b..c6776df942 100644 --- a/bazel/envoy_mobile_repositories.bzl +++ b/bazel/envoy_mobile_repositories.bzl @@ -143,7 +143,7 @@ def kotlin_repos(): def android_repos(): http_archive( name = "build_bazel_rules_android", - urls = ["https://github.com/bazelbuild/rules_android/archive/v0.1.1.zip"], + urls = ["https://github.com/bazelbuild/rules_android/archive/refs/tags/v0.1.1.zip"], sha256 = "cd06d15dd8bb59926e4d65f9003bfc20f9da4b2519985c27e190cddc8b7a7806", strip_prefix = "rules_android-0.1.1", ) From 687371aeda1fafacdbb2f05081968c3b21fefbb0 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 1 Feb 2022 17:38:11 -0500 Subject: [PATCH 20/21] [OWNERS] Update Envoy Mobile owners (#2031) I'm adding myself as an owner and moving Michael Rebello to a new "Emiritus maintainers" section to match the structure and naming used by the Envoy project. Signed-off-by: JP Simard --- OWNERS.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/OWNERS.md b/OWNERS.md index 46bf5a324a..ffb21a1b07 100644 --- a/OWNERS.md +++ b/OWNERS.md @@ -11,10 +11,10 @@ routing PRs, questions, etc. to the right place. * Envoy mobile core, C bridge layer, C++ extensions, build system, Envoy upstream. * Snow Pettersen ([snowp](https://github.com/snowp)) (aickck@gmail.com) * Envoy Upstream. Stats. -* Michael Rebello ([rebello95](https://github.com/rebello95)) (mrebello@lyft.com) - * iOS (swift/objective-c) platform bindings. * Mike Schore ([goaway](https://github.com/goaway)) (mschore@lyft.com) * General. +* JP Simard ([jpsim](https://github.com/jpsim)) (jp@lyft.com) + * iOS (swift/objective-c) platform bindings. # Friends of Envoy Mobile @@ -31,3 +31,7 @@ matter expert reviews. Feel free to loop them in as needed. * Cronet API shim aka "Cronvoy". * Keith Smiley ([keith](https://github.com/keith)) (keithbsmiley@gmail.com) * Bazel build magician + +# Emeritus maintainers + +* Michael Rebello ([rebello95](https://github.com/rebello95)) (mrebello@lyft.com) From b1b8a295b881f79d665006deac8a78a4f8d06ed9 Mon Sep 17 00:00:00 2001 From: alyssawilk Date: Thu, 3 Feb 2022 08:29:05 -0500 Subject: [PATCH 21/21] owners: adding charles (#2038) Signed-off-by: Alyssa Wilk --- OWNERS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/OWNERS.md b/OWNERS.md index ffb21a1b07..25fe157dee 100644 --- a/OWNERS.md +++ b/OWNERS.md @@ -15,6 +15,8 @@ routing PRs, questions, etc. to the right place. * General. * JP Simard ([jpsim](https://github.com/jpsim)) (jp@lyft.com) * iOS (swift/objective-c) platform bindings. +* Charles Le Borgne ([carloseltuerto](http://github.com/carloseltuerto)) (cleborgne@google.com) + * Cronvoy. # Friends of Envoy Mobile