Certificate Transparency: Fetching of Signed Tree Heads (DRAFT)

components/certificate_transparency/log_proofs_fetcher.h

+// Copyright 2015 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.
+
+#ifndef COMPONENTS_CERTIFICATE_TRANSPARENCY_LOG_PROOFS_FETCHER_H_
+#define COMPONENTS_CERTIFICATE_TRANSPARENCY_LOG_PROOFS_FETCHER_H_
+
+#include <map>
+#include <string>
+
+#include "base/memory/scoped_ptr.h"
+#include "net/base/io_buffer.h"

[Ryan Sleevi, 2015/04/24 10:42:07]

You don't need to include this; you can forward declare IOBufferWithSize

[Eran Messeri, 2015/06/18 15:18:41]

Done.

+#include "net/url_request/url_request.h"
+

[Ryan Sleevi, 2015/04/24 10:42:07]

IWYU: Missing ref_counted

[Eran Messeri, 2015/06/18 15:18:41]

Done.

+namespace base {
+class Value;
+}

[Ryan Sleevi, 2015/04/24 10:42:07]

// namespace base

[Eran Messeri, 2015/06/18 15:18:41]

Done.

+
+namespace net {
+class URLRequestContext;
+class CTLogVerifier;
+
+namespace ct {
+struct SignedTreeHead;
+}  // namespace ct
+
+}  // namespace net
+
+namespace certificate_transparency {
+
+class LogProofsFetcher : public net::URLRequest::Delegate {

[Ryan Sleevi, 2015/04/24 10:42:07]

naming nit: It's weird to call this plural, and I think reads weird.

From the point of view of consumers, they fetch a single STH (re: line 37), so
this probably makes more sense as LogProofFetcher [even if it's used to fetch
multiple proofs through multiple calls to FetchSTH]

[Ryan Sleevi, 2015/04/24 10:42:07]

style: documentation

[Eran Messeri, 2015/06/18 15:18:41]

Done.

[Eran Messeri, 2015/06/18 15:18:42]

Done.

+ public:
+  explicit LogProofsFetcher(net::URLRequestContext* request_context);
+
+  ~LogProofsFetcher() override;

[Ryan Sleevi, 2015/04/24 10:42:07]

delete newline on 34?

[Eran Messeri, 2015/06/18 15:18:41]

Done.

+
+  void FetchSTH(net::CTLogVerifier* verifier);

[Ryan Sleevi, 2015/04/24 10:42:07]

documentation

[Eran Messeri, 2015/06/18 15:18:42]

Done.

+
+  // net::URLRequest::Delegate
+  void OnResponseStarted(net::URLRequest* request) override;
+  void OnReadCompleted(net::URLRequest* request, int bytes_read) override;
+
+ private:
+  scoped_ptr<net::URLRequest> CreateURLRequest(GURL fetch_sth_url);

[Ryan Sleevi, 2015/04/24 10:42:07]

pass as const-ref

[Eran Messeri, 2015/06/18 15:18:42]

Done.

+
+  // Performs post-report cleanup.
+  void RequestComplete(net::URLRequest* request, int bytes_read);
+  void RequestCleanup(net::URLRequest* request);

[Ryan Sleevi, 2015/04/24 10:42:07]

naming nit: RequestCleanup -> CleanupRequest (RequestCleanup can either be
Verb-Noun or Noun-Verb, and has different meanings; CleanupRequest makes it
clear it's Verb-Noun)

[Eran Messeri, 2015/06/18 15:18:42]

Done.

+
+  struct NET_EXPORT_PRIVATE FetchParams {

[Ryan Sleevi, 2015/04/24 10:42:07]

1) NET_EXPORT_PRIVATE isn't needed
2) You can forward declare this, since you use a pointer in line 59
3) Ordering -
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Declaration_O...

[Eran Messeri, 2015/06/18 15:18:41]

Done.

+    explicit FetchParams(net::CTLogVerifier* verifier);
+    ~FetchParams();
+
+    net::CTLogVerifier* verifier;
+    scoped_refptr<net::IOBufferWithSize> sth_buffer;
+    int read_bytes;
+  };
+
+  typedef std::map<net::URLRequest*, FetchParams*> RequestsMap;

[Ryan Sleevi, 2015/04/24 10:42:07]

You don't actually need this typedef, do you, since you can use auto & friends

[Eran Messeri, 2015/06/18 15:18:41]

Done.

+
+  net::URLRequestContext* const request_context_;
+
+  // Owns the contained requests.
+  RequestsMap inflight_requests_;
+
+  DISALLOW_COPY_AND_ASSIGN(LogProofsFetcher);
+};
+
+class CTLogResponseParser {

[Ryan Sleevi, 2015/04/24 10:42:07]

style: move to a separate file.

[Eran Messeri, 2015/06/18 15:18:41]

Done.

+ public:
+  // Extracts STH from the json struct

[Ryan Sleevi, 2015/04/24 10:42:07]

s/json/JSON/

[Eran Messeri, 2015/06/18 15:18:41]

Done.

+  static bool FillSignedTreeHead(const base::StringPiece& json_sth,
+                                 net::ct::SignedTreeHead* sth);
+};
+
+}  // namespace certificate_transparency
+
+#endif  // COMPONENTS_CERTIFICATE_TRANSPARENCY_LOG_PROOFS_FETCHER_H_