Store attachment crc in AttachmentStore

sync/internal_api/attachments/attachment_downloader_impl.cc

 // Copyright 2014 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 "sync/internal_api/public/attachments/attachment_downloader_impl.h"
 
+#include "base/base64.h"
 #include "base/bind.h"
 #include "base/message_loop/message_loop.h"
+#include "base/sys_byteorder.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
 #include "net/http/http_util.h"
 #include "net/url_request/url_fetcher.h"
 #include "sync/internal_api/public/attachments/attachment_uploader_impl.h"
+#include "sync/internal_api/public/attachments/attachment_util.h"
 #include "sync/protocol/sync.pb.h"
 #include "url/gurl.h"
 
 namespace syncer {
 
 struct AttachmentDownloaderImpl::DownloadState {
  public:
   DownloadState(const AttachmentId& attachment_id,
                 const AttachmentUrl& attachment_url);
 
@@ skipping 84 matching lines @@
   DCHECK(CalledOnValidThread());
   DCHECK(request == access_token_request_.get());
   access_token_request_.reset();
   StateList::const_iterator iter;
   // Without access token all downloads fail.
   for (iter = requests_waiting_for_access_token_.begin();
        iter != requests_waiting_for_access_token_.end();
        ++iter) {
     DownloadState* download_state = *iter;
     scoped_refptr<base::RefCountedString> null_attachment_data;
-    ReportResult(
-        *download_state, DOWNLOAD_TRANSIENT_ERROR, null_attachment_data);
+    ReportResult(*download_state, DOWNLOAD_TRANSIENT_ERROR,
+                 null_attachment_data, 0);
     DCHECK(state_map_.find(download_state->attachment_url) != state_map_.end());
     state_map_.erase(download_state->attachment_url);
   }
   requests_waiting_for_access_token_.clear();
 }
 
 void AttachmentDownloaderImpl::OnURLFetchComplete(
     const net::URLFetcher* source) {
   DCHECK(CalledOnValidThread());
 
   // Find DownloadState by url.
   AttachmentUrl url = source->GetOriginalURL().spec();
   StateMap::iterator iter = state_map_.find(url);
   DCHECK(iter != state_map_.end());
   const DownloadState& download_state = *iter->second;
   DCHECK(source == download_state.url_fetcher.get());
 
   DownloadResult result = DOWNLOAD_TRANSIENT_ERROR;
   scoped_refptr<base::RefCountedString> attachment_data;
+  uint32_t attachment_crc = 0;
 
   const int response_code = source->GetResponseCode();
   if (response_code == net::HTTP_OK) {
     std::string data_as_string;
     source->GetResponseAsString(&data_as_string);
-    if (VerifyHashIfPresent(*source, data_as_string)) {
+    attachment_data = base::RefCountedString::TakeString(&data_as_string);
+
+    attachment_crc = ComputeCrc32c(attachment_data);
+    uint32_t crc32c_from_headers = 0;
+    if (ExtractCrc32c(source->GetResponseHeaders(), &crc32c_from_headers) &&
+        attachment_crc != crc32c_from_headers) {
+      // Fail download only if there is useful crc in header and it doesn't

[maniscalco, 2014/11/11 00:44:54]

crc -> crc32c (and elsewhere in this comment)

[pavely, 2014/11/11 22:27:14]

Done.

+      // match data. All other cases are fine. When crc is not in headers
+      // locally calculated one will be  stored and used for further checks.

[maniscalco, 2014/11/11 00:44:54]

extra space between be and stored

[pavely, 2014/11/11 22:27:14]

Done.

+      result = DOWNLOAD_TRANSIENT_ERROR;
+    } else {
       result = DOWNLOAD_SUCCESS;
-      attachment_data = base::RefCountedString::TakeString(&data_as_string);
-    } else {
-      // TODO(maniscalco): Test me!
-      result = DOWNLOAD_TRANSIENT_ERROR;
     }
   } else if (response_code == net::HTTP_UNAUTHORIZED) {
     // Server tells us we've got a bad token so invalidate it.
     OAuth2TokenServiceRequest::InvalidateToken(token_service_provider_.get(),
                                                account_id_,
                                                oauth2_scopes_,
                                                download_state.access_token);
     // Fail the request, but indicate that it may be successful if retried.
     result = DOWNLOAD_TRANSIENT_ERROR;
   } else if (response_code == net::HTTP_FORBIDDEN) {
     // User is not allowed to use attachments.  Retrying won't help.
     result = DOWNLOAD_UNSPECIFIED_ERROR;
   } else if (response_code == net::URLFetcher::RESPONSE_CODE_INVALID) {
     result = DOWNLOAD_TRANSIENT_ERROR;
   }
-  ReportResult(download_state, result, attachment_data);
+  ReportResult(download_state, result, attachment_data, attachment_crc);
   state_map_.erase(iter);
 }
 
 scoped_ptr<net::URLFetcher> AttachmentDownloaderImpl::CreateFetcher(
     const AttachmentUrl& url,
     const std::string& access_token) {
   scoped_ptr<net::URLFetcher> url_fetcher(
       net::URLFetcher::Create(GURL(url), net::URLFetcher::GET, this));
   url_fetcher->SetAutomaticallyRetryOn5xx(false);
   const std::string auth_header("Authorization: Bearer " + access_token);
@@ skipping 13 matching lines @@
   // Start access token request if there is no active one.
   if (access_token_request_ == NULL) {
     access_token_request_ = OAuth2TokenServiceRequest::CreateAndStart(
         token_service_provider_.get(), account_id_, oauth2_scopes_, this);
   }
 }
 
 void AttachmentDownloaderImpl::ReportResult(
     const DownloadState& download_state,
     const DownloadResult& result,
-    const scoped_refptr<base::RefCountedString>& attachment_data) {
+    const scoped_refptr<base::RefCountedString>& attachment_data,
+    uint32_t attachment_crc) {

[maniscalco, 2014/11/11 00:44:54]

crc -> crc32c (and elsewhere in this file)

[pavely, 2014/11/11 22:27:15]

Done.

   std::vector<DownloadCallback>::const_iterator iter;
   for (iter = download_state.user_callbacks.begin();
        iter != download_state.user_callbacks.end();
        ++iter) {
     scoped_ptr<Attachment> attachment;
     if (result == DOWNLOAD_SUCCESS) {
-      attachment.reset(new Attachment(Attachment::CreateWithId(
-          download_state.attachment_id, attachment_data)));
+      attachment.reset(new Attachment(Attachment::RestoreExisting(
+          download_state.attachment_id, attachment_data, attachment_crc)));
     }
 
     base::MessageLoop::current()->PostTask(
         FROM_HERE, base::Bind(*iter, result, base::Passed(&attachment)));
   }
 }
 
-bool AttachmentDownloaderImpl::VerifyHashIfPresent(
-    const net::URLFetcher& fetcher,
-    const std::string& data) {
-  const net::HttpResponseHeaders* headers = fetcher.GetResponseHeaders();
-  if (!headers) {
-    // No headers?  It passes.
-    return true;
-  }
-
-  std::string value;
-  if (!ExtractCrc32c(*headers, &value)) {
-    // No crc32c?  It passes.
-    return true;
-  }
-
-  if (value ==
-      AttachmentUploaderImpl::ComputeCrc32cHash(data.data(), data.size())) {
-    return true;
-  } else {
-    return false;
-  }
-}
-
 bool AttachmentDownloaderImpl::ExtractCrc32c(
-    const net::HttpResponseHeaders& headers,
-    std::string* crc32c) {
+    const net::HttpResponseHeaders* headers,
+    uint32_t* crc32c) {
   DCHECK(crc32c);
+  if (!headers) {
+    return false;
+  }
+
+  std::string crc32c_encoded;
   std::string header_value;
   void* iter = NULL;
   // Iterate over all matching headers.
-  while (headers.EnumerateHeader(&iter, "x-goog-hash", &header_value)) {
+  while (headers->EnumerateHeader(&iter, "x-goog-hash", &header_value)) {
     // Because EnumerateHeader is smart about list values, header_value will
     // either be empty or a single name=value pair.
     net::HttpUtil::NameValuePairsIterator pair_iter(
         header_value.begin(), header_value.end(), ',');
     if (pair_iter.GetNext()) {
       if (pair_iter.name() == "crc32c") {
-        *crc32c = pair_iter.value();
+        crc32c_encoded = pair_iter.value();
         DCHECK(!pair_iter.GetNext());
-        return true;
+        break;
       }
     }
   }
+  // Check if header was found
+  if (crc32c_encoded.empty())
+    return false;
+  std::string crc32c_raw;
+  if (!base::Base64Decode(crc32c_encoded, &crc32c_raw))
+    return false;
 
-  return false;
+  if (crc32c_raw.size() != sizeof(*crc32c))
+    return false;
+
+  *crc32c =
+      base::NetToHost32(*reinterpret_cast<const uint32_t*>(crc32c_raw.c_str()));
+  return true;
 }
 
 }  // namespace syncer