Skip to content

Commit

Permalink
Merge branch 'envoyproxy:main' into fix_async_tcp_client
Browse files Browse the repository at this point in the history
  • Loading branch information
ohadvano authored Jul 31, 2024
2 parents d51122c + b0cc284 commit 26e2439
Show file tree
Hide file tree
Showing 41 changed files with 263 additions and 570 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,5 @@ api_proto_package(
deps = [
"//envoy/config/core/v3:pkg",
"@com_github_cncf_xds//udpa/annotations:pkg",
"@com_github_cncf_xds//xds/annotations/v3:pkg",
],
)
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ package envoy.extensions.quic.server_preferred_address.v3;

import "envoy/config/core/v3/base.proto";

import "xds/annotations/v3/status.proto";

import "udpa/annotations/status.proto";
import "validate/validate.proto";

Expand All @@ -20,10 +18,6 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;

// Configuration for DataSourceServerPreferredAddressConfig.
message DataSourceServerPreferredAddressConfig {
// [#comment:TODO(danzh2010): discuss with API shepherds before removing WiP status.]

option (xds.annotations.v3.message_status).work_in_progress = true;

// Addresses for server preferred address for a single address family (IPv4 or IPv6).
message AddressFamilyConfig {
// The server preferred address sent to clients. The data must contain an IP address string.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ package envoy.extensions.quic.server_preferred_address.v3;

import "envoy/config/core/v3/address.proto";

import "xds/annotations/v3/status.proto";

import "udpa/annotations/status.proto";

option java_package = "io.envoyproxy.envoy.extensions.quic.server_preferred_address.v3";
Expand All @@ -19,10 +17,6 @@ option (udpa.annotations.file_status).package_version_status = ACTIVE;

// Configuration for FixedServerPreferredAddressConfig.
message FixedServerPreferredAddressConfig {
// [#comment:TODO(danzh2010): discuss with API shepherds before removing WiP status.]

option (xds.annotations.v3.message_status).work_in_progress = true;

// Addresses for server preferred address for a single address family (IPv4 or IPv6).
message AddressFamilyConfig {
// The server preferred address sent to clients.
Expand Down
4 changes: 0 additions & 4 deletions bazel/external/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,10 @@ exports_files(["boringssl_fips.genrule_cmd"])
cc_library(
name = "all_external",
srcs = [":empty.cc"],
defines = ["OPENTRACING_STATIC"],
# TODO: external/io_opentracing_cpp/BUILD.bazel:19:1: Executing genrule
# @io_opentracing_cpp//:generate_version_h failed - needs porting
tags = ["skip_on_windows"],
deps = [
"@com_github_datadog_dd_trace_cpp//:dd_trace_cpp",
"@com_google_googletest//:gtest",
"@io_opentracing_cpp//:opentracing",
],
)

Expand Down
1 change: 1 addition & 0 deletions bazel/external/quiche.BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3102,6 +3102,7 @@ envoy_quic_cc_library(
":quic_core_http_spdy_utils_lib",
":quic_core_types_lib",
":quic_platform_base",
"@com_google_absl//absl/base:nullability",
],
)

Expand Down
16 changes: 0 additions & 16 deletions bazel/external/quiche_sequencer_fix.patch

This file was deleted.

58 changes: 58 additions & 0 deletions bazel/foreign_cc/cares.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
From a070d7835d667b2fae5266fe1b790677dae47d25 Mon Sep 17 00:00:00 2001
From: Brad House <[email protected]>
Date: Thu, 12 Oct 2023 09:29:14 -0400
Subject: [PATCH] Socket callbacks were passed SOCK_STREAM instead of
SOCK_DGRAM on udp

A regression was introduced in 1.20.0 that would pass SOCK_STREAM on udp
connections due to code refactoring. If a client application validated this
data, it could cause issues as seen in gRPC.

Fixes Issue: #571
Fix By: Brad House (@bradh352)
---
src/lib/ares_process.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/lib/ares_process.c b/src/lib/ares_process.c
index ca597db7ad..2f8e4de30d 100644
--- a/src/lib/ares_process.c
+++ b/src/lib/ares_process.c
@@ -1065,6 +1065,7 @@ static ares_status_t open_socket(ares_channel channel,
unsigned short port;
struct server_connection *conn;
ares__llist_node_t *node;
+ int type = is_tcp?SOCK_STREAM:SOCK_DGRAM;

if (is_tcp) {
port = aresx_sitous(server->addr.tcp_port?
@@ -1098,8 +1099,7 @@ static ares_status_t open_socket(ares_channel channel,
}

/* Acquire a socket. */
- s = ares__open_socket(channel, server->addr.family,
- is_tcp?SOCK_STREAM:SOCK_DGRAM, 0);
+ s = ares__open_socket(channel, server->addr.family, type, 0);
if (s == ARES_SOCKET_BAD)
return ARES_ECONNREFUSED;

@@ -1129,8 +1129,7 @@ static ares_status_t open_socket(ares_channel channel,
#endif

if (channel->sock_config_cb) {
- int err = channel->sock_config_cb(s, SOCK_STREAM,
- channel->sock_config_cb_data);
+ int err = channel->sock_config_cb(s, type, channel->sock_config_cb_data);
if (err < 0) {
ares__close_socket(channel, s);
return ARES_ECONNREFUSED;
@@ -1148,8 +1147,7 @@ static ares_status_t open_socket(ares_channel channel,
}

if (channel->sock_create_cb) {
- int err = channel->sock_create_cb(s, SOCK_STREAM,
- channel->sock_create_cb_data);
+ int err = channel->sock_create_cb(s, type, channel->sock_create_cb_data);
if (err < 0) {
ares__close_socket(channel, s);
return ARES_ECONNREFUSED;
22 changes: 5 additions & 17 deletions bazel/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,6 @@ def envoy_dependencies(skip_targets = []):
_com_googlesource_googleurl()
_io_hyperscan()
_io_vectorscan()
_io_opentracing_cpp()
_io_opentelemetry_api_cpp()
_net_colm_open_source_colm()
_net_colm_open_source_ragel()
Expand Down Expand Up @@ -444,6 +443,11 @@ def _com_github_c_ares_c_ares():
external_http_archive(
name = "com_github_c_ares_c_ares",
build_file_content = BUILD_ALL_CONTENT,
# Patch c-ares library aith commit
# https://github.com/c-ares/c-ares/commit/a070d7835d667b2fae5266fe1b790677dae47d25
# This commit fixes an issue when the gRPC library attempts to resolve a domain name.
patches = ["@envoy//bazel/foreign_cc:cares.patch"],
patch_args = ["-p1"],
)
native.bind(
name = "ares",
Expand Down Expand Up @@ -757,18 +761,6 @@ def _io_vectorscan():
patches = ["@envoy//bazel/foreign_cc:vectorscan.patch"],
)

def _io_opentracing_cpp():
external_http_archive(
name = "io_opentracing_cpp",
patch_args = ["-p1"],
# Workaround for LSAN false positive in https://github.com/envoyproxy/envoy/issues/7647
patches = ["@envoy//bazel:io_opentracing_cpp.patch"],
)
native.bind(
name = "opentracing",
actual = "@io_opentracing_cpp//:opentracing",
)

def _io_opentelemetry_api_cpp():
external_http_archive(name = "io_opentelemetry_cpp")
native.bind(
Expand Down Expand Up @@ -1133,10 +1125,6 @@ def _com_github_google_quiche():
external_http_archive(
name = "com_github_google_quiche",
patch_cmds = ["find quiche/ -type f -name \"*.bazel\" -delete"],
patches = [
"@envoy//bazel/external:quiche_sequencer_fix.patch",
],
patch_args = ["-p1"],
build_file = "@envoy//bazel/external:quiche.BUILD",
)
native.bind(
Expand Down
24 changes: 3 additions & 21 deletions bazel/repository_locations.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -563,24 +563,6 @@ REPOSITORY_LOCATIONS_SPEC = dict(
license = "BSD-3-Clause",
license_url = "https://github.com/VectorCamp/vectorscan/blob/vectorscan/{version}/LICENSE",
),
io_opentracing_cpp = dict(
project_name = "OpenTracing",
project_desc = "Vendor-neutral APIs and instrumentation for distributed tracing",
project_url = "https://opentracing.io",
version = "1.5.1",
sha256 = "015c4187f7a6426a2b5196f0ccd982aa87f010cf61f507ae3ce5c90523f92301",
strip_prefix = "opentracing-cpp-{version}",
urls = ["https://github.com/opentracing/opentracing-cpp/archive/v{version}.tar.gz"],
use_category = ["observability_ext"],
extensions = [
"envoy.tracers.datadog",
"envoy.tracers.dynamic_ot",
],
release_date = "2019-01-16",
cpe = "N/A",
license = "Apache-2.0",
license_url = "https://github.com/opentracing/opentracing-cpp/blob/v{version}/LICENSE",
),
io_opentelemetry_cpp = dict(
project_name = "OpenTelemetry",
project_desc = "Observability framework and toolkit designed to create and manage telemetry data such as traces, metrics, and logs.",
Expand Down Expand Up @@ -1204,12 +1186,12 @@ REPOSITORY_LOCATIONS_SPEC = dict(
project_name = "QUICHE",
project_desc = "QUICHE (QUIC, HTTP/2, Etc) is Google‘s implementation of QUIC and related protocols",
project_url = "https://github.com/google/quiche",
version = "f8ca4ffbe5eb5c099bd11ba3e90553fa282c8421",
sha256 = "7648ede3f32bc6367a629b245d268c9be47ba05e23b4345a54152dabeba387d9",
version = "f4ed5e0c74485fb302367b833b8974373fed9e4c",
sha256 = "05e40b18e78b76a14bfa02eca1d6ebcf4c2ea0333c5db9fbe04287f912db2c20",
urls = ["https://github.com/google/quiche/archive/{version}.tar.gz"],
strip_prefix = "quiche-{version}",
use_category = ["controlplane", "dataplane_core"],
release_date = "2024-07-17",
release_date = "2024-07-26",
cpe = "N/A",
license = "BSD-3-Clause",
license_url = "https://github.com/google/quiche/blob/{version}/LICENSE",
Expand Down
7 changes: 7 additions & 0 deletions changelogs/current.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ bug_fixes:
- area: quic
change: |
Fixes access log formatter %CONNECTION_ID% for QUIC connections.
- area: c-ares
change: |
Applying a C-ares patch to fix DNS resoultion by the Google gRPC library.
- area: ext_authz
change: |
Fixed fail-open behavior of the :ref:`failure_mode_allow config option
Expand Down Expand Up @@ -77,6 +80,10 @@ removed_config_or_runtime:
- area: stateful_session
change: |
Removed ``envoy.reloadable_features.stateful_session_encode_ttl_in_cookie`` runtime flag and legacy code paths.
- area: upstream flow control
change: |
Removed ``envoy.reloadable_features.upstream_wait_for_response_headers_before_disabling_read`` runtime flag
and legacy code paths.
new_features:
- area: tls
Expand Down
2 changes: 1 addition & 1 deletion mobile/examples/java/hello_world/MainActivity.java
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ private void makeRequest() {
return Unit.INSTANCE;
})
.start(Executors.newSingleThreadExecutor())
.sendHeaders(requestHeaders, true);
.sendHeaders(requestHeaders, /* endStream= */ true, /* idempotent= */ false);

clear_text = !clear_text;
}
Expand Down
4 changes: 2 additions & 2 deletions mobile/library/cc/stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ namespace Platform {

Stream::Stream(InternalEngine* engine, envoy_stream_t handle) : engine_(engine), handle_(handle) {}

Stream& Stream::sendHeaders(Http::RequestHeaderMapPtr headers, bool end_stream) {
engine_->sendHeaders(handle_, std::move(headers), end_stream);
Stream& Stream::sendHeaders(Http::RequestHeaderMapPtr headers, bool end_stream, bool idempotent) {
engine_->sendHeaders(handle_, std::move(headers), end_stream, idempotent);
return *this;
}

Expand Down
5 changes: 4 additions & 1 deletion mobile/library/cc/stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@ class Stream {
*
* @param headers the headers to send.
* @param end_stream indicates whether to close the stream locally after sending this frame.
* @param idempotent indicates that the request is idempotent. When idempotent is set to true
* Envoy Mobile will retry on HTTP/3 post-handshake failures. By default, it is
* set to false.
*/
Stream& sendHeaders(Http::RequestHeaderMapPtr headers, bool end_stream);
Stream& sendHeaders(Http::RequestHeaderMapPtr headers, bool end_stream, bool idempotent = false);

/**
* Send data over an open HTTP stream. This method can be invoked multiple times.
Expand Down
10 changes: 8 additions & 2 deletions mobile/library/common/http/client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "absl/strings/str_join.h"
#include "absl/strings/string_view.h"
#include "library/common/bridge/utility.h"
#include "library/common/http/header_utility.h"
#include "library/common/stream_info/extra_stream_info.h"
#include "library/common/system/system_helper.h"

Expand Down Expand Up @@ -552,7 +551,8 @@ void Client::startStream(envoy_stream_t new_stream_handle, EnvoyStreamCallbacks&
ENVOY_LOG(debug, "[S{}] start stream", new_stream_handle);
}

void Client::sendHeaders(envoy_stream_t stream, RequestHeaderMapPtr headers, bool end_stream) {
void Client::sendHeaders(envoy_stream_t stream, RequestHeaderMapPtr headers, bool end_stream,
bool idempotent) {
ASSERT(dispatcher_.isThreadSafe());
Client::DirectStreamSharedPtr direct_stream =
getStream(stream, GetStreamFilters::AllowOnlyForOpenStreams);
Expand Down Expand Up @@ -597,6 +597,12 @@ void Client::sendHeaders(envoy_stream_t stream, RequestHeaderMapPtr headers, boo
// a request here:
// https://github.com/envoyproxy/envoy/blob/c9e3b9d2c453c7fe56a0e3615f0c742ac0d5e768/source/common/router/config_impl.cc#L1091-L1096
headers->setReferenceForwardedProto(Headers::get().SchemeValues.Https);
// When the request is idempotent, it is safe to retry.
if (idempotent) {
// https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_filters/router_filter#x-envoy-retry-on
headers->addCopy(Headers::get().EnvoyRetryOn,
Headers::get().EnvoyRetryOnValues.Http3PostConnectFailure);
}
ENVOY_LOG(debug, "[S{}] request headers for stream (end_stream={}):\n{}", stream, end_stream,
*headers);
request_decoder->decodeHeaders(std::move(headers), end_stream);
Expand Down
6 changes: 5 additions & 1 deletion mobile/library/common/http/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,12 @@ class Client : public Logger::Loggable<Logger::Id::http> {
* @param stream the stream to send headers over.
* @param headers the headers to send.
* @param end_stream indicates whether to close the stream locally after sending this frame.
* @param idempotent indicates that the request is idempotent. When idempotent is set to true
* Envoy Mobile will retry on HTTP/3 post-handshake failures. By default, it is
* set to false.
*/
void sendHeaders(envoy_stream_t stream, RequestHeaderMapPtr headers, bool end_stream);
void sendHeaders(envoy_stream_t stream, RequestHeaderMapPtr headers, bool end_stream,
bool idempotent = false);

/**
* Notify the stream that the caller is ready to receive more data from the response stream. Only
Expand Down
9 changes: 5 additions & 4 deletions mobile/library/common/internal_engine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,11 @@ envoy_status_t InternalEngine::startStream(envoy_stream_t stream,
}

envoy_status_t InternalEngine::sendHeaders(envoy_stream_t stream, Http::RequestHeaderMapPtr headers,
bool end_stream) {
return dispatcher_->post([this, stream, headers = std::move(headers), end_stream]() mutable {
http_client_->sendHeaders(stream, std::move(headers), end_stream);
});
bool end_stream, bool idempotent) {
return dispatcher_->post(
[this, stream, headers = std::move(headers), end_stream, idempotent]() mutable {
http_client_->sendHeaders(stream, std::move(headers), end_stream, idempotent);
});
return ENVOY_SUCCESS;
}

Expand Down
5 changes: 4 additions & 1 deletion mobile/library/common/internal_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,12 @@ class InternalEngine : public Logger::Loggable<Logger::Id::main> {
* @param stream the stream to send headers over.
* @param headers the headers to send.
* @param end_stream indicates whether to close the stream locally after sending this frame.
* @param idempotent indicates that the request is idempotent. When idempotent is set to true
* Envoy Mobile will retry on HTTP/3 post-handshake failures. By default, it is
* set to false.
*/
envoy_status_t sendHeaders(envoy_stream_t stream, Http::RequestHeaderMapPtr headers,
bool end_stream);
bool end_stream, bool idempotent = false);

envoy_status_t readData(envoy_stream_t stream, size_t bytes_to_read);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,14 @@ public EnvoyHTTPStream(long engineHandle, long streamHandle, EnvoyHTTPCallbacks
* Send headers over an open HTTP streamHandle. This method can be invoked once
* and needs to be called before send_data.
*
* @param headers, the headers to send.
* @param endStream, supplies whether this is headers only.
* @param headers the headers to send.
* @param endStream supplies whether this is headers only.
* @param idempotent indicates that the request is idempotent. When idempotent is set to true
* Envoy Mobile will retry on HTTP/3 post-handshake failures.
*/
public void sendHeaders(Map<String, List<String>> headers, boolean endStream) {
JniLibrary.sendHeaders(engineHandle, streamHandle, headers, endStream);
public void sendHeaders(Map<String, List<String>> headers, boolean endStream,
boolean idempotent) {
JniLibrary.sendHeaders(engineHandle, streamHandle, headers, endStream, idempotent);
}

/**
Expand Down
Loading

0 comments on commit 26e2439

Please sign in to comment.