Cleanup and minor refactoring of LogDnsClient

components/certificate_transparency/log_dns_client.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/certificate_transparency/log_dns_client.h"
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/format_macros.h"
 #include "base/location.h"
@@ skipping 89 matching lines @@
 
 // Encapsulates the state machine required to get an audit proof from a Merkle
 // leaf hash. This requires a DNS request to obtain the leaf index, then a
 // series of DNS requests to get the nodes of the proof.
 class LogDnsClient::AuditProofQuery {
  public:
   // The LogDnsClient is guaranteed to outlive the AuditProofQuery, so it's safe
   // to leave ownership of |dns_client| with LogDnsClient.
   AuditProofQuery(net::DnsClient* dns_client,
                   const std::string& domain_for_log,
-                  uint64_t tree_size,
                   const net::NetLogWithSource& net_log);
 
-  // Begins the process of getting an audit |proof| for the CT log entry with a
-  // leaf hash of |leaf_hash|. If it cannot be obtained synchronously,
-  // net::ERR_IO_PENDING will be returned and |callback| will be invoked when
-  // the operation has completed asynchronously.
-  // Ownership of |proof| remains with the caller, and it must not be deleted
-  // until the operation is complete.
+  // Begins the process of getting an audit proof for the CT log entry with a
+  // leaf hash of |leaf_hash|. The proof will be for a tree of size |tree_size|.
+  // If it cannot be obtained synchronously, net::ERR_IO_PENDING will be
+  // returned and |callback| will be invoked when the operation has completed
+  // asynchronously. Ownership of |proof| remains with the caller, and it must
+  // not be deleted until the operation is complete.
   net::Error Start(std::string leaf_hash,
+                   uint64_t tree_size,
                    const net::CompletionCallback& callback,
-                   net::ct::MerkleAuditProof* proof);
+                   net::ct::MerkleAuditProof* out_proof);
 
  private:
   enum class State {
     NONE,
     REQUEST_LEAF_INDEX,
     REQUEST_LEAF_INDEX_COMPLETE,
     REQUEST_AUDIT_PROOF_NODES,
     REQUEST_AUDIT_PROOF_NODES_COMPLETE,
   };
 
@@ skipping 39 matching lines @@
   // Returns true if the request could be started.
   // OnDnsTransactionComplete() will be invoked with the result of the request.
   bool StartDnsTransaction(const std::string& qname);
 
   // The next state that this query will enter.
   State next_state_;
   // The DNS domain of the CT log that is being queried.
   std::string domain_for_log_;
   // The Merkle leaf hash of the CT log entry an audit proof is required for.
   std::string leaf_hash_;
-  // The size of the CT log's tree, from which the proof is requested.
-  // TODO(robpercival): Remove |tree_size| once |proof_| has a tree_size member.
-  uint64_t tree_size_;
   // The audit proof to populate.
   net::ct::MerkleAuditProof* proof_;
   // The callback to invoke when the query is complete.
   net::CompletionCallback callback_;
   // The DnsClient to use for sending DNS requests to the CT log.
   net::DnsClient* dns_client_;
   // The most recent DNS request. Null if no DNS requests have been made.
   std::unique_ptr<net::DnsTransaction> current_dns_transaction_;
   // The most recent DNS response. Only valid so long as the corresponding DNS
   // request is stored in |current_dns_transaction_|.
   const net::DnsResponse* last_dns_response_;
   // The NetLog that DNS transactions will log to.
   net::NetLogWithSource net_log_;
   // Produces WeakPtrs to |this| for binding callbacks.
   base::WeakPtrFactory<AuditProofQuery> weak_ptr_factory_;
 };
 
 LogDnsClient::AuditProofQuery::AuditProofQuery(
     net::DnsClient* dns_client,
     const std::string& domain_for_log,
-    uint64_t tree_size,
     const net::NetLogWithSource& net_log)
     : next_state_(State::NONE),
       domain_for_log_(domain_for_log),
