Commit e8c969b6 authored by Josh Nohle's avatar Josh Nohle Committed by Commit Bot

[Nearby] Do not populate ID list in ListPublicCertificates request

The ListPublicCertificates RPC has an optional field in the request
where the client can list the IDs on all locally stored public
certificates. Certificates with these IDs will not be returned in the
response. However, this RPC is made in Chromium via an HTTP GET
request, and One Platform documentation notes request-URL size
limits [1].

There are ways to circumvent these size restrictions, but until the
Nearby Share server makes the appropriate changes to the API, we will
not send the ID list.

[1]https://g3doc.corp.google.com/google/g3doc/oneplatform/http.md#long-request-url

Bug: b/168701170
Change-Id: Ieec8ad883c152769006c9c8dd94a0341ff46dd3b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2423323
Commit-Queue: James Vecore <vecore@google.com>
Reviewed-by: default avatarJames Vecore <vecore@google.com>
Auto-Submit: Josh Nohle <nohle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809191}
parent 16b3ef3a
...@@ -526,10 +526,10 @@ void NearbyShareCertificateManagerImpl::OnDownloadPublicCertificatesRequest( ...@@ -526,10 +526,10 @@ void NearbyShareCertificateManagerImpl::OnDownloadPublicCertificatesRequest(
if (page_token) if (page_token)
request.set_page_token(*page_token); request.set_page_token(*page_token);
for (const std::string& id : // TODO(b/168701170): One Platform has a length restriction on request URLs.
certificate_storage_->GetPublicCertificateIds()) { // Adding all secret IDs to the request, and subsequently as query parameters,
request.add_secret_ids(id); // could result in hitting this limit. Add the secret IDs of all locally
} // stored public certificates when this length restriction is circumvented.
NS_LOG(VERBOSE) << __func__ << ": Downloading public certificates."; NS_LOG(VERBOSE) << __func__ << ": Downloading public certificates.";
...@@ -539,8 +539,6 @@ void NearbyShareCertificateManagerImpl::OnDownloadPublicCertificatesRequest( ...@@ -539,8 +539,6 @@ void NearbyShareCertificateManagerImpl::OnDownloadPublicCertificatesRequest(
&NearbyShareCertificateManagerImpl::OnListPublicCertificatesTimeout, &NearbyShareCertificateManagerImpl::OnListPublicCertificatesTimeout,
base::Unretained(this), page_number, certificate_count)); base::Unretained(this), page_number, certificate_count));
// TODO(https://crbug.com/1116910): Enforce a timeout for each
// ListPublicCertificates call.
client_ = client_factory_->CreateInstance(); client_ = client_factory_->CreateInstance();
client_->ListPublicCertificates( client_->ListPublicCertificates(
request, request,
......
...@@ -320,11 +320,14 @@ class NearbyShareCertificateManagerImplTest ...@@ -320,11 +320,14 @@ class NearbyShareCertificateManagerImplTest
const nearbyshare::proto::ListPublicCertificatesRequest& request, const nearbyshare::proto::ListPublicCertificatesRequest& request,
const std::string& page_token) { const std::string& page_token) {
EXPECT_EQ(request.parent(), std::string(kDeviceIdPrefix) + kDeviceId); EXPECT_EQ(request.parent(), std::string(kDeviceIdPrefix) + kDeviceId);
ASSERT_EQ(request.secret_ids_size(),
static_cast<int>(kPublicCertificateIds.size())); // TODO(b/168701170): One Platform has a length restriction on request URLs.
for (size_t i = 0; i < kPublicCertificateIds.size(); ++i) { // Adding all secret IDs to the request, and subsequently as query
EXPECT_EQ(request.secret_ids(i), kPublicCertificateIds[i]); // parameters, could result in hitting this limit. Add the secret IDs of all
} // locally stored public certificates when this length restriction is
// circumvented.
EXPECT_TRUE(request.secret_ids().empty());
EXPECT_EQ(request.page_token(), page_token); EXPECT_EQ(request.page_token(), page_token);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment