impl(rest): only populate userIp when requested via options (#8399)

googleapis · Feb 16, 2022 · 541eb09 · 541eb09
1 parent 207a602
commit 541eb09
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 7 deletions.
diff --git a/google/cloud/internal/rest_client.cc b/google/cloud/internal/rest_client.cc
@@ -103,10 +103,14 @@ StatusOr<std::unique_ptr<CurlImpl>> CurlRestClient::CreateCurlImpl(
   impl->SetHeader(HostHeader(options_, endpoint_address_));
   impl->SetHeader(x_goog_api_client_header_);
   impl->SetHeaders(request);
-  auto user_ip = options_.get<UserIpOption>();
-  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<UserIpOption>()) {
+    auto user_ip = options_.get<UserIpOption>();
+    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
@@ -149,7 +149,8 @@ TEST_F(RestClientIntegrationTest, Get) {
 
 TEST_F(RestClientIntegrationTest, Delete) {
   options_.set<UnifiedCredentialsOption>(MakeInsecureCredentials());
-  auto client = GetDefaultRestClient(url_, {});
+  options_.set<UserIpOption>("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(),