Bugfix/0 byte download from s3 using http

[MaxEliaserAWS]

Issue #, if available:

This PR fixes an issue similar to #738, but which only appears when using HTTP endpoints. When downloading an empty (0-byte) file from S3 using Transfer API and using HTTP (not HTTPS) endpoints, the download fails with CurlCode 56 (connection reset by peer.) Issue has been present since 1.7.x release series.

The 0-byte file size causes TransferManager::DoSinglePartDownload to call request.SetRange(FormatRangeSpecifier, 0, 0+0-1)), which results in a malformed range: header due to integer underflow. Protection had been added for this underflow issue to some codepaths, but not to this one.

Interestingly, in HTTPS mode, the S3 REST API seems to accept this malformed range header without complaint. I have no explanation for the behavior difference between HTTP and HTTPS endpoints, but maybe it has something to do with the way sigv4 signing is forced on with HTTP endpoints.

Description of changes:

• Adds integer underflow protection
• To avoid issues like this in the future, I have parameterized the unit tests for TransferManager (TransferTests) such that they all run in both HTTP and HTTPS mode.

The existing EmptyFileTest is sufficient to regression-test this issue when it is run against the HTTP endpoint. I have verified that the unit test as currently written in this PR will repro the issue reliably and that my bugfix causes the tests to pass.

Check all that applies:

• Did a review by yourself.
• Added proper tests to cover this PR. (If tests are not applicable, explain.)
• Checked if this PR is a breaking (APIs have been changed) change.
• Checked if this PR will not introduce cross-platform inconsistent behavior.
• Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

• Linux
• Windows
• Android
• MacOS
• IOS
• Other Platforms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[MaxEliaserAWS]

For whatever reason, the EmptyFileTest only reproduces the issue if I move it to the beginning of the test suite. If I use --gtest_filter to run only EmptyFileTest it reproduces regardless of what order I declare the tests in. Running the HTTPS tests before the HTTP tests doesn't seem to hurt anything, but in order to repro the problem I need EmptyFileTest to be the first HTTP test.

I have no explanation, but I conjecture that it might have something to do with the CURL connection pool and connections being reused.

I think looking over the server-side implementations that this code is talking to will also help to illuminate what accounts for the behavior difference between HTTP and HTTPS endpoints and the behavior difference caused by different execution orderings of these unit tests.

[MaxEliaserAWS]

Thanks for reviewing Sergey. I'm not able to hit the merge button myself, nor am I familiar with your team's process w.r.t minimum number of approvals, etc.

[SergeyRyabinin]

Hi @MaxEliaserAWS,

Please sorry for a delayed response and delayed merge.
We had some issues in our CI build pipeline that prevented the validation of the PR: the PR was based on the revision before bc020dd / bc020dd that prevented the CI build from passing on Amazon Linux 2023. Once rebased it built fine.

Thank you a lot for your contribution.

Best regards,
Sergey

[MaxEliaserAWS]

No worries, glad I could help!