dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.26k stars 4.73k forks source link

HttpClient failed to handle Request Entity Too Large (413) response #69048

Closed saintogod closed 2 years ago

saintogod commented 2 years ago

Description

Setting the RequestSizeLimit on ApiController. When Httpclient sends a request over the size limitation, it will get a SocketException, not a 413 response.

Reproduction Steps

server code Program.cs

using Microsoft.AspNetCore.Http.Features;
using Microsoft.AspNetCore.Mvc;

var builder = WebApplication.CreateBuilder(args);

var app = builder.Build();

app.MapPost("/", [RequestSizeLimit(2 << 20)] /* attribute not working? */ async (HttpContext context, HttpRequest request) =>
{
    context.Features.Get<IHttpMaxRequestBodySizeFeature>()!.MaxRequestBodySize = (2 << 20);
    using var stream = new MemoryStream();
    await request.Body.CopyToAsync(stream);
    stream.Seek(0, SeekOrigin.Begin);

    return Results.Json(new { message = "Content length is: " + stream.Length });
});

app.Run();

client side Program.cs

using System.Net.Sockets;

using var httpClient = new HttpClient()
{
    BaseAddress = new Uri("http://localhost:5260/")
};
using var stream = new MemoryStream();
using (var writer = new StreamWriter(stream, leaveOpen: true))
{
    var str = new string(' ', 1024);
    for (var j = 0; j < 2580; j++)
        writer.Write(str);
}
stream.Seek(0, SeekOrigin.Begin);

var dataFile = Path.GetTempFileName();
using (var file = new FileStream(dataFile, FileMode.Create))
    await stream.CopyToAsync(file);

for (var i = 0; i < 10; i++)
{
    try
    {
        using var request = new HttpRequestMessage(HttpMethod.Post, "")
        {
            Content = new StreamContent(File.OpenRead(dataFile))

        };
        using var response = await httpClient.PostAsync("", new StreamContent(File.OpenRead(dataFile)));

        Console.Write(i + ", ");
        // Console.WriteLine(await response.Content.ReadAsStringAsync());
    }
    catch (Exception ex)
    {
        PrintError(i, ex);
    }
}
void PrintError(int i, Exception ex)
{
    Console.Write("\n" + i + "(EX):\t");
    Console.WriteLine($"{ex.GetType().FullName}: {ex.Message}");
    ex = ex.InnerException!;
    Console.WriteLine($"{ex.GetType().FullName}: {ex.Message}");

    var soEx = (ex.InnerException as SocketException)!;
    Console.WriteLine($"{soEx.GetType().FullName} ({soEx.SocketErrorCode}, {soEx.NativeErrorCode}): {soEx.Message}");
}

Expected behavior

HttpClient should return a valid 413 response.

Actual behavior

System.Net.Http.HttpRequestException: Error while copying content to a stream. System.IO.IOException: Unable to write data to the transport connection: An existing connection was forcibly closed by the remote host.. System.Net.Sockets.SocketException (ConnectionReset, 10054): An existing connection was forcibly closed by the remote host.

Regression?

No response

Known Workarounds

No response

Configuration

❯ dotnet --info .NET SDK (reflecting any global.json): Version: 6.0.202 Commit: f8a55617d2

Runtime Environment: OS Name: Windows OS Version: 10.0.19042 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\6.0.202\

Host (useful for support): Version: 6.0.4 Commit: be98e88c76

Other information

If this is the expected behavior, what's the correct way to handle 413 response in HttpClient.

ghost commented 2 years ago

Tagging subscribers to this area: @dotnet/ncl See info in area-owners.md if you want to be subscribed.

