diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 49d6aacd8..1f8a7a1e9 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -15,6 +15,21 @@ <release date="XXXX-XX-XX" version="2.35dev" title="UNDER DEVELOPMENT"> <release-core-list> <release-bug-list> + <release-item> + <github-issue id="1116"/> + <github-issue id="1433"/> + <github-pull-request id="1438"/> + + <release-item-contributor-list> + <release-item-ideator id="marco.montagna"/> + <release-item-ideator id="lev.kokotov"/> + <release-item-contributor id="david.steele"/> + <release-item-reviewer id="cynthia.shang"/> + </release-item-contributor-list> + + <p>Detect errors in <proper>S3</proper> multi-part upload finalize.</p> + </release-item> + <release-item> <github-issue id="1447"/> <github-pull-request id="1452"/> @@ -10172,6 +10187,11 @@ <contributor-id type="github">marco44</contributor-id> </contributor> + <contributor id="marco.montagna"> + <contributor-name-display>Marco Montagna</contributor-name-display> + <contributor-id type="github">mmontagna</contributor-id> + </contributor> + <contributor id="markus.nullmeier"> <contributor-name-display>Markus Nullmeier</contributor-name-display> <contributor-id type="github">mnullmei</contributor-id> diff --git a/src/storage/s3/write.c b/src/storage/s3/write.c index 4662e9051..ea79e8c46 100644 --- a/src/storage/s3/write.c +++ b/src/storage/s3/write.c @@ -235,10 +235,18 @@ storageWriteS3Close(THIS_VOID) } // Finalize the multi-part upload - storageS3RequestP( + HttpRequest *request = storageS3RequestAsyncP( this->storage, HTTP_VERB_POST_STR, this->interface.name, .query = httpQueryAdd(httpQueryNewP(), S3_QUERY_UPLOAD_ID_STR, this->uploadId), .content = xmlDocumentBuf(partList)); + HttpResponse *response = storageS3ResponseP(request); + + // Error if there is no etag in the result. This indicates that the request did not succeed despite the success code. + if (xmlNodeChild( + xmlDocumentRoot(xmlDocumentNewBuf(httpResponseContent(response))), S3_XML_TAG_ETAG_STR, false) == NULL) + { + httpRequestError(request, response); + } } // Else upload all the data in a single put else diff --git a/test/src/module/storage/s3Test.c b/test/src/module/storage/s3Test.c index b83232ca1..b7281ea8a 100644 --- a/test/src/module/storage/s3Test.c +++ b/test/src/module/storage/s3Test.c @@ -686,11 +686,69 @@ testRun(void) "<Part><PartNumber>1</PartNumber><ETag>WxRt1</ETag></Part>" "<Part><PartNumber>2</PartNumber><ETag>WxRt2</ETag></Part>" "</CompleteMultipartUpload>\n"); - testResponseP(service); + testResponseP( + service, + .content = + "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" + "<CompleteMultipartUploadResult><ETag>XXX</ETag></CompleteMultipartUploadResult>"); TEST_ASSIGN(write, storageNewWriteP(s3, STRDEF("file.txt")), "new write"); TEST_RESULT_VOID(storagePutP(write, BUFSTRDEF("12345678901234567890123456789012")), "write"); + // ----------------------------------------------------------------------------------------------------------------- + TEST_TITLE("error in success response of multipart upload"); + + testRequestP(service, s3, HTTP_VERB_POST, "/file.txt?uploads="); + testResponseP( + service, + .content = + "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" + "<InitiateMultipartUploadResult xmlns=\"http://s3.amazonaws.com/doc/2006-03-01/\">" + "<Bucket>bucket</Bucket>" + "<Key>file.txt</Key>" + "<UploadId>WxRt</UploadId>" + "</InitiateMultipartUploadResult>"); + + testRequestP(service, s3, HTTP_VERB_PUT, "/file.txt?partNumber=1&uploadId=WxRt", .content = "1234567890123456"); + testResponseP(service, .header = "etag:WxRt1"); + + testRequestP(service, s3, HTTP_VERB_PUT, "/file.txt?partNumber=2&uploadId=WxRt", .content = "7890123456789012"); + testResponseP(service, .header = "eTag:WxRt2"); + + testRequestP( + service, s3, HTTP_VERB_POST, "/file.txt?uploadId=WxRt", + .content = + "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" + "<CompleteMultipartUpload>" + "<Part><PartNumber>1</PartNumber><ETag>WxRt1</ETag></Part>" + "<Part><PartNumber>2</PartNumber><ETag>WxRt2</ETag></Part>" + "</CompleteMultipartUpload>\n"); + testResponseP( + service, + .content = + "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" + "<Error><Code>AccessDenied</Code><Message>Access Denied</Message></Error>"); + + TEST_ASSIGN(write, storageNewWriteP(s3, STRDEF("file.txt")), "new write"); + TEST_ERROR( + storagePutP(write, BUFSTRDEF("12345678901234567890123456789012")), ProtocolError, + "HTTP request failed with 200 (OK):\n" + "*** Path/Query ***:\n" + "/file.txt?uploadId=WxRt\n" + "*** Request Headers ***:\n" + "authorization: <redacted>\n" + "content-length: 205\n" + "content-md5: 37smUM6Ah2/EjZbp420dPw==\n" + "host: bucket.s3.amazonaws.com\n" + "x-amz-content-sha256: 0838a79dfbddc2128d28fb4fa8d605e0a8e6d1355094000f39b6eb3feff4641f\n" + "x-amz-date: <redacted>\n" + "x-amz-security-token: <redacted>\n" + "*** Response Headers ***:\n" + "content-length: 110\n" + "*** Response Content ***:\n" + "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" + "<Error><Code>AccessDenied</Code><Message>Access Denied</Message></Error>"); + // ----------------------------------------------------------------------------------------------------------------- TEST_TITLE("write file in chunks with something left over on close"); @@ -719,7 +777,11 @@ testRun(void) "<Part><PartNumber>1</PartNumber><ETag>RR551</ETag></Part>" "<Part><PartNumber>2</PartNumber><ETag>RR552</ETag></Part>" "</CompleteMultipartUpload>\n"); - testResponseP(service); + testResponseP( + service, + .content = + "<?xml version=\"1.0\" encoding=\"UTF-8\"?>" + "<CompleteMultipartUploadResult><ETag>XXX</ETag></CompleteMultipartUploadResult>"); TEST_ASSIGN(write, storageNewWriteP(s3, STRDEF("file.txt")), "new write"); TEST_RESULT_VOID(storagePutP(write, BUFSTRDEF("12345678901234567890")), "write");