From 1b43e3a6e46def7a50df4d6be81a3dd4ac977e71 Mon Sep 17 00:00:00 2001 From: "guangjun.hgj" Date: Wed, 14 Jul 2021 15:44:15 +0800 Subject: [PATCH 1/3] add CURLE_SEND_FAIL_REWIND into retryable error. --- sdk/src/client/ClientConfiguration.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/src/client/ClientConfiguration.cc b/sdk/src/client/ClientConfiguration.cc index 698420c..d763626 100644 --- a/sdk/src/client/ClientConfiguration.cc +++ b/sdk/src/client/ClientConfiguration.cc @@ -81,6 +81,7 @@ bool DefaultRetryStrategy::shouldRetry(const Error & error, long attemptedRetrie case (ERROR_CURL_BASE + 52): //CURLE_GOT_NOTHING case (ERROR_CURL_BASE + 55): //CURLE_SEND_ERROR case (ERROR_CURL_BASE + 56): //CURLE_RECV_ERROR + case (ERROR_CURL_BASE + 65): //CURLE_SEND_FAIL_REWIND return true; default: break; From 2f4bed107dee6b2ea2427e31c63036d26820fb06 Mon Sep 17 00:00:00 2001 From: "guangjun.hgj" Date: Wed, 14 Jul 2021 15:47:21 +0800 Subject: [PATCH 2/3] support to unencode slash in presigned url path. --- .../oss/model/GeneratePresignedUrlRequest.h | 2 + sdk/src/OssClientImpl.cc | 9 ++- sdk/src/model/GeneratePresignedUrlRequest.cc | 8 +- test/src/Object/ObjectSignedUrlTest.cc | 77 +++++++++++++++++++ test/src/Other/UtilsFunctionTest.cc | 4 + 5 files changed, 98 insertions(+), 2 deletions(-) diff --git a/sdk/include/alibabacloud/oss/model/GeneratePresignedUrlRequest.h b/sdk/include/alibabacloud/oss/model/GeneratePresignedUrlRequest.h index 0101c17..bfe4e3f 100644 --- a/sdk/include/alibabacloud/oss/model/GeneratePresignedUrlRequest.h +++ b/sdk/include/alibabacloud/oss/model/GeneratePresignedUrlRequest.h @@ -44,6 +44,7 @@ namespace OSS void addResponseHeaders(RequestResponseHeader header, const std::string& value); void addParameter(const std::string& key, const std::string& value); MetaData& UserMetaData(); + void setUnencodedSlash(bool value); private: friend class OssClientImpl; std::string bucket_; @@ -51,6 +52,7 @@ namespace OSS Http::Method method_; ObjectMetaData metaData_; ParameterCollection parameters_; + bool unencodedSlash_; }; } } diff --git a/sdk/src/OssClientImpl.cc b/sdk/src/OssClientImpl.cc index 5ba6242..00bff8f3 100644 --- a/sdk/src/OssClientImpl.cc +++ b/sdk/src/OssClientImpl.cc @@ -1313,9 +1313,16 @@ StringOutcome OssClientImpl::GeneratePresignedUrl(const GeneratePresignedUrlRequ parameters["OSSAccessKeyId"] = credentials.AccessKeyId(); parameters["Signature"] = signature; + //host std::stringstream ss; ss << CombineHostString(endpoint_, request.bucket_, configuration().isCname); - ss << CombinePathString(endpoint_, request.bucket_, request.key_); + //path + auto path = CombinePathString(endpoint_, request.bucket_, request.key_); + if (request.unencodedSlash_) { + StringReplace(path, "%2F", "/"); + } + ss << path; + //query ss << "?"; ss << CombineQueryString(parameters); diff --git a/sdk/src/model/GeneratePresignedUrlRequest.cc b/sdk/src/model/GeneratePresignedUrlRequest.cc index dfe791d..2e01730 100644 --- a/sdk/src/model/GeneratePresignedUrlRequest.cc +++ b/sdk/src/model/GeneratePresignedUrlRequest.cc @@ -30,7 +30,8 @@ GeneratePresignedUrlRequest::GeneratePresignedUrlRequest(const std::string &buck GeneratePresignedUrlRequest::GeneratePresignedUrlRequest(const std::string &bucket, const std::string &key, Http::Method method): bucket_(bucket), key_(key), - method_(method) + method_(method), + unencodedSlash_(false) { //defalt 15 min std::time_t t = std::time(nullptr) + 15*60; @@ -102,3 +103,8 @@ MetaData &GeneratePresignedUrlRequest::UserMetaData() { return metaData_.UserMetaData(); } + +void GeneratePresignedUrlRequest::setUnencodedSlash(bool value) +{ + unencodedSlash_ = value; +} diff --git a/test/src/Object/ObjectSignedUrlTest.cc b/test/src/Object/ObjectSignedUrlTest.cc index f276d20..b4a17d5 100644 --- a/test/src/Object/ObjectSignedUrlTest.cc +++ b/test/src/Object/ObjectSignedUrlTest.cc @@ -590,6 +590,83 @@ TEST_F(ObjectSignedUrlTest, PutObjectByUrlRequestFunctionTest) EXPECT_TRUE(paramters.empty()); } +TEST_F(ObjectSignedUrlTest, UnencodedSlashTest) +{ + std::string key = TestUtils::GetObjectKey("UnencodedSlashTest/123/456%2F/123"); + std::shared_ptr content = TestUtils::GetRandomStream(2048); + + std::string md5 = ComputeContentMD5(*content.get()); + + GeneratePresignedUrlRequest request(BucketName, key, Http::Put); + request.setExpires(GetExpiresDelayS(120)); + request.setContentMd5(md5); + request.setContentType("text/rtf"); + request.UserMetaData()["Author"] = "oss"; + request.UserMetaData()["Test"] = "test"; + request.addParameter("x-param-null", ""); + request.addParameter("x-param-space0", " "); + request.addParameter("x-param-value", "value"); + request.addParameter("x-param-space1", " "); + + auto urlOutcome = Client->GeneratePresignedUrl(request); + EXPECT_EQ(urlOutcome.isSuccess(), true); + EXPECT_TRUE(urlOutcome.result().find("UnencodedSlashTest%2F123%2F456%252F%2F123") != std::string::npos); + + ObjectMetaData meta; + meta.setContentMd5(md5); + meta.setContentType("text/rtf"); + meta.UserMetaData()["Author"] = "oss"; + meta.UserMetaData()["Test"] = "test"; + + auto pOutcome = Client->PutObjectByUrl(urlOutcome.result(), content, meta); + EXPECT_EQ(pOutcome.isSuccess(), true); + EXPECT_EQ(TestUtils::ObjectExists(*Client, BucketName, key), true); + + auto metaOutcome = Client->HeadObject(BucketName, key); + EXPECT_EQ(metaOutcome.isSuccess(), true); + EXPECT_STREQ(metaOutcome.result().ContentType().c_str(), "text/rtf"); + EXPECT_STREQ(metaOutcome.result().UserMetaData().at("Author").c_str(), "oss"); + EXPECT_STREQ(metaOutcome.result().UserMetaData().at("author").c_str(), "oss"); + EXPECT_STREQ(metaOutcome.result().UserMetaData().at("Test").c_str(), "test"); + EXPECT_STREQ(metaOutcome.result().UserMetaData().at("tesT").c_str(), "test"); + + // + request = GeneratePresignedUrlRequest(BucketName, key, Http::Put); + request.setExpires(GetExpiresDelayS(120)); + request.setContentMd5(md5); + request.setContentType("text/rtf"); + request.UserMetaData()["Author"] = "oss1"; + request.UserMetaData()["Test"] = "test1"; + request.addParameter("x-param-null", ""); + request.addParameter("x-param-space0", " "); + request.addParameter("x-param-value", "value"); + request.addParameter("x-param-space1", " "); + request.setUnencodedSlash(true); + + urlOutcome = Client->GeneratePresignedUrl(request); + EXPECT_EQ(urlOutcome.isSuccess(), true); + EXPECT_TRUE(urlOutcome.result().find("UnencodedSlashTest/123/456%252F/123") != std::string::npos); + + meta = ObjectMetaData(); + meta.setContentMd5(md5); + meta.setContentType("text/rtf"); + meta.UserMetaData()["Author"] = "oss1"; + meta.UserMetaData()["Test"] = "test1"; + + pOutcome = Client->PutObjectByUrl(urlOutcome.result(), content, meta); + EXPECT_EQ(pOutcome.isSuccess(), true); + EXPECT_EQ(TestUtils::ObjectExists(*Client, BucketName, key), true); + + metaOutcome = Client->HeadObject(BucketName, key); + EXPECT_EQ(metaOutcome.isSuccess(), true); + EXPECT_STREQ(metaOutcome.result().ContentType().c_str(), "text/rtf"); + EXPECT_STREQ(metaOutcome.result().UserMetaData().at("Author").c_str(), "oss1"); + EXPECT_STREQ(metaOutcome.result().UserMetaData().at("author").c_str(), "oss1"); + EXPECT_STREQ(metaOutcome.result().UserMetaData().at("Test").c_str(), "test1"); + EXPECT_STREQ(metaOutcome.result().UserMetaData().at("tesT").c_str(), "test1"); +} + + } } \ No newline at end of file diff --git a/test/src/Other/UtilsFunctionTest.cc b/test/src/Other/UtilsFunctionTest.cc index 924eed6..9a97803 100644 --- a/test/src/Other/UtilsFunctionTest.cc +++ b/test/src/Other/UtilsFunctionTest.cc @@ -606,6 +606,10 @@ TEST_F(UtilsFunctionTest, StringReplaceTest) StringReplace(test, "abcd", "A"); EXPECT_EQ(test, "1234AABCD1234"); + + test = "12212"; + StringReplace(test, "12", "21"); + EXPECT_EQ(test, "21221"); } TEST_F(UtilsFunctionTest, UploadAndDownloadObject) From 02989aa5bf43a4a73d9a106ee5fb5342a9df2d21 Mon Sep 17 00:00:00 2001 From: "guangjun.hgj" Date: Wed, 14 Jul 2021 20:56:39 +0800 Subject: [PATCH 3/3] fix progress bug in resumable upload/download. --- sdk/src/resumable/ResumableDownloader.cc | 18 +++++++++++++++--- sdk/src/resumable/ResumableUploader.cc | 17 ++++++++++++++--- .../src/MultipartUpload/ResumableObjectTest.cc | 4 +++- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/sdk/src/resumable/ResumableDownloader.cc b/sdk/src/resumable/ResumableDownloader.cc index 8b6606d..e326e7f 100644 --- a/sdk/src/resumable/ResumableDownloader.cc +++ b/sdk/src/resumable/ResumableDownloader.cc @@ -29,6 +29,11 @@ using namespace AlibabaCloud::OSS; +struct DownloaderTransferState { + int64_t transfered; + void *userData; +}; + GetObjectOutcome ResumableDownloader::Download() { OssError err; @@ -79,9 +84,12 @@ GetObjectOutcome ResumableDownloader::Download() getObjectReq.setRange(start, end); getObjectReq.setFlags(getObjectReq.Flags() | REQUEST_FLAG_CHECK_CRC64 | REQUEST_FLAG_SAVE_CLIENT_CRC64); + DownloaderTransferState transferState; auto process = request_.TransferProgress(); if (process.Handler) { - TransferProgress uploadPartProcess = { DownloadPartProcessCallback, (void *)this }; + transferState.transfered = 0; + transferState.userData = (void *)this; + TransferProgress uploadPartProcess = { DownloadPartProcessCallback, (void *)&transferState }; getObjectReq.setTransferProgress(uploadPartProcess); } if (request_.RequestPayer() == RequestPayer::Requester) { @@ -468,10 +476,14 @@ void ResumableDownloader::initRecord() void ResumableDownloader::DownloadPartProcessCallback(size_t increment, int64_t transfered, int64_t total, void *userData) { - UNUSED_PARAM(transfered); UNUSED_PARAM(total); + auto transferState = (DownloaderTransferState *)userData; + auto downloader = (ResumableDownloader*)transferState->userData; + auto inc = transfered - transferState->transfered; + transferState->transfered = std::max(transfered, transferState->transfered); + inc = std::max(inc, static_cast(0)); + increment = static_cast(inc); - auto downloader = (ResumableDownloader*)userData; std::lock_guard lck(downloader->lock_); downloader->consumedSize_ += increment; diff --git a/sdk/src/resumable/ResumableUploader.cc b/sdk/src/resumable/ResumableUploader.cc index ce807c4..e272523 100644 --- a/sdk/src/resumable/ResumableUploader.cc +++ b/sdk/src/resumable/ResumableUploader.cc @@ -35,6 +35,10 @@ using namespace AlibabaCloud::OSS; +struct UploaderTransferState { + int64_t transfered; + void *userData; +}; ResumableUploader::ResumableUploader(const UploadObjectRequest& request, const OssClientImpl *client) : ResumableBaseWorker(request.ObjectSize(), request.PartSize()), @@ -101,9 +105,12 @@ PutObjectOutcome ResumableUploader::Upload() UploadPartRequest uploadPartRequest(request_.Bucket(), request_.Key(), part.PartNumber(), uploadID_, content); uploadPartRequest.setContentLength(length); + UploaderTransferState transferState; auto process = request_.TransferProgress(); if (process.Handler) { - TransferProgress uploadPartProcess = { UploadPartProcessCallback, (void *)this }; + transferState.transfered = 0; + transferState.userData = (void *)this; + TransferProgress uploadPartProcess = { UploadPartProcessCallback, (void *)&transferState }; uploadPartRequest.setTransferProgress(uploadPartProcess); } if (request_.RequestPayer() == RequestPayer::Requester) { @@ -409,10 +416,14 @@ void ResumableUploader::dumpRecordInfo(AlibabaCloud::OSS::Json::Value& root) void ResumableUploader::UploadPartProcessCallback(size_t increment, int64_t transfered, int64_t total, void *userData) { - UNUSED_PARAM(transfered); UNUSED_PARAM(total); + auto transferState = (UploaderTransferState *)userData; + auto uploader = (ResumableUploader*)transferState->userData; + auto inc = transfered - transferState->transfered; + transferState->transfered = std::max(transfered, transferState->transfered); + inc = std::max(inc, static_cast(0)); + increment = static_cast(inc); - auto uploader = (ResumableUploader*)userData; std::lock_guard lck(uploader->lock_); uploader->consumedSize_ += increment; diff --git a/test/src/MultipartUpload/ResumableObjectTest.cc b/test/src/MultipartUpload/ResumableObjectTest.cc index 7f42139..7091376 100644 --- a/test/src/MultipartUpload/ResumableObjectTest.cc +++ b/test/src/MultipartUpload/ResumableObjectTest.cc @@ -664,6 +664,7 @@ class ResumableObjectTest : public::testing::Test { EXPECT_EQ(CreateDirectory(checkpointDir), true); EXPECT_EQ(IsDirectoryExist(checkpointDir), true); + std::cout << "this ptr:" << this << std::endl; TransferProgress progressCallback = { ProgressCallback, this }; UploadObjectRequest request(BucketName, key, tmpFile, checkpointDir, 102400, 1); request.setTransferProgress(progressCallback); @@ -1325,6 +1326,7 @@ class ResumableObjectTest : public::testing::Test { EXPECT_EQ(putObjectOutcome.isSuccess(), true); EXPECT_EQ(Client->DoesObjectExist(BucketName, sourceKey), true); + std::cout << "this ptr:" << this << std::endl; TransferProgress progressCallback = { ProgressCallback, this }; DownloadObjectRequest request(BucketName, sourceKey, targetKey); request.setTransferProgress(progressCallback); @@ -2354,7 +2356,7 @@ class ResumableObjectTest : public::testing::Test { std::string checkpointDir = TestUtils::GetTargetFileName("checkpoint"); EXPECT_EQ(CreateDirectory(checkpointDir), true); EXPECT_EQ(IsDirectoryExist(checkpointDir), true); - + std::cout << "this ptr:" << this << std::endl; TransferProgress progressCallback = { ProgressCallback, this }; UploadObjectRequest request(BucketName, key, tmpFile, checkpointDir, 102400, 1); request.setTransferProgress(progressCallback);