diff options
Diffstat (limited to 'sync')
-rw-r--r-- | sync/BUILD.gn | 1 | ||||
-rw-r--r-- | sync/internal_api/attachments/DEPS | 1 | ||||
-rw-r--r-- | sync/internal_api/attachments/attachment_uploader_impl.cc | 29 | ||||
-rw-r--r-- | sync/internal_api/attachments/attachment_uploader_impl_unittest.cc | 132 | ||||
-rw-r--r-- | sync/internal_api/public/attachments/attachment_uploader_impl.h | 7 | ||||
-rw-r--r-- | sync/sync.gyp | 1 |
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', |