Issue Details
### Description Setting the RequestSizeLimit on ApiController. When Httpclient sends a request over the size limitation, it will get a SocketException, not a 413 response. ### Reproduction Steps server code Program.cs ``` using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Mvc; var builder = WebApplication.CreateBuilder(args); var app = builder.Build(); app.MapPost("/", [RequestSizeLimit(2 << 20)] /* attribute not working? */ async (HttpContext context, HttpRequest request) => { context.Features.Get()!.MaxRequestBodySize = (2 << 20); using var stream = new MemoryStream(); await request.Body.CopyToAsync(stream); stream.Seek(0, SeekOrigin.Begin); return Results.Json(new { message = "Content length is: " + stream.Length }); }); app.Run(); ``` client side Program.cs ``` using System.Net.Sockets; using var httpClient = new HttpClient() { BaseAddress = new Uri("http://localhost:5260/") }; using var stream = new MemoryStream(); using (var writer = new StreamWriter(stream, leaveOpen: true)) { var str = new string(' ', 1024); for (var j = 0; j < 2580; j++) writer.Write(str); } stream.Seek(0, SeekOrigin.Begin); var dataFile = Path.GetTempFileName(); using (var file = new FileStream(dataFile, FileMode.Create)) await stream.CopyToAsync(file); for (var i = 0; i < 10; i++) { try { using var request = new HttpRequestMessage(HttpMethod.Post, "") { Content = new StreamContent(File.OpenRead(dataFile)) }; using var response = await httpClient.PostAsync("", new StreamContent(File.OpenRead(dataFile))); Console.Write(i + ", "); // Console.WriteLine(await response.Content.ReadAsStringAsync()); } catch (Exception ex) { PrintError(i, ex); } } void PrintError(int i, Exception ex) { Console.Write("\n" + i + "(EX):\t"); Console.WriteLine($"{ex.GetType().FullName}: {ex.Message}"); ex = ex.InnerException!; Console.WriteLine($"{ex.GetType().FullName}: {ex.Message}"); var soEx = (ex.InnerException as SocketException)!; Console.WriteLine($"{soEx.GetType().FullName} ({soEx.SocketErrorCode}, {soEx.NativeErrorCode}): {soEx.Message}"); } ``` ### Expected behavior HttpClient should return a valid 413 response. ### Actual behavior System.Net.Http.HttpRequestException: Error while copying content to a stream. System.IO.IOException: Unable to write data to the transport connection: An existing connection was forcibly closed by the remote host.. System.Net.Sockets.SocketException (ConnectionReset, 10054): An existing connection was forcibly closed by the remote host. ### Regression? _No response_ ### Known Workarounds _No response_ ### Configuration ❯ dotnet --info .NET SDK (reflecting any global.json): Version: 6.0.202 Commit: f8a55617d2 Runtime Environment: OS Name: Windows OS Version: 10.0.19042 OS Platform: Windows RID: win10-x64 Base Path: C:\Program Files\dotnet\sdk\6.0.202\ Host (useful for support): Version: 6.0.4 Commit: be98e88c76 ### Other information If this is the expected behavior, what's the correct way to handle 413 response in HttpClient.
Author: saintogod
Assignees: -
Labels: `area-System.Net.Http`, `untriaged`
Milestone: -
wfurt commented 2 years ago

Do you have packet capture? Is it possible that the server is also closing socket? That could still bubble up as socket exception since the operation did not complete. I would recommend to use 100Continue to avoid this situation.

saintogod commented 2 years ago

Do you have packet capture? Is it possible that the server is also closing socket? That could still bubble up as socket exception since the operation did not complete. I would recommend to use 100Continue to avoid this situation.

curl always display the expected response, and the wireshark shows the server response 413 with message correctly.

davidfowl commented 2 years ago

That attribute doesn't work on minimal APIs, it's currently an MVC filter. Kestrel is probably closing the connection.

cc @Tratcher @halter73

saintogod commented 2 years ago

Do you have packet capture? Is it possible that the server is also closing socket? That could still bubble up as socket exception since the operation did not complete. I would recommend to use 100Continue to avoid this situation.

