HTTP Proxy: Ignore empty entry in no_proxy list (#29217)

* HTTP Proxy: Ignore empty entry in no_proxy list

* Reviewer comments and add unit test

* Reviewer comments

* Fix use-after-free

* Clang-tidy
reviewable/pr29111/r6^2
Yash Tibrewal 3 years ago committed by GitHub
parent 14c078bcdc
commit 4b8ea48c35
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 36
      CMakeLists.txt
  2. 10
      build_autogenerated.yaml
  3. 198
      src/core/ext/filters/client_channel/http_proxy.cc
  4. 17
      src/core/ext/filters/client_channel/http_proxy.h
  5. 13
      test/core/client_channel/BUILD
  6. 100
      test/core/client_channel/http_proxy_mapper_test.cc
  7. 24
      tools/run_tests/generated/tests.json

36
CMakeLists.txt generated

@ -891,6 +891,7 @@ if(gRPC_BUILD_TESTS)
add_dependencies(buildtests_cxx hpack_parser_table_test)
add_dependencies(buildtests_cxx hpack_parser_test)
add_dependencies(buildtests_cxx http2_client)
add_dependencies(buildtests_cxx http_proxy_mapper_test)
if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX)
add_dependencies(buildtests_cxx httpcli_test)
endif()
@ -11795,6 +11796,41 @@ target_link_libraries(http2_client
)
endif()
if(gRPC_BUILD_TESTS)
add_executable(http_proxy_mapper_test
test/core/client_channel/http_proxy_mapper_test.cc
third_party/googletest/googletest/src/gtest-all.cc
third_party/googletest/googlemock/src/gmock-all.cc
)
target_include_directories(http_proxy_mapper_test
PRIVATE
${CMAKE_CURRENT_SOURCE_DIR}
${CMAKE_CURRENT_SOURCE_DIR}/include
${_gRPC_ADDRESS_SORTING_INCLUDE_DIR}
${_gRPC_RE2_INCLUDE_DIR}
${_gRPC_SSL_INCLUDE_DIR}
${_gRPC_UPB_GENERATED_DIR}
${_gRPC_UPB_GRPC_GENERATED_DIR}
${_gRPC_UPB_INCLUDE_DIR}
${_gRPC_XXHASH_INCLUDE_DIR}
${_gRPC_ZLIB_INCLUDE_DIR}
third_party/googletest/googletest/include
third_party/googletest/googletest
third_party/googletest/googlemock/include
third_party/googletest/googlemock
${_gRPC_PROTO_GENS_DIR}
)
target_link_libraries(http_proxy_mapper_test
${_gRPC_PROTOBUF_LIBRARIES}
${_gRPC_ALLTARGETS_LIBRARIES}
grpc_test_util
)
endif()
if(gRPC_BUILD_TESTS)
if(_gRPC_PLATFORM_LINUX OR _gRPC_PLATFORM_MAC OR _gRPC_PLATFORM_POSIX)

@ -6163,6 +6163,16 @@ targets:
deps:
- grpc++_test_config
- grpc++_test_util
- name: http_proxy_mapper_test
gtest: true
build: test
language: c++
headers: []
src:
- test/core/client_channel/http_proxy_mapper_test.cc
deps:
- grpc_test_util
uses_polling: false
- name: httpcli_test
gtest: true
build: test

