Skip to content

Commit 7624e88

Browse files
laramielcopybara-github
authored andcommitted
Enable http proxies on windows
Add a test using proxy env vars; this test validates a curl error. Fixes: #243 PiperOrigin-RevId: 786531981 Change-Id: I9f38071b01fe10a63021127a8dd961aee2bfe3fe
1 parent 2b91250 commit 7624e88

File tree

5 files changed

+51
-11
lines changed

5 files changed

+51
-11
lines changed

tensorstore/internal/curl/BUILD

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,11 +156,14 @@ tensorstore_cc_test(
156156
deps = [
157157
":curl_transport",
158158
":default_factory",
159+
"//tensorstore/internal:env",
159160
"//tensorstore/internal/http",
160161
"//tensorstore/internal/http:test_httpserver",
162+
"//tensorstore/util:status_testutil",
161163
"@abseil-cpp//absl/base",
162164
"@abseil-cpp//absl/base:no_destructor",
163165
"@abseil-cpp//absl/log:absl_log",
166+
"@abseil-cpp//absl/status",
164167
"@abseil-cpp//absl/strings",
165168
"@abseil-cpp//absl/strings:str_format",
166169
"@abseil-cpp//absl/time",

tensorstore/internal/curl/default_factory.cc

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,19 +147,31 @@ CurlPtr DefaultCurlHandleFactory::CreateHandle() {
147147
ABSL_LOG_IF(INFO, curl_logging.Level(1)) << "Setting ca_path " << *x;
148148
ABSL_CHECK_EQ(CURLE_OK,
149149
curl_easy_setopt(handle.get(), CURLOPT_CAPATH, x->c_str()));
150+
ABSL_CHECK_EQ(
151+
CURLE_OK,
152+
curl_easy_setopt(handle.get(), CURLOPT_PROXY_CAPATH, x->c_str()));
150153
}
151154
if (auto& x = config_.ca_bundle) {
152155
ABSL_LOG_IF(INFO, curl_logging.Level(1)) << "Setting ca_bundle " << *x;
153156
ABSL_CHECK_EQ(CURLE_OK,
154157
curl_easy_setopt(handle.get(), CURLOPT_CAINFO, x->c_str()));
158+
ABSL_CHECK_EQ(
159+
CURLE_OK,
160+
curl_easy_setopt(handle.get(), CURLOPT_PROXY_CAINFO, x->c_str()));
155161
}
156162
}
163+
157164
// Disable host verification if requested.
158165
if (!config_.verify_host) {
166+
ABSL_LOG_IF(INFO, curl_logging.Level(1)) << "Disabling host verification";
159167
ABSL_CHECK_EQ(CURLE_OK,
160168
curl_easy_setopt(handle.get(), CURLOPT_SSL_VERIFYHOST, 0L));
161169
ABSL_CHECK_EQ(CURLE_OK,
162170
curl_easy_setopt(handle.get(), CURLOPT_SSL_VERIFYPEER, 0L));
171+
ABSL_CHECK_EQ(CURLE_OK, curl_easy_setopt(handle.get(),
172+
CURLOPT_PROXY_SSL_VERIFYHOST, 0L));
173+
ABSL_CHECK_EQ(CURLE_OK, curl_easy_setopt(handle.get(),
174+
CURLOPT_PROXY_SSL_VERIFYPEER, 0L));
163175
}
164176
return handle;
165177
};

tensorstore/internal/curl/http2_test.cc

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,25 @@
1616
#include <string>
1717
#include <utility>
1818

19+
#include <gmock/gmock.h>
1920
#include <gtest/gtest.h>
2021
#include "absl/base/call_once.h"
2122
#include "absl/base/no_destructor.h"
2223
#include "absl/log/absl_log.h"
24+
#include "absl/status/status.h"
2325
#include "absl/strings/str_cat.h"
2426
#include "absl/strings/str_format.h"
2527
#include "absl/time/time.h"
2628
#include "tensorstore/internal/curl/curl_transport.h"
2729
#include "tensorstore/internal/curl/default_factory.h"
30+
#include "tensorstore/internal/env.h"
2831
#include "tensorstore/internal/http/http_request.h"
2932
#include "tensorstore/internal/http/http_response.h"
3033
#include "tensorstore/internal/http/http_transport.h"
3134
#include "tensorstore/internal/http/test_httpserver.h"
35+
#include "tensorstore/util/status_testutil.h"
3236

