S3 Breaking Change - 3.7.412.0+ doesn't respect seekable streams and incorrectly sets Content-Length Header

[niemyjski]

Describe the bug

I've tracked it down to this was working in 3.7.411.7 but broken in any future release. Due to how you tag releases and different client versions it's pretty hard to track down...

System.Net.Http.HttpRequestException
Sent 298 request content bytes, but Content-Length promised 305.
   at System.Net.Http.HttpConnection.SendRequestContentAsync(HttpRequestMessage request, HttpContentWriteStream stream, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnection.SendRequestContentWithExpect100ContinueAsync(HttpRequestMessage request, Task`1 allowExpect100ToContinueTask, HttpContentWriteStream stream, Timer expect100Timer, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnection.SendAsync(HttpRequestMessage request, Boolean async, CancellationToken cancellationToken)
   at System.Net.Http.HttpConnectionPool.SendWithVersionDetectionAndRetryAsync(HttpRequestMessage request, Boolean async, Boolean doRequestAuth, CancellationToken cancellationToken)
   at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
   at Amazon.Runtime.HttpWebRequestMessage.GetResponseAsync(CancellationToken cancellationToken)
   at Amazon.Runtime.Internal.HttpHandler`1.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.RedirectHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.Unmarshaller.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.S3.Internal.AmazonS3ResponseHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.ErrorHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.ErrorHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.CallbackHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.Signer.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.S3.Internal.S3Express.S3ExpressPreSigner.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.EndpointDiscoveryHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.EndpointDiscoveryHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.CredentialsRetriever.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.RetryHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.RetryHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.CallbackHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.CallbackHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.S3.Internal.AmazonS3ExceptionHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.ErrorCallbackHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Amazon.Runtime.Internal.MetricsHandler.InvokeAsync[T](IExecutionContext executionContext)
   at Foundatio.Storage.S3FileStorage.SaveFileAsync(String path, Stream stream, CancellationToken cancellationToken) in /Users/blakeniemyjski/code/Foundatio.AWS/src/Foundatio.AWS/Storage/S3FileStorage.cs:line 220

Regression Issue

• Select this option if this issue appears to be a regression.

Expected Behavior

I should be able to post a partial stream.

Current Behavior

Crashes with an error

System.Net.Http.HttpRequestException
Sent 298 request content bytes, but Content-Length promised 305.

Reproduction Steps

string path = "blake.txt";
            using (var memoryStream = new MemoryStream())
            {
                long offset;
                await using (var writer = new StreamWriter(memoryStream, Encoding.UTF8, 1024, true))
                {
                    writer.AutoFlush = true;
                    await writer.WriteAsync("Eric");
                    offset = memoryStream.Position;
                    await writer.WriteAsync("Blake");
                    await writer.FlushAsync();
                }

                memoryStream.Seek(offset, SeekOrigin.Begin);
                await SaveFileAsync(path, memoryStream);
            }

    public async Task<bool> SaveFileAsync(string path, Stream stream, CancellationToken cancellationToken = default(CancellationToken))
    {
        if (String.IsNullOrEmpty(path))
            throw new ArgumentNullException(nameof(path));
        if (stream == null)
            throw new ArgumentNullException(nameof(stream));

        var req = new PutObjectRequest
        {
            CannedACL = _cannedAcl,
            BucketName = _bucket,
            Key = NormalizePath(path),
            AutoResetStreamPosition = false,
            AutoCloseStream = !stream.CanSeek,
            InputStream = stream.CanSeek ? stream : AmazonS3Util.MakeStreamSeekable(stream),
            UseChunkEncoding = _useChunkEncoding
        };

        _logger.LogTrace("Saving {Path}", req.Key);
        var response = await _client.PutObjectAsync(req, cancellationToken).AnyContext();
        return response.HttpStatusCode.IsSuccessful();
    }

Possible Solution

Set the content length header to stream length - stream position instead of stream length

Additional Information/Context

No response

AWS .NET SDK and/or Package version used

AWSSDK.S3 3.7.412.0 +

Targeted .NET Platform

.NET Core 8

Operating System and version

Mac Os

[dscpinheiro]

There was a change in version 3.7.412.0, described in one of the announcements pinned to this repo: #3610

Are you using S3 or a 3rd-party implementation?

[niemyjski]

I'm using S3, tests run against localstack (latest)

[niemyjski]

That announcement I didn't know about it, but even reading it doesn't know how it would apply to anything I do (pointing out the messaging may need work).

You don't seek the the start of the stream and this scenario should just work. It seems like a bug with how you are setting the header. The content I'm sending is expected, I don't want to have to allocate and buffer a memory streams just so the library sets the correct header length.

[niemyjski]

My full test is asserting I can save partial content based on stream position. We have in various storage providers and they all allows this:

    public virtual async Task WillRespectStreamOffsetAsync()
    {
        var storage = GetStorage();
        if (storage == null)
            return;

        await ResetAsync(storage);

        using (storage)
        {
            string path = "blake.txt";
            using (var memoryStream = new MemoryStream())
            {
                long offset;
                await using (var writer = new StreamWriter(memoryStream, Encoding.UTF8, 1024, true))
                {
                    writer.AutoFlush = true;
                    await writer.WriteAsync("Eric");
                    offset = memoryStream.Position;
                    await writer.WriteAsync("Blake");
                    await writer.FlushAsync();
                }

                memoryStream.Seek(offset, SeekOrigin.Begin);
                await storage.SaveFileAsync(path, memoryStream);
            }

            Assert.Equal("Blake", await storage.GetFileContentsAsync(path));
        }
    }

[dscpinheiro]

We were able to reproduce it when using S3 too and are investigating it.

We do have a test that seeks a stream before uploading an object, trying to verify what we missed there:

aws-sdk-net/sdk/test/Services/S3/IntegrationTests/PutObjectTests.cs

Line 1309 in 29fda0c

public void TestResetStreamPosition()

[niemyjski]

Could it be something with PutObject vs PutObjectAsync and or chunk encoding? I don't think that would matter?

Also, I noticed you could speed up your tests by moving Thread.Sleep(1000 * retries); to the last line of for (seems kind of odd to do it before first retry, unless there is some kind of consistency issue but in that way a mutex might work better (we use https://github.com/StephenCleary/AsyncEx/blob/master/src/Nito.AsyncEx.Coordination/AsyncCountdownEvent.cs for this use case)

[dscpinheiro]

No, I don't believe so. I tried your sample code without disabling chunk encoding and it still failed.

One odd thing I found out so far is that I don't see the error when using the .NET Framework (which is the target that integration test I linked uses). But it does happen consistently in .NET 8

[ashishdhingra]

@niemyjski / @dscpinheiro I'm getting a different error though with the latest version of AWSSDK.S3: Amazon.S3.AmazonS3Exception: 'The request signature we calculated does not match the signature you provided. Check your key and signing method.'.

It's working in AWSSDK.S3 version 3.7.411.7 though as mentioned in issue description.

My project is targeting net6.0.

[dscpinheiro]

@niemyjski I put up a PR to address this issue (#3631); your suggestion was correct, we had an implementation specific for .NET Standard / .NET 8 that was not taking into account the stream position when setting the header.

It's pending approvals, but hopefully it'll be included in our next release. For the moment, you can keep using a version newer than 3.7.412 (I'll update this issue when the new version is available).

---

@ashishdhingra That seems like a different problem Never mind, it looks like .NET 6 (and .NET Core 3.1) return a different error message (which mentions the signature mismatch). The PR above will also fix this, I verified by looking at the netcoreapp3.1 output of our internal build.

[niemyjski]

Awesome job! Any idea why the entire test suite isn't running across all TFMs?