summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--sync/BUILD.gn1
-rw-r--r--sync/internal_api/attachments/DEPS1
-rw-r--r--sync/internal_api/attachments/attachment_uploader_impl.cc29
-rw-r--r--sync/internal_api/attachments/attachment_uploader_impl_unittest.cc132
-rw-r--r--sync/internal_api/public/attachments/attachment_uploader_impl.h7
-rw-r--r--sync/sync.gyp1
6 files changed, 107 insertions, 64 deletions
diff --git a/sync/BUILD.gn b/sync/BUILD.gn
index d32575b..9b187cf 100644
--- a/sync/BUILD.gn
+++ b/sync/BUILD.gn
@@ -399,6 +399,7 @@ source_set("sync_core") {
"//google_apis",
"//net",
"//sql",
+ "//third_party/leveldatabase",
"//third_party/zlib",
"//url",
"//sync/protocol",
diff --git a/sync/internal_api/attachments/DEPS b/sync/internal_api/attachments/DEPS
index 50828ba..6fbaa4d 100644
--- a/sync/internal_api/attachments/DEPS
+++ b/sync/internal_api/attachments/DEPS
@@ -1,3 +1,4 @@
include_rules = [
"+sync/api/attachments",
+ "+third_party/leveldatabase",
]
diff --git a/sync/internal_api/attachments/attachment_uploader_impl.cc b/sync/internal_api/attachments/attachment_uploader_impl.cc
index 5ca3304..9c7276f 100644
--- a/sync/internal_api/attachments/attachment_uploader_impl.cc
+++ b/sync/internal_api/attachments/attachment_uploader_impl.cc
@@ -4,9 +4,14 @@
#include "sync/internal_api/public/attachments/attachment_uploader_impl.h"
+#include "base/base64.h"
#include "base/bind.h"
+#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/message_loop/message_loop.h"
+#include "base/strings/string_piece.h"
+#include "base/strings/stringprintf.h"
+#include "base/sys_byteorder.h"
#include "base/threading/non_thread_safe.h"
#include "google_apis/gaia/gaia_constants.h"
#include "net/base/load_flags.h"
@@ -15,6 +20,7 @@
#include "net/url_request/url_fetcher_delegate.h"
#include "sync/api/attachments/attachment.h"
#include "sync/protocol/sync.pb.h"
+#include "third_party/leveldatabase/src/util/crc32c.h"
namespace {
@@ -201,12 +207,15 @@ void AttachmentUploaderImpl::UploadState::OnGetTokenSuccess(
fetcher_->SetUploadData(kContentType, upload_content);
const std::string auth_header("Authorization: Bearer " + access_token_);
fetcher_->AddExtraRequestHeader(auth_header);
+ // TODO(maniscalco): Consider computing the hash once and storing the value as
+ // a new field in the Attachment object to avoid recomputing when an upload
+ // fails and is retried (bug 417794).
+ fetcher_->AddExtraRequestHeader(ComputeHashHeader(memory));
fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES |
net::LOAD_DO_NOT_SEND_COOKIES |
net::LOAD_DISABLE_CACHE);
- // TODO(maniscalco): Set an appropriate headers (User-Agent, Content-type, and
- // Content-length) on the request and include the content's MD5,
- // AttachmentId's unique_id and the "sync birthday" (bug 371521).
+ // TODO(maniscalco): Set appropriate headers (e.g. User-Agent) on the request
+ // and include the "sync birthday" (bug 371521).
fetcher_->Start();
}
@@ -334,4 +343,18 @@ void AttachmentUploaderImpl::OnUploadStateStopped(const UniqueId& unique_id) {
}
}
+std::string AttachmentUploaderImpl::ComputeHashHeader(
+ const scoped_refptr<base::RefCountedMemory>& memory) {
+ // Generate an X-Goog-Hash header containing the object's crc32c, big-endian,
+ // base64 encoded. See also
+ // https://cloud.google.com/storage/docs/reference-headers#xgooghash
+ const uint32_t crc32c_big_endian = base::HostToNet32(
+ leveldb::crc32c::Value(memory->front_as<char>(), memory->size()));
+ const base::StringPiece raw(reinterpret_cast<const char*>(&crc32c_big_endian),
+ sizeof(crc32c_big_endian));
+ std::string encoded;
+ base::Base64Encode(raw, &encoded);
+ return base::StringPrintf("X-Goog-Hash: crc32c=%s", encoded.c_str());
+}
+
} // namespace syncer
diff --git a/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc b/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc
index e682253..9bfbb4e75 100644
--- a/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc
+++ b/sync/internal_api/attachments/attachment_uploader_impl_unittest.cc
@@ -33,6 +33,10 @@ const char kAttachmentData[] = "some data";
const char kAccountId[] = "some-account-id";
const char kAccessToken[] = "some-access-token";
const char kAuthorization[] = "Authorization";
+const char kContentLength[] = "Content-Length";
+const char kContentType[] = "Content-Type";
+const char kContentTypeValue[] = "application/octet-stream";
+const char kXGoogHash[] = "X-Goog-Hash";
const char kAttachments[] = "/attachments/";
} // namespace
@@ -184,6 +188,11 @@ class AttachmentUploaderImplTest : public testing::Test,
// Run the message loop until UploadDone has been invoked |num_uploads| times.
void RunAndWaitFor(int num_uploads);
+ // Upload an attachment and have the server respond with |status_code|.
+ //
+ // Returns the attachment that was uploaded.
+ Attachment UploadAndRespondWith(const net::HttpStatusCode& status_code);
+
scoped_ptr<AttachmentUploader>& uploader();
const AttachmentUploader::UploadCallback& upload_callback() const;
std::vector<HttpRequest>& http_requests_received();
@@ -297,6 +306,17 @@ void AttachmentUploaderImplTest::RunAndWaitFor(int num_uploads) {
}
}
+Attachment AttachmentUploaderImplTest::UploadAndRespondWith(
+ const net::HttpStatusCode& status_code) {
+ token_service().AddAccount(kAccountId);
+ request_handler().SetStatusCode(status_code);
+ scoped_refptr<base::RefCountedString> some_data(new base::RefCountedString);
+ some_data->data() = kAttachmentData;
+ Attachment attachment = Attachment::Create(some_data);
+ uploader()->UploadAttachment(attachment, upload_callback());
+ return attachment;
+}
+
scoped_ptr<AttachmentUploader>& AttachmentUploaderImplTest::uploader() {
return uploader_;
}
@@ -419,13 +439,7 @@ TEST_F(AttachmentUploaderImplTest, GetURLForAttachmentId_PathAndSlash) {
// Token is requested, token is returned, HTTP request is made, attachment is
// received by server.
TEST_F(AttachmentUploaderImplTest, UploadAttachment_HappyCase) {
- token_service().AddAccount(kAccountId);
- request_handler().SetStatusCode(net::HTTP_OK);
-
- scoped_refptr<base::RefCountedString> some_data(new base::RefCountedString);
- some_data->data() = kAttachmentData;
- Attachment attachment = Attachment::Create(some_data);
- uploader()->UploadAttachment(attachment, upload_callback());
+ Attachment attachment = UploadAndRespondWith(net::HTTP_OK);
// Run until the done callback is invoked.
RunAndWaitFor(1);
@@ -445,23 +459,45 @@ TEST_F(AttachmentUploaderImplTest, UploadAttachment_HappyCase) {
EXPECT_EQ(expected_relative_url, http_request.relative_url);
EXPECT_TRUE(http_request.has_content);
EXPECT_EQ(kAttachmentData, http_request.content);
- const std::string header_name(kAuthorization);
- const std::string header_value(std::string("Bearer ") + kAccessToken);
+}
+
+// Verify the request contains the appropriate headers.
+TEST_F(AttachmentUploaderImplTest, UploadAttachment_Headers) {
+ Attachment attachment = UploadAndRespondWith(net::HTTP_OK);
+
+ // Run until the done callback is invoked.
+ RunAndWaitFor(1);
+
+ // See that the done callback was invoked with the right arguments.
+ ASSERT_EQ(1U, upload_results().size());
+ EXPECT_EQ(AttachmentUploader::UPLOAD_SUCCESS, upload_results()[0]);
+ ASSERT_EQ(1U, attachment_ids().size());
+ EXPECT_EQ(attachment.GetId(), attachment_ids()[0]);
+
+ // See that the HTTP server received one request.
+ ASSERT_EQ(1U, http_requests_received().size());
+ const HttpRequest& http_request = http_requests_received().front();
+
+ const std::string auth_header_name(kAuthorization);
+ const std::string auth_header_value(std::string("Bearer ") + kAccessToken);
+
+ EXPECT_THAT(
+ http_request.headers,
+ testing::Contains(testing::Pair(kAuthorization, auth_header_value)));
+ EXPECT_THAT(http_request.headers,
+ testing::Contains(testing::Key(kContentLength)));
+ EXPECT_THAT(
+ http_request.headers,
+ testing::Contains(testing::Pair(kContentType, kContentTypeValue)));
EXPECT_THAT(http_request.headers,
- testing::Contains(testing::Pair(header_name, header_value)));
+ testing::Contains(testing::Key(kXGoogHash)));
}
// Verify two overlapping calls to upload the same attachment result in only one
// HTTP request.
TEST_F(AttachmentUploaderImplTest, UploadAttachment_Collapse) {
- token_service().AddAccount(kAccountId);
- request_handler().SetStatusCode(net::HTTP_OK);
-
- scoped_refptr<base::RefCountedString> some_data(new base::RefCountedString);
- some_data->data() = kAttachmentData;
- Attachment attachment1 = Attachment::Create(some_data);
+ Attachment attachment1 = UploadAndRespondWith(net::HTTP_OK);
Attachment attachment2 = attachment1;
- uploader()->UploadAttachment(attachment1, upload_callback());
uploader()->UploadAttachment(attachment2, upload_callback());
// Wait for upload_callback() to be invoked twice.
@@ -474,17 +510,12 @@ TEST_F(AttachmentUploaderImplTest, UploadAttachment_Collapse) {
// uplaod finishes. We do this by issuing two non-overlapping uploads for the
// same attachment and see that it results in two HTTP requests.
TEST_F(AttachmentUploaderImplTest, UploadAttachment_CleanUpAfterUpload) {
- token_service().AddAccount(kAccountId);
- request_handler().SetStatusCode(net::HTTP_OK);
-
- scoped_refptr<base::RefCountedString> some_data(new base::RefCountedString);
- some_data->data() = kAttachmentData;
- Attachment attachment1 = Attachment::Create(some_data);
- Attachment attachment2 = attachment1;
- uploader()->UploadAttachment(attachment1, upload_callback());
+ Attachment attachment1 = UploadAndRespondWith(net::HTTP_OK);
// Wait for upload_callback() to be invoked before starting the second upload.
RunAndWaitFor(1);
+
+ Attachment attachment2 = attachment1;
uploader()->UploadAttachment(attachment2, upload_callback());
// Wait for upload_callback() to be invoked a second time.
@@ -519,13 +550,7 @@ TEST_F(AttachmentUploaderImplTest, UploadAttachment_FailToGetToken) {
// Verify behavior when the server returns "503 Service Unavailable".
TEST_F(AttachmentUploaderImplTest, UploadAttachment_ServiceUnavilable) {
- token_service().AddAccount(kAccountId);
- request_handler().SetStatusCode(net::HTTP_SERVICE_UNAVAILABLE);
-
- scoped_refptr<base::RefCountedString> some_data(new base::RefCountedString);
- some_data->data() = kAttachmentData;
- Attachment attachment = Attachment::Create(some_data);
- uploader()->UploadAttachment(attachment, upload_callback());
+ Attachment attachment = UploadAndRespondWith(net::HTTP_SERVICE_UNAVAILABLE);
RunAndWaitFor(1);
@@ -544,11 +569,6 @@ TEST_F(AttachmentUploaderImplTest, UploadAttachment_ServiceUnavilable) {
EXPECT_EQ(expected_relative_url, http_request.relative_url);
EXPECT_TRUE(http_request.has_content);
EXPECT_EQ(kAttachmentData, http_request.content);
- std::string expected_header(kAuthorization);
- const std::string header_name(kAuthorization);
- const std::string header_value(std::string("Bearer ") + kAccessToken);
- EXPECT_THAT(http_request.headers,
- testing::Contains(testing::Pair(header_name, header_value)));
// See that we did not invalidate the token.
ASSERT_EQ(0, token_service().num_invalidate_token());
@@ -556,13 +576,7 @@ TEST_F(AttachmentUploaderImplTest, UploadAttachment_ServiceUnavilable) {
// Verify that we "403 Forbidden" as a non-transient error.
TEST_F(AttachmentUploaderImplTest, UploadAttachment_Forbidden) {
- token_service().AddAccount(kAccountId);
- request_handler().SetStatusCode(net::HTTP_FORBIDDEN);
-
- scoped_refptr<base::RefCountedString> some_data(new base::RefCountedString);
- some_data->data() = kAttachmentData;
- Attachment attachment = Attachment::Create(some_data);
- uploader()->UploadAttachment(attachment, upload_callback());
+ Attachment attachment = UploadAndRespondWith(net::HTTP_FORBIDDEN);
RunAndWaitFor(1);
@@ -581,11 +595,6 @@ TEST_F(AttachmentUploaderImplTest, UploadAttachment_Forbidden) {
EXPECT_EQ(expected_relative_url, http_request.relative_url);
EXPECT_TRUE(http_request.has_content);
EXPECT_EQ(kAttachmentData, http_request.content);
- std::string expected_header(kAuthorization);
- const std::string header_name(kAuthorization);
- const std::string header_value(std::string("Bearer ") + kAccessToken);
- EXPECT_THAT(http_request.headers,
- testing::Contains(testing::Pair(header_name, header_value)));
// See that we did not invalidate the token.
ASSERT_EQ(0, token_service().num_invalidate_token());
@@ -594,13 +603,7 @@ TEST_F(AttachmentUploaderImplTest, UploadAttachment_Forbidden) {
// Verify that when we receive an "401 Unauthorized" we invalidate the access
// token.
TEST_F(AttachmentUploaderImplTest, UploadAttachment_BadToken) {
- token_service().AddAccount(kAccountId);
- request_handler().SetStatusCode(net::HTTP_UNAUTHORIZED);
-
- scoped_refptr<base::RefCountedString> some_data(new base::RefCountedString);
- some_data->data() = kAttachmentData;
- Attachment attachment = Attachment::Create(some_data);
- uploader()->UploadAttachment(attachment, upload_callback());
+ Attachment attachment = UploadAndRespondWith(net::HTTP_UNAUTHORIZED);
RunAndWaitFor(1);
@@ -619,16 +622,23 @@ TEST_F(AttachmentUploaderImplTest, UploadAttachment_BadToken) {
EXPECT_EQ(expected_relative_url, http_request.relative_url);
EXPECT_TRUE(http_request.has_content);
EXPECT_EQ(kAttachmentData, http_request.content);
- std::string expected_header(kAuthorization);
- const std::string header_name(kAuthorization);
- const std::string header_value(std::string("Bearer ") + kAccessToken);
- EXPECT_THAT(http_request.headers,
- testing::Contains(testing::Pair(header_name, header_value)));
// See that we invalidated the token.
ASSERT_EQ(1, token_service().num_invalidate_token());
}
+TEST_F(AttachmentUploaderImplTest, ComputeHashHeader) {
+ scoped_refptr<base::RefCountedString> empty(new base::RefCountedString);
+ empty->data() = "";
+ EXPECT_EQ("X-Goog-Hash: crc32c=AAAAAA==",
+ AttachmentUploaderImpl::ComputeHashHeader(empty));
+
+ scoped_refptr<base::RefCountedString> hello_world(new base::RefCountedString);
+ hello_world->data() = "hello world";
+ EXPECT_EQ("X-Goog-Hash: crc32c=yZRlqg==",
+ AttachmentUploaderImpl::ComputeHashHeader(hello_world));
+}
+
// TODO(maniscalco): Add test case for when we are uploading an attachment that
// already exists. 409 Conflict? (bug 379825)
diff --git a/sync/internal_api/public/attachments/attachment_uploader_impl.h b/sync/internal_api/public/attachments/attachment_uploader_impl.h
index a88c967..674443f 100644
--- a/sync/internal_api/public/attachments/attachment_uploader_impl.h
+++ b/sync/internal_api/public/attachments/attachment_uploader_impl.h
@@ -6,6 +6,7 @@
#define SYNC_INTERNAL_API_PUBLIC_ATTACHMENTS_ATTACHMENT_UPLOADER_IMPL_H_
#include "base/containers/scoped_ptr_hash_map.h"
+#include "base/gtest_prod_util.h"
#include "base/threading/non_thread_safe.h"
#include "google_apis/gaia/oauth2_token_service_request.h"
#include "net/url_request/url_request_context_getter.h"
@@ -51,12 +52,18 @@ class SYNC_EXPORT AttachmentUploaderImpl : public AttachmentUploader,
const AttachmentId& attachment_id);
private:
+ FRIEND_TEST_ALL_PREFIXES(AttachmentUploaderImplTest, ComputeHashHeader);
+
class UploadState;
typedef std::string UniqueId;
typedef base::ScopedPtrHashMap<UniqueId, UploadState> StateMap;
void OnUploadStateStopped(const UniqueId& unique_id);
+ // Returns an X-Goog-Hash header for |memory|. Potentially expensive.
+ static std::string ComputeHashHeader(
+ const scoped_refptr<base::RefCountedMemory>& memory);
+
GURL sync_service_url_;
scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
std::string account_id_;
diff --git a/sync/sync.gyp b/sync/sync.gyp
index c5532be..55ad858 100644
--- a/sync/sync.gyp
+++ b/sync/sync.gyp
@@ -48,6 +48,7 @@
'../google_apis/google_apis.gyp:google_apis',
'../net/net.gyp:net',
'../sql/sql.gyp:sql',
+ '../third_party/leveldatabase/leveldatabase.gyp:leveldatabase',
'../third_party/protobuf/protobuf.gyp:protobuf_lite',
'../third_party/zlib/zlib.gyp:zlib',
'../url/url.gyp:url_lib',
generated by cgit v1.1 at 2024-06-18 18:35:22 +0000