From 541eb09c32c91c09ea84a55fc0861e916ebec56a Mon Sep 17 00:00:00 2001 From: Scott Hart Date: Wed, 16 Feb 2022 16:07:30 -0500 Subject: [PATCH] impl(rest): only populate userIp when requested via options (#8399) --- google/cloud/internal/rest_client.cc | 10 +++++++--- google/cloud/internal/rest_client_integration_test.cc | 9 +++++---- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/google/cloud/internal/rest_client.cc b/google/cloud/internal/rest_client.cc index 5cc1c58aa7c23..07e68c7a1a73e 100644 --- a/google/cloud/internal/rest_client.cc +++ b/google/cloud/internal/rest_client.cc @@ -103,10 +103,14 @@ StatusOr> CurlRestClient::CreateCurlImpl( impl->SetHeader(HostHeader(options_, endpoint_address_)); impl->SetHeader(x_goog_api_client_header_); impl->SetHeaders(request); - auto user_ip = options_.get(); - if (user_ip.empty()) user_ip = impl->LastClientIpAddress(); RestRequest::HttpParameters additional_parameters; - if (!user_ip.empty()) additional_parameters.emplace_back("userIp", user_ip); + // The UserIp option has been deprecated in favor of quotaUser. Only add the + // parameter if the option has been set. + if (options_.has()) { + auto user_ip = options_.get(); + if (user_ip.empty()) user_ip = impl->LastClientIpAddress(); + if (!user_ip.empty()) additional_parameters.emplace_back("userIp", user_ip); + } impl->SetUrl(endpoint_address_, request, additional_parameters); return impl; } diff --git a/google/cloud/internal/rest_client_integration_test.cc b/google/cloud/internal/rest_client_integration_test.cc index 0ee90d9effa32..c9495fee2de38 100644 --- a/google/cloud/internal/rest_client_integration_test.cc +++ b/google/cloud/internal/rest_client_integration_test.cc @@ -149,7 +149,8 @@ TEST_F(RestClientIntegrationTest, Get) { TEST_F(RestClientIntegrationTest, Delete) { options_.set(MakeInsecureCredentials()); - auto client = GetDefaultRestClient(url_, {}); + options_.set("127.0.0.1"); + auto client = GetDefaultRestClient(url_, options_); RestRequest request; request.SetPath("delete"); request.AddQueryParameter({"key", "value"}); @@ -164,11 +165,10 @@ TEST_F(RestClientIntegrationTest, Delete) { EXPECT_STATUS_OK(body); EXPECT_GT(body->size(), 0); auto parsed_response = nlohmann::json::parse(*body, nullptr, false); - EXPECT_FALSE(parsed_response.is_discarded()); - ASSERT_FALSE(parsed_response.is_null()); + ASSERT_TRUE(parsed_response.is_object()); auto url = parsed_response.find("url"); ASSERT_NE(url, parsed_response.end()); - EXPECT_THAT(url.value(), HasSubstr("/delete?key=value")); + EXPECT_THAT(url.value(), HasSubstr("/delete?key=value&userIp=127.0.0.1")); } TEST_F(RestClientIntegrationTest, PatchJsonContentType) { @@ -199,6 +199,7 @@ TEST_F(RestClientIntegrationTest, PatchJsonContentType) { ASSERT_NE(url, parsed_response.end()); EXPECT_THAT(url.value(), HasSubstr("/patch?type=service_account&project_id=foo-project")); + EXPECT_THAT(url.value(), Not(HasSubstr("userIp="))); auto data = parsed_response.find("data"); ASSERT_NE(data, parsed_response.end()); EXPECT_THAT(data.value(),