@ -23,7 +23,9 @@
#include <stdbool.h>
#include <string.h>
#include "absl/strings/match.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/str_split.h"
#include "absl/strings/strip.h"
#include <grpc/support/alloc.h>
@ -64,24 +66,24 @@ char* GetHttpProxyServer(const grpc_channel_args* args, char** user_cred) {
* 4. http_proxy environment variable
* If none of the above are set, then no HTTP proxy will be used.
*/
char* uri_str =
gpr_strdup(grpc_channel_args_find_string(args, GRPC_ARG_HTTP_PROXY));
if (uri_str == nullptr) uri_str = gpr_getenv("grpc_proxy");
if (uri_str == nullptr) uri_str = gpr_getenv("https_proxy");
if (uri_str == nullptr) uri_str = gpr_getenv("http_proxy");
auto uri_str = UniquePtr<char>(
gpr_strdup(grpc_channel_args_find_string(args, GRPC_ARG_HTTP_PROXY)));
if (uri_str == nullptr) uri_str = UniquePtr<char>(gpr_getenv("grpc_proxy"));
if (uri_str == nullptr) uri_str = UniquePtr<char>(gpr_getenv("https_proxy"));
if (uri_str == nullptr) uri_str = UniquePtr<char>(gpr_getenv("http_proxy"));
if (uri_str == nullptr) return nullptr;
// an emtpy value means "don't use proxy"
if (uri_str[0] == '\0') goto done;
uri = URI::Parse(uri_str);
if (uri_str.get()[0] == '\0') return nullptr;
uri = URI::Parse(uri_str.get());
if (!uri.ok() || uri->authority().empty()) {
gpr_log(GPR_ERROR, "cannot parse value of 'http_proxy' env var. Error: %s",
uri.status().ToString().c_str());
goto done;
return nullptr;
}
if (uri->scheme() != "http") {
gpr_log(GPR_ERROR, "'%s' scheme not supported in proxy URI",
uri->scheme().c_str());
goto done;
return nullptr;
}
/* Split on '@' to separate user credentials from host */
gpr_string_split(uri->authority().c_str(), "@", &authority_strs,
@ -103,8 +105,6 @@ char* GetHttpProxyServer(const grpc_channel_args* args, char** user_cred) {
proxy_name = nullptr;
}
gpr_free(authority_strs);
done:
gpr_free(uri_str);
return proxy_name;
}
@ -119,111 +119,89 @@ std::string MaybeAddDefaultPort(absl::string_view target) {
return std::string(target);
}
class HttpProxyMapper : public ProxyMapperInterface {
public:
bool MapName(const char* server_uri, const grpc_channel_args* args,
char** name_to_resolve, grpc_channel_args** new_args) override {
if (!grpc_channel_args_find_bool(args, GRPC_ARG_ENABLE_HTTP_PROXY, true)) {
return false;
}
char* user_cred = nullptr;
*name_to_resolve = GetHttpProxyServer(args, &user_cred);
if (*name_to_resolve == nullptr) return false;
char* no_proxy_str = nullptr;
std::string server_target;
absl::StatusOr<URI> uri = URI::Parse(server_uri);
if (!uri.ok() || uri->path().empty()) {
gpr_log(GPR_ERROR,
"'http_proxy' environment variable set, but cannot "
"parse server URI '%s' -- not using proxy. Error: %s",
server_uri, uri.status().ToString().c_str());
goto no_use_proxy;
}
if (uri->scheme() == "unix") {
gpr_log(GPR_INFO, "not using proxy for Unix domain socket '%s'",
server_uri);
goto no_use_proxy;
}
/* Prefer using 'no_grpc_proxy'. Fallback on 'no_proxy' if it is not set. */
no_proxy_str = gpr_getenv("no_grpc_proxy");
if (no_proxy_str == nullptr) no_proxy_str = gpr_getenv("no_proxy");
if (no_proxy_str != nullptr) {
static const char* NO_PROXY_SEPARATOR = ",";
bool use_proxy = true;
std::string server_host;
std::string server_port;
if (!SplitHostPort(absl::StripPrefix(uri->path(), "/"), &server_host,
&server_port)) {
gpr_log(GPR_INFO,
"unable to split host and port, not checking no_proxy list for "
"host '%s'",
server_uri);
gpr_free(no_proxy_str);
} else {
size_t uri_len = server_host.size();
char** no_proxy_hosts;
size_t num_no_proxy_hosts;
gpr_string_split(no_proxy_str, NO_PROXY_SEPARATOR, &no_proxy_hosts,
&num_no_proxy_hosts);
for (size_t i = 0; i < num_no_proxy_hosts; i++) {
char* no_proxy_entry = no_proxy_hosts[i];
size_t no_proxy_len = strlen(no_proxy_entry);
if (no_proxy_len <= uri_len &&
gpr_stricmp(no_proxy_entry,
&(server_host.c_str()[uri_len - no_proxy_len])) ==
0) {
gpr_log(GPR_INFO, "not using proxy for host in no_proxy list '%s'",
server_uri);
use_proxy = false;
break;
}
}
for (size_t i = 0; i < num_no_proxy_hosts; i++) {
gpr_free(no_proxy_hosts[i]);
}
gpr_free(no_proxy_hosts);
gpr_free(no_proxy_str);
if (!use_proxy) goto no_use_proxy;
}
}
server_target =
MaybeAddDefaultPort(absl::StripPrefix(uri->path(), "/")).c_str();
grpc_arg args_to_add[2];
args_to_add[0] = grpc_channel_arg_string_create(
const_cast<char*>(GRPC_ARG_HTTP_CONNECT_SERVER),
const_cast<char*>(server_target.c_str()));
if (user_cred != nullptr) {
/* Use base64 encoding for user credentials as stated in RFC 7617 */
char* encoded_user_cred =
grpc_base64_encode(user_cred, strlen(user_cred), 0, 0);
std::string header =
absl::StrCat("Proxy-Authorization:Basic ", encoded_user_cred);
gpr_free(encoded_user_cred);
args_to_add[1] = grpc_channel_arg_string_create(
const_cast<char*>(GRPC_ARG_HTTP_CONNECT_HEADERS),
const_cast<char*>(header.c_str()));
*new_args = grpc_channel_args_copy_and_add(args, args_to_add, 2);
} else {
*new_args = grpc_channel_args_copy_and_add(args, args_to_add, 1);
}
gpr_free(user_cred);
return true;
no_use_proxy:
} // namespace
bool HttpProxyMapper::MapName(const char* server_uri,
const grpc_channel_args* args,
char** name_to_resolve,
grpc_channel_args** new_args) {
if (!grpc_channel_args_find_bool(args, GRPC_ARG_ENABLE_HTTP_PROXY, true)) {
return false;
}
char* user_cred = nullptr;
*name_to_resolve = GetHttpProxyServer(args, &user_cred);
if (*name_to_resolve == nullptr) return false;
std::string server_target;
absl::StatusOr<URI> uri = URI::Parse(server_uri);
auto no_use_proxy = [&]() {
gpr_free(*name_to_resolve);
*name_to_resolve = nullptr;
gpr_free(user_cred);
return false;
};
if (!uri.ok() || uri->path().empty()) {
gpr_log(GPR_ERROR,
"'http_proxy' environment variable set, but cannot "
"parse server URI '%s' -- not using proxy. Error: %s",
server_uri, uri.status().ToString().c_str());
return no_use_proxy();
}
bool MapAddress(const grpc_resolved_address& /*address*/,
const grpc_channel_args* /*args*/,
grpc_resolved_address** /*new_address*/,
grpc_channel_args** /*new_args*/) override {
return false;
if (uri->scheme() == "unix") {
gpr_log(GPR_INFO, "not using proxy for Unix domain socket '%s'",
server_uri);
return no_use_proxy();
}
};
} // namespace
/* Prefer using 'no_grpc_proxy'. Fallback on 'no_proxy' if it is not set. */
auto no_proxy_str = UniquePtr<char>(gpr_getenv("no_grpc_proxy"));
if (no_proxy_str == nullptr) {
no_proxy_str = UniquePtr<char>(gpr_getenv("no_proxy"));
}
if (no_proxy_str != nullptr) {
bool use_proxy = true;
std::string server_host;
std::string server_port;
if (!SplitHostPort(absl::StripPrefix(uri->path(), "/"), &server_host,
&server_port)) {
gpr_log(GPR_INFO,
"unable to split host and port, not checking no_proxy list for "
"host '%s'",
server_uri);
} else {
std::vector<absl::string_view> no_proxy_hosts =
absl::StrSplit(no_proxy_str.get(), ',', absl::SkipEmpty());
for (const auto& no_proxy_entry : no_proxy_hosts) {
if (absl::EndsWithIgnoreCase(server_host, no_proxy_entry)) {
gpr_log(GPR_INFO, "not using proxy for host in no_proxy list '%s'",
server_uri);
use_proxy = false;
break;
}
}
if (!use_proxy) return no_use_proxy();
}
}
server_target =
MaybeAddDefaultPort(absl::StripPrefix(uri->path(), "/")).c_str();
absl::InlinedVector<grpc_arg, 2> args_to_add;
args_to_add.push_back(grpc_channel_arg_string_create(
const_cast<char*>(GRPC_ARG_HTTP_CONNECT_SERVER),
const_cast<char*>(server_target.c_str())));
std::string header;
if (user_cred != nullptr) {
/* Use base64 encoding for user credentials as stated in RFC 7617 */
auto encoded_user_cred =
UniquePtr<char>(grpc_base64_encode(user_cred, strlen(user_cred), 0, 0));
header =
absl::StrCat("Proxy-Authorization:Basic ", encoded_user_cred.get());
args_to_add.push_back(grpc_channel_arg_string_create(
const_cast<char*>(GRPC_ARG_HTTP_CONNECT_HEADERS),
const_cast<char*>(header.c_str())));
}
*new_args = grpc_channel_args_copy_and_add(args, args_to_add.data(),
args_to_add.size());
gpr_free(user_cred);
return true;
}
void RegisterHttpProxyMapper() {
ProxyMapperRegistry::Register(

@ -19,8 +19,25 @@
#ifndef GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_HTTP_PROXY_H
#define GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_HTTP_PROXY_H
#include <grpc/support/port_platform.h>
#include "src/core/ext/filters/client_channel/proxy_mapper.h"
namespace grpc_core {
class HttpProxyMapper : public ProxyMapperInterface {
public:
bool MapName(const char* server_uri, const grpc_channel_args* args,
char** name_to_resolve, grpc_channel_args** new_args) override;
bool MapAddress(const grpc_resolved_address& /*address*/,
const grpc_channel_args* /*args*/,
grpc_resolved_address** /*new_address*/,
grpc_channel_args** /*new_args*/) override {
return false;
}
};
void RegisterHttpProxyMapper();
} // namespace grpc_core

@ -30,6 +30,19 @@ grpc_cc_test(
],
)
grpc_cc_test(
name = "http_proxy_mapper_test",
srcs = ["http_proxy_mapper_test.cc"],
external_deps = ["gtest"],
language = "C++",
uses_polling = False,
deps = [
"//:gpr",
"//:grpc",
"//test/core/util:grpc_test_util",
],
)
grpc_cc_test(
name = "retry_throttle_test",
srcs = ["retry_throttle_test.cc"],

@ -0,0 +1,100 @@
//
//
// Copyright 2022 gRPC authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
//
#include <gmock/gmock.h>
#include "src/core/ext/filters/client_channel/http_connect_handshaker.h"
#include "src/core/ext/filters/client_channel/http_proxy.h"
#include "src/core/lib/channel/channel_args.h"
#include "src/core/lib/gpr/env.h"
#include "test/core/util/test_config.h"
namespace grpc_core {
namespace testing {
namespace {
// Test that an empty no_proxy works as expected, i.e., proxy is used.
TEST(NoProxyTest, EmptyList) {
gpr_setenv("no_proxy", "");
grpc_arg proxy_arg = grpc_channel_arg_string_create(
const_cast<char*>(GRPC_ARG_HTTP_PROXY),
const_cast<char*>("http://proxy.google.com"));
grpc_channel_args args = {1, &proxy_arg};
grpc_channel_args* new_args = nullptr;
char* name_to_resolve = nullptr;
EXPECT_TRUE(HttpProxyMapper().MapName("dns:///test.google.com:443", &args,
&name_to_resolve, &new_args));
EXPECT_STREQ(name_to_resolve, "proxy.google.com");
EXPECT_STREQ(grpc_channel_args_find_string(
new_args, const_cast<char*>(GRPC_ARG_HTTP_CONNECT_SERVER)),
"test.google.com:443");
gpr_free(name_to_resolve);
grpc_channel_args_destroy(new_args);
gpr_unsetenv("no_proxy");
}
// Test basic usage of 'no_proxy' to avoid using proxy for certain domain names.
TEST(NoProxyTest, Basic) {
gpr_setenv("no_proxy", "google.com");
grpc_arg proxy_arg = grpc_channel_arg_string_create(
const_cast<char*>(GRPC_ARG_HTTP_PROXY),
const_cast<char*>("http://proxy.google.com"));
grpc_channel_args args = {1, &proxy_arg};
grpc_channel_args* new_args = nullptr;
char* name_to_resolve = nullptr;
EXPECT_FALSE(HttpProxyMapper().MapName("dns:///test.google.com:443", &args,
&name_to_resolve, &new_args));
EXPECT_EQ(name_to_resolve, nullptr);
EXPECT_EQ(grpc_channel_args_find_string(
new_args, const_cast<char*>(GRPC_ARG_HTTP_CONNECT_SERVER)),
nullptr);
gpr_free(name_to_resolve);
grpc_channel_args_destroy(new_args);
gpr_unsetenv("no_proxy");
}
// Test empty entries in 'no_proxy' list.
TEST(NoProxyTest, EmptyEntries) {
gpr_setenv("no_proxy", "foo.com,,google.com,,");
grpc_arg proxy_arg = grpc_channel_arg_string_create(
const_cast<char*>(GRPC_ARG_HTTP_PROXY),
const_cast<char*>("http://proxy.google.com"));
grpc_channel_args args = {1, &proxy_arg};
grpc_channel_args* new_args = nullptr;
char* name_to_resolve = nullptr;
EXPECT_FALSE(HttpProxyMapper().MapName("dns:///test.google.com:443", &args,
&name_to_resolve, &new_args));
EXPECT_EQ(name_to_resolve, nullptr);
EXPECT_EQ(grpc_channel_args_find_string(
new_args, const_cast<char*>(GRPC_ARG_HTTP_CONNECT_SERVER)),
nullptr);
gpr_free(name_to_resolve);
grpc_channel_args_destroy(new_args);
gpr_unsetenv("no_proxy");
}
} // namespace
} // namespace testing
} // namespace grpc_core
int main(int argc, char** argv) {
::testing::InitGoogleTest(&argc, argv);
grpc::testing::TestEnvironment env(&argc, argv);
auto result = RUN_ALL_TESTS();
return result;
}

@ -4831,6 +4831,30 @@
],
"uses_polling": false
},
{
"args": [],
"benchmark": false,
"ci_platforms": [
"linux",
"mac",
"posix",
"windows"
],
"cpu_cost": 1.0,
"exclude_configs": [],
"exclude_iomgrs": [],
"flaky": false,
"gtest": true,
"language": "c++",
"name": "http_proxy_mapper_test",
"platforms": [
"linux",
"mac",
"posix",
"windows"
],
"uses_polling": false
},
{
"args": [],
"benchmark": false,

Loading…
Cancel
Save