37+
using ::tensorstore::internal::SetEnv;
3338
using ::tensorstore::internal_http::CurlTransport;
3439
using ::tensorstore::internal_http::DefaultCurlHandleFactory;
3540
using ::tensorstore::internal_http::HttpRequestBuilder;
@@ -80,7 +85,7 @@ TEST(HttpserverTest, Basic) {
8085
.SetRequestTimeout(absl::Seconds(10)));
8186

8287
// Waits for the response.
83-
ABSL_LOG(INFO) << response.status();
88+
response.Wait();
8489

8590
EXPECT_EQ(200, response.value().status_code);
8691
EXPECT_EQ("Received 5 bytes\n", response.value().payload);
@@ -96,12 +101,40 @@ TEST(HttpserverTest, Basic) {
96101
.SetRequestTimeout(absl::Seconds(10)));
97102

98103
// Waits for the response.
99-
ABSL_LOG(INFO) << response.status();
104+
response.Wait();
100105

101106
EXPECT_EQ(404, response.value().status_code);
102107
EXPECT_EQ("Not Found: /missing\n", response.value().payload);
103108
}
104109
GetHttpServer().MaybeLogStdoutPipe();
105110
}
106111

112+
TEST(HttpserverTest, WithProxy) {
113+
auto base_url = absl::StrFormat("https://%s", GetHttpServer().http_address());
114+
auto transport = GetTransport();
115+
GetHttpServer().MaybeLogStdoutPipe();
116+
117+
SetEnv("https_proxy", base_url.c_str());
118+
SetEnv("http_proxy", base_url.c_str());
119+
120+
// Issue request.
121+
{
122+
auto request = HttpRequestBuilder("GET", absl::StrCat(base_url, "/proxy"))
123+
.BuildRequest();
124+
auto options = IssueRequestOptions()
125+
.SetConnectTimeout(absl::Seconds(10))
126+
.SetRequestTimeout(absl::Seconds(10));
127+
auto response = transport->IssueRequest(request, options);
128+
129+
// Waits for the response.
130+
response.Wait();
131+
132+
// TestHttpServer is not a proxy; should return a curl error.
133+
EXPECT_THAT(response.status(),
134+
tensorstore::MatchesStatus(absl::StatusCode::kUnavailable));
135+
}
136+
137+
GetHttpServer().MaybeLogStdoutPipe();
138+
}
139+
107140
} // namespace

third_party/curl/config/curl_config.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@
4242

4343
#if defined(_WIN32)
4444
#include "lib/config-win32.h"
45-
#define BUILDING_LIBCURL 1
4645
#define CURL_DISABLE_CRYPTO_AUTH 1
4746
#define CURL_PULL_WS2TCPIP_H 1
4847
#define CURL_PULL_WS2TCPIP_H 1

third_party/curl/curl.BUILD.bazel

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,7 @@ LOCAL_DEFINES = (
3434
# "USE_ARES", # TODO: Enable c-ares; after debugging broken test.
3535
] + select(
3636
{
37-
"@platforms//os:windows": [
38-
"CURL_DISABLE_PROXY",
39-
# Defining _USING_V110_SDK71_ is hackery to defeat curl's incorrect
40-
# detection of what OS releases we can build on with VC 2012. This
41-
# may not be needed (or may have to change) if the WINVER setting
42-
# changes in //third_party/msvc/vc_12_0/CROSSTOOL.
43-
"_USING_V110_SDK71_",
44-
],
37+
"@platforms//os:windows": [],
4538
"//conditions:default": ["_GNU_SOURCE"],
4639
},
4740
) + select(

0 commit comments

Comments
 (0)