soto-project / soto

Swift SDK for AWS that works on Linux, macOS and iOS
https://soto.codes
Apache License 2.0
878 stars 83 forks source link

Fix a bug where empty files can't be uploaded with multipart. #649

Closed bridger closed 1 year ago

bridger commented 1 year ago

This fixes a bug when upload a zero-length file via multipart upload.

Previously, this would result in the error: MultipartUploadError(error: MalformedXML: The XML you provided was not well-formed or did not validate against our published schema, completedParts: [])

The root of this bug was that an empty file would have 0 parts. This fix makes sure there is at least 1 part uploaded, even if that part is 0 bytes in length.

bridger commented 1 year ago

My project is using 5.x, which is why I opened the PR against that branch. I believe this fix should go in all branches. What's the best way to do that?

Thank you for making Soto! I've been using it for years in my project!

adam-fowler commented 1 year ago

That's an interesting edge case. I'll have a look at the change later today. It'd be good if you could add a test for this as well. If you could add one to S3ExtensionTests.swift that'd be great.

Regarding pushing to other branches, it'll only need a PR to main.

adam-fowler commented 1 year ago

Forgot to say tests by default use localstack to mock AWS services. If you have docker running you can run localstack using ./scripts/localstack start. To stop it replace start with stop. You can get tests to use actual AWS services by setting environment variable AWS_DISABLE_LOCALSTACK to true.

bridger commented 1 year ago

Thanks for taking a look, @adam-fowler! I've added a test, which was surprisingly easy to do with your excellent testing framework. Hopefully, the PR is good to go now.

adam-fowler commented 1 year ago

Hey @bridger this is now in v5.13.2. It would be great if you could add a PR for the main branch as well so we can bring this fix across to v6