Chromium Code Reviews| Index: net/cert/internal/cert_issuer_source_aia.cc |
| diff --git a/net/cert/internal/cert_issuer_source_aia.cc b/net/cert/internal/cert_issuer_source_aia.cc |
| index a6fb1b703d11bd5f8bc11598c2cf8c2fe6d3f371..4f07dbebb64ea73752e45cdd9ec95954211d899a 100644 |
| --- a/net/cert/internal/cert_issuer_source_aia.cc |
| +++ b/net/cert/internal/cert_issuer_source_aia.cc |
| @@ -20,88 +20,75 @@ const int kMaxFetchesPerCert = 5; |
| class AiaRequest : public CertIssuerSource::Request { |
| public: |
| - explicit AiaRequest(const CertIssuerSource::IssuerCallback& issuers_callback); |
| + AiaRequest() {} |
| ~AiaRequest() override; |
| // CertIssuerSource::Request implementation. |
| - CompletionStatus GetNext(scoped_refptr<ParsedCertificate>* out_cert) override; |
| + void GetNext(ParsedCertificateList* issuers) override; |
| void AddCertFetcherRequest( |
| std::unique_ptr<CertNetFetcher::Request> cert_fetcher_request); |
| - void OnFetchCompleted(Error error, const std::vector<uint8_t>& fetched_bytes); |
| + bool OnFetchCompleted(Error error, |
| + std::vector<uint8_t> fetched_bytes, |
| + ParsedCertificateList* results); |
| private: |
| - bool HasNext() const { return current_result_ < results_.size(); } |
| - |
| - CertIssuerSource::IssuerCallback issuers_callback_; |
| std::vector<std::unique_ptr<CertNetFetcher::Request>> cert_fetcher_requests_; |
| - size_t pending_requests_ = 0; |
| - ParsedCertificateList results_; |
| - size_t current_result_ = 0; |
| + size_t current_request_ = 0; |
| DISALLOW_COPY_AND_ASSIGN(AiaRequest); |
| }; |
| -AiaRequest::AiaRequest(const CertIssuerSource::IssuerCallback& issuers_callback) |
| - : issuers_callback_(issuers_callback) {} |
| - |
| AiaRequest::~AiaRequest() = default; |
| -CompletionStatus AiaRequest::GetNext( |
| - scoped_refptr<ParsedCertificate>* out_cert) { |
| - if (HasNext()) { |
| - *out_cert = std::move(results_[current_result_++]); |
| - return CompletionStatus::SYNC; |
| +void AiaRequest::GetNext(ParsedCertificateList* out_certs) { |
| + // TODO(eroman): Rather than blocking in FIFO order, select the one that |
| + // completes first. |
| + while (current_request_ < cert_fetcher_requests_.size()) { |
| + Error error; |
| + std::vector<uint8_t> bytes; |
| + auto req = std::move(cert_fetcher_requests_[current_request_++]); |
| + req->WaitForResult(&error, &bytes); |
| + |
| + if (OnFetchCompleted(error, std::move(bytes), out_certs)) |
| + return; |
| } |
| - *out_cert = nullptr; |
| - if (pending_requests_) |
| - return CompletionStatus::ASYNC; |
| - return CompletionStatus::SYNC; |
| } |
| void AiaRequest::AddCertFetcherRequest( |
| std::unique_ptr<CertNetFetcher::Request> cert_fetcher_request) { |
| DCHECK(cert_fetcher_request); |
| cert_fetcher_requests_.push_back(std::move(cert_fetcher_request)); |
| - pending_requests_++; |
| } |
| -void AiaRequest::OnFetchCompleted(Error error, |
| - const std::vector<uint8_t>& fetched_bytes) { |
| - DCHECK_GT(pending_requests_, 0U); |
| - pending_requests_--; |
| - bool client_waiting_for_callback = !HasNext(); |
| +bool AiaRequest::OnFetchCompleted(Error error, |
|
mattm
2016/10/28 02:11:55
Wondering if this should be renamed, since it make
eroman
2016/11/23 22:10:21
Done -- renamed to AddCompletedFetchToResults()
|
| + std::vector<uint8_t> fetched_bytes, |
| + ParsedCertificateList* results) { |
| if (error != OK) { |
| // TODO(mattm): propagate error info. |
| LOG(ERROR) << "AiaRequest::OnFetchCompleted got error " << error; |
| - } else { |
| - // RFC 5280 section 4.2.2.1: |
| - // |
| - // Conforming applications that support HTTP or FTP for accessing |
| - // certificates MUST be able to accept individual DER encoded |
| - // certificates and SHOULD be able to accept "certs-only" CMS messages. |
| - // |
| - // TODO(mattm): Is supporting CMS message format important? |
| - // |
| - // TODO(mattm): Avoid copying bytes. Change the CertNetFetcher and |
| - // ParsedCertificate interface to allow passing through ownership of the |
| - // bytes. |
| - CertErrors errors; |
| - if (!ParsedCertificate::CreateAndAddToVector(fetched_bytes.data(), |
| - fetched_bytes.size(), {}, |
| - &results_, &errors)) { |
| - // TODO(crbug.com/634443): propagate error info. |
| - LOG(ERROR) << "Error parsing cert retrieved from AIA:\n" |
| - << errors.ToDebugString(); |
| - } |
| + return false; |
| } |
| - // If the client is waiting for results, need to run callback if: |
| - // * Some are available now. |
| - // * The last fetch finished, even with no results. (Client needs to know to |
| - // stop waiting.) |
| - if (client_waiting_for_callback && (HasNext() || pending_requests_ == 0)) |
| - issuers_callback_.Run(this); |
| + // RFC 5280 section 4.2.2.1: |
| + // |
| + // Conforming applications that support HTTP or FTP for accessing |
| + // certificates MUST be able to accept individual DER encoded |
| + // certificates and SHOULD be able to accept "certs-only" CMS messages. |
| + // |
| + // TODO(mattm): Is supporting CMS message format important? |
| + // |
| + // TODO(eroman): Avoid copying bytes in the certificate? |
| + CertErrors errors; |
| + if (!ParsedCertificate::CreateAndAddToVector( |
| + fetched_bytes.data(), fetched_bytes.size(), {}, results, &errors)) { |
| + // TODO(crbug.com/634443): propagate error info. |
| + LOG(ERROR) << "Error parsing cert retrieved from AIA:\n" |
| + << errors.ToDebugString(); |
| + return false; |
| + } |
| + |
| + return true; |
| } |
| } // namespace |
| @@ -116,10 +103,8 @@ void CertIssuerSourceAia::SyncGetIssuersOf(const ParsedCertificate* cert, |
| // CertIssuerSourceAia never returns synchronous results. |
| } |
| -void CertIssuerSourceAia::AsyncGetIssuersOf( |
| - const ParsedCertificate* cert, |
| - const IssuerCallback& issuers_callback, |
| - std::unique_ptr<Request>* out_req) { |
| +void CertIssuerSourceAia::AsyncGetIssuersOf(const ParsedCertificate* cert, |
| + std::unique_ptr<Request>* out_req) { |
| out_req->reset(); |
| if (!cert->has_authority_info_access()) |
| @@ -152,16 +137,14 @@ void CertIssuerSourceAia::AsyncGetIssuersOf( |
| if (urls.empty()) |
| return; |
| - std::unique_ptr<AiaRequest> aia_request(new AiaRequest(issuers_callback)); |
| + std::unique_ptr<AiaRequest> aia_request(new AiaRequest()); |
| for (const auto& url : urls) { |
| // TODO(mattm): add synchronous failure mode to FetchCaIssuers interface so |
| // that this doesn't need to wait for async callback just to tell that an |
| // URL has an unsupported scheme? |
| aia_request->AddCertFetcherRequest(cert_fetcher_->FetchCaIssuers( |
| - url, kTimeoutMilliseconds, kMaxResponseBytes, |
| - base::Bind(&AiaRequest::OnFetchCompleted, |
| - base::Unretained(aia_request.get())))); |
| + url, kTimeoutMilliseconds, kMaxResponseBytes)); |
| } |
| *out_req = std::move(aia_request); |