Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(143)

Unified Diff: net/cert/internal/cert_issuer_source_aia.cc

Issue 2453093004: Remove dependence on a message loop for net::PathBuilder. (Closed)
Patch Set: Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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);

Powered by Google App Engine
This is Rietveld 408576698