From d21a5b2f0ca9abd4813d767098023a4b4d026fed Mon Sep 17 00:00:00 2001 From: Michael Kolupaev Date: Sat, 1 Jun 2024 06:56:42 +0000 Subject: [PATCH 1/2] Check for missing Upload ID in CreateMultipartUpload reply --- src/IO/S3/copyS3File.cpp | 17 +++++++++++------ src/IO/WriteBufferFromS3.cpp | 8 +++++++- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/IO/S3/copyS3File.cpp b/src/IO/S3/copyS3File.cpp index 24e14985758..ffec42aa855 100644 --- a/src/IO/S3/copyS3File.cpp +++ b/src/IO/S3/copyS3File.cpp @@ -149,16 +149,21 @@ namespace dest_bucket, dest_key, /* local_path_ */ {}, /* data_size */ 0, outcome.IsSuccess() ? nullptr : &outcome.GetError()); - if (outcome.IsSuccess()) - { - multipart_upload_id = outcome.GetResult().GetUploadId(); - LOG_TRACE(log, "Multipart upload has created. Bucket: {}, Key: {}, Upload id: {}", dest_bucket, dest_key, multipart_upload_id); - } - else + if (!outcome.IsSuccess()) { ProfileEvents::increment(ProfileEvents::WriteBufferFromS3RequestsErrors, 1); throw S3Exception(outcome.GetError().GetMessage(), outcome.GetError().GetErrorType()); } + else if (outcome.GetResult().GetUploadId().empty()) + { + ProfileEvents::increment(ProfileEvents::WriteBufferFromS3RequestsErrors, 1); + throw Exception(ErrorCodes::S3_ERROR, "Invalid CreateMultipartUpload result: missing UploadId."); + } + else + { + multipart_upload_id = outcome.GetResult().GetUploadId(); + LOG_TRACE(log, "Multipart upload was created. Bucket: {}, Key: {}, Upload id: {}", dest_bucket, dest_key, multipart_upload_id); + } } void completeMultipartUpload() diff --git a/src/IO/WriteBufferFromS3.cpp b/src/IO/WriteBufferFromS3.cpp index ff18a77f09f..b796c029051 100644 --- a/src/IO/WriteBufferFromS3.cpp +++ b/src/IO/WriteBufferFromS3.cpp @@ -413,7 +413,13 @@ void WriteBufferFromS3::createMultipartUpload() multipart_upload_id = outcome.GetResult().GetUploadId(); - LOG_TRACE(limitedLog, "Multipart upload has created. {}", getShortLogDetails()); + if (multipart_upload_id.empty()) + { + ProfileEvents::increment(ProfileEvents::WriteBufferFromS3RequestsErrors, 1); + throw Exception(ErrorCodes::S3_ERROR, "Invalid CreateMultipartUpload result: missing UploadId."); + } + + LOG_TRACE(limitedLog, "Multipart upload was created. {}", getShortLogDetails()); } void WriteBufferFromS3::abortMultipartUpload() From 1cb3961f9d74a74f1c7afcce8ec39b02c22c3697 Mon Sep 17 00:00:00 2001 From: Michael Kolupaev Date: Sat, 1 Jun 2024 18:12:30 +0000 Subject: [PATCH 2/2] Call GetResult() once --- src/IO/S3/copyS3File.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/IO/S3/copyS3File.cpp b/src/IO/S3/copyS3File.cpp index ffec42aa855..d3968d883e8 100644 --- a/src/IO/S3/copyS3File.cpp +++ b/src/IO/S3/copyS3File.cpp @@ -154,16 +154,13 @@ namespace ProfileEvents::increment(ProfileEvents::WriteBufferFromS3RequestsErrors, 1); throw S3Exception(outcome.GetError().GetMessage(), outcome.GetError().GetErrorType()); } - else if (outcome.GetResult().GetUploadId().empty()) + multipart_upload_id = outcome.GetResult().GetUploadId(); + if (multipart_upload_id.empty()) { ProfileEvents::increment(ProfileEvents::WriteBufferFromS3RequestsErrors, 1); throw Exception(ErrorCodes::S3_ERROR, "Invalid CreateMultipartUpload result: missing UploadId."); } - else - { - multipart_upload_id = outcome.GetResult().GetUploadId(); - LOG_TRACE(log, "Multipart upload was created. Bucket: {}, Key: {}, Upload id: {}", dest_bucket, dest_key, multipart_upload_id); - } + LOG_TRACE(log, "Multipart upload was created. Bucket: {}, Key: {}, Upload id: {}", dest_bucket, dest_key, multipart_upload_id); } void completeMultipartUpload()