-      tree_size_(tree_size),
       dns_client_(dns_client),
       net_log_(net_log),
       weak_ptr_factory_(this) {
   DCHECK(dns_client_);
   DCHECK(!domain_for_log_.empty());
 }
 
 // |leaf_hash| is not a const-ref to allow callers to std::move that string into
 // the method, avoiding the need to make a copy.
 net::Error LogDnsClient::AuditProofQuery::Start(
     std::string leaf_hash,
+    uint64_t tree_size,
     const net::CompletionCallback& callback,
     net::ct::MerkleAuditProof* proof) {
   // It should not already be in progress.
   DCHECK_EQ(State::NONE, next_state_);
   proof_ = proof;
+  proof_->tree_size = tree_size;
   leaf_hash_ = std::move(leaf_hash);
   callback_ = callback;
   // The first step in the query is to request the leaf index corresponding to
   // |leaf_hash| from the CT log.
   next_state_ = State::REQUEST_LEAF_INDEX;
   // Begin the state machine.
   return DoLoop(net::OK);
 }
 
 net::Error LogDnsClient::AuditProofQuery::DoLoop(net::Error result) {
@@ skipping 70 matching lines @@
   if (!ParseLeafIndex(*last_dns_response_, &proof_->leaf_index)) {
     return net::ERR_DNS_MALFORMED_RESPONSE;
   }
 
   // Reject leaf index if it is out-of-range.
   // This indicates either:
   // a) the wrong tree_size was provided.
   // b) the wrong leaf hash was provided.
   // c) there is a bug server-side.
   // The first two are more likely, so return ERR_INVALID_ARGUMENT.
-  if (proof_->leaf_index >= tree_size_) {
+  if (proof_->leaf_index >= proof_->tree_size) {
     return net::ERR_INVALID_ARGUMENT;
   }
 
   next_state_ = State::REQUEST_AUDIT_PROOF_NODES;
   return net::OK;
 }
 
 net::Error LogDnsClient::AuditProofQuery::RequestAuditProofNodes() {
   // Test pre-conditions (should be guaranteed by DNS response validation).
-  if (proof_->leaf_index >= tree_size_ ||
-      proof_->nodes.size() >=
-          net::ct::CalculateAuditPathLength(proof_->leaf_index, tree_size_)) {
+  if (proof_->leaf_index >= proof_->tree_size ||
+      proof_->nodes.size() >= net::ct::CalculateAuditPathLength(
+                                  proof_->leaf_index, proof_->tree_size)) {
     return net::ERR_UNEXPECTED;
   }
 
   std::string qname = base::StringPrintf(
       "%zu.%" PRIu64 ".%" PRIu64 ".tree.%s.", proof_->nodes.size(),
-      proof_->leaf_index, tree_size_, domain_for_log_.c_str());
+      proof_->leaf_index, proof_->tree_size, domain_for_log_.c_str());
 
   if (!StartDnsTransaction(qname)) {
     return net::ERR_NAME_RESOLUTION_FAILED;
   }
 
   next_state_ = State::REQUEST_AUDIT_PROOF_NODES_COMPLETE;
   return net::ERR_IO_PENDING;
 }
 
 net::Error LogDnsClient::AuditProofQuery::RequestAuditProofNodesComplete(
     net::Error result) {
   if (result != net::OK) {
     return result;
   }
 
   const uint64_t audit_path_length =
-      net::ct::CalculateAuditPathLength(proof_->leaf_index, tree_size_);
+      net::ct::CalculateAuditPathLength(proof_->leaf_index, proof_->tree_size);
 
   // The calculated |audit_path_length| can't ever be greater than 64, so
   // deriving the amount of memory to reserve from the untrusted |leaf_index|
   // is safe.
   proof_->nodes.reserve(audit_path_length);
 
   DCHECK(last_dns_response_);
   if (!ParseAuditPath(*last_dns_response_, proof_)) {
     return net::ERR_DNS_MALFORMED_RESPONSE;
   }
@@ skipping 64 matching lines @@
 
   if (domain_for_log.empty() || leaf_hash.size() != crypto::kSHA256Length) {
     return net::ERR_INVALID_ARGUMENT;
   }
 
   if (HasMaxConcurrentQueriesInProgress()) {
     return net::ERR_TEMPORARILY_THROTTLED;
   }
 
   AuditProofQuery* query = new AuditProofQuery(
-      dns_client_.get(), domain_for_log.as_string(), tree_size, net_log_);
+      dns_client_.get(), domain_for_log.as_string(), net_log_);
   // Transfers ownership of |query| to |audit_proof_queries_|.
   audit_proof_queries_.emplace_back(query);
 
-  return query->Start(std::move(leaf_hash),
+  return query->Start(std::move(leaf_hash), tree_size,
                       base::Bind(&LogDnsClient::QueryAuditProofComplete,
                                  weak_ptr_factory_.GetWeakPtr(),
                                  base::Unretained(query), callback),
                       proof);
 }
 
 void LogDnsClient::QueryAuditProofComplete(
     AuditProofQuery* query,
     const net::CompletionCallback& callback,
     int net_error) {
@@ skipping 23 matching lines @@
 }
 
 void LogDnsClient::UpdateDnsConfig() {
   net::DnsConfig config;
   net::NetworkChangeNotifier::GetDnsConfig(&config);
   if (config.IsValid())
     dns_client_->SetConfig(config);
 }
 
 }  // namespace certificate_transparency