After add the Expect: 100-continue to the request, it can get the response if all the content has been loaded. e.g.

    try
    {
        stream.Seek(0, SeekOrigin.Begin);
        using var _stream = new MemoryStream();
        await stream.CopyToAsync(_stream);
        _stream.Seek(0, SeekOrigin.Begin);
        using var request = new HttpRequestMessage(HttpMethod.Post, "")
        {
            Content = new StreamContent(_stream)
        };
        request.Headers.ExpectContinue = true;
        using var response = await httpClient.SendAsync(request);
        Console.Write(i + ", ");
        // Console.WriteLine(await response.Content.ReadAsStringAsync());
    }
    catch (Exception ex)
    {
        PrintError(i, ex);
    }

But if loaded from file as posted, still got the exception.

karelz commented 2 years ago

Triage: @saintogod can you please provide the packet capture?

Tratcher commented 2 years ago

HttpClient finishes the upload before looking at the response. If the upload fails then it reports that instead of the status code. I agree it would be preferable to report a 4xx or 5xx response instead of the upload failure.

wfurt commented 2 years ago

Is there option for Kestrel to not close/reset the connection? We can perhaps look into handing out the error but it may be tricky and dependent on protocol version.

Tratcher commented 2 years ago

Is there option for Kestrel to not close/reset the connection? We can perhaps look into handing out the error but it may be tricky and dependent on protocol version.

Not for 413, the connection is intentionally aborted after the response to prevent DOS attacks.

saintogod commented 2 years ago

Triage: @saintogod can you please provide the packet capture?

attached related capture, dummy data are trimmed

No.     Time           Source                Destination           Protocol SourcePort DestPort Info
    160 4.891271       127.0.0.1             127.0.0.1             HTTP     3575       5260     POST / HTTP/1.1 

Frame 160: 20524 bytes on wire (164192 bits), 20524 bytes captured (164192 bits) on interface \Device\NPF_Loopback, id 0
Null/Loopback
Internet Protocol Version 4, Src: 127.0.0.1, Dst: 127.0.0.1
Transmission Control Protocol, Src Port: 3575, Dst Port: 5260, Seq: 2621507, Ack: 1, Len: 20480
[61 Reassembled TCP Segments (2641986 bytes): #79(4096), #81(65495), #82(61547), #84(65495), #85(65495), #86(82), #88(65495), #89(65495), #90(82), #92(65495), #93(65495), #94(82), #96(65495), #97(65495), #98(82), #100(65495), #101(65495), #]
Hypertext Transfer Protocol
    POST / HTTP/1.1\r\n
    Host: localhost:5260\r\n
    Content-Length: 2641920\r\n
    \r\n
    [Full request URI: http://localhost:5260/]
    [HTTP request 1/1]
    [Response in frame: 162]
    File Data: 2641920 bytes
    Data (2641920 bytes)

No.     Time           Source                Destination           Protocol SourcePort DestPort Info
    162 4.905218       127.0.0.1             127.0.0.1             HTTP     5260       3575     HTTP/1.1 413 Payload Too Large  (text/plain)

Frame 162: 1612 bytes on wire (12896 bits), 1612 bytes captured (12896 bits) on interface \Device\NPF_Loopback, id 0
Null/Loopback
Internet Protocol Version 4, Src: 127.0.0.1, Dst: 127.0.0.1
Transmission Control Protocol, Src Port: 5260, Dst Port: 3575, Seq: 1, Ack: 2641987, Len: 1568
Hypertext Transfer Protocol
    HTTP/1.1 413 Payload Too Large\r\n
    Content-Type: text/plain; charset=utf-8\r\n
    Date: Wed, 11 May 2022 03:14:43 GMT\r\n
    Server: Kestrel\r\n
    Transfer-Encoding: chunked\r\n
    \r\n
    [HTTP response 1/1]
    [Time since request: 0.013947000 seconds]
    [Request in frame: 160]
    [Request URI: http://localhost:5260/]
    HTTP chunked response
    File Data: 1399 bytes
Line-based text data: text/plain (16 lines)
saintogod commented 2 years ago

as @Tratcher said, the behavior is expected. probably not an issue of HttpClient.

Going to close this issue. Thank you all for your time.

wfurt commented 2 years ago

We may still choose to return the error response if that arrives soon enough. But I'm not sure if we can make it predictable.