Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[HTTP/3] Test failure SendAsync_WithZeroLengthHeaderName_Throws #56292

Closed
karelz opened this issue Jul 26, 2021 · 4 comments · Fixed by #56785
Closed

[HTTP/3] Test failure SendAsync_WithZeroLengthHeaderName_Throws #56292

karelz opened this issue Jul 26, 2021 · 4 comments · Fixed by #56785
Assignees
@wfurt
Labels
area-System.Net.Http disabled-test The test is disabled in source code against the issue test-run-core Test failures in .NET Core test runs
Milestone
6.0.0

Comments

@karelz
Copy link
Member

karelz commented Jul 26, 2021 •
edited
Loading

Failures 5/26-7/27 (incl. PRs):

Day Failures
7/25 2x Official run
7/26 3x Official run + 8x PR
7/27 (incomplete day) 1x Official run + 7x PR

Test type: System.Net.Http.Functional.Tests.SocketsHttpHandlerTest_HttpClientHandlerTest_Headers_Http3_MsQuic

Failure:

System.Net.Quic.QuicConnectionAbortedException : Connection aborted by peer (256).

   at System.Net.Quic.Implementations.MsQuic.MsQuicStream.ShutdownCompleted(CancellationToken cancellationToken) in /_/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs:line 576
   at System.Net.Test.Common.Http3LoopbackStream.SendResponseBodyAsync(Byte[] content, Boolean isFinal) in /_/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs:line 274
   at System.Net.Test.Common.Http3LoopbackStream.SendResponseAsync(HttpStatusCode statusCode, IList`1 headers, String content, Boolean isFinal) in /_/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs:line 235
   at System.Net.Test.Common.Http3LoopbackConnection.HandleRequestAsync(HttpStatusCode statusCode, IList`1 headers, String content) in /_/src/libraries/Common/tests/System/Net/Http/Http3LoopbackConnection.cs:line 195
   at System.Net.Test.Common.Http3LoopbackServer.HandleRequestAsync(HttpStatusCode statusCode, IList`1 headers, String content) in /_/src/libraries/Common/tests/System/Net/Http/Http3LoopbackServer.cs:line 76
   at System.Net.Http.Functional.Tests.HttpClientHandlerTest_Headers.<>c.<<SendAsync_WithZeroLengthHeaderName_Throws>b__16_1>d.MoveNext() in /_/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs:line 425
--- End of stack trace from previous location ---
   at System.Threading.Tasks.TaskTimeoutExtensions.WhenAllOrAnyFailed(Task[] tasks) in /_/src/libraries/Common/tests/System/Threading/Tasks/TaskTimeoutExtensions.cs:line 63
   at System.Threading.Tasks.TaskTimeoutExtensions.WhenAllOrAnyFailed(Task[] tasks) in /_/src/libraries/Common/tests/System/Threading/Tasks/TaskTimeoutExtensions.cs:line 82
   at System.Net.Test.Common.LoopbackServerFactory.<>c__DisplayClass5_0.<<CreateClientAndServerAsync>b__0>d.MoveNext() in /_/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs:line 39
--- End of stack trace from previous location ---
   at System.Net.Test.Common.Http3LoopbackServerFactory.CreateServerAsync(Func`3 funcAsync, Int32 millisecondsTimeout, GenericLoopbackOptions options) in /_/src/libraries/Common/tests/System/Net/Http/Http3LoopbackServer.cs:line 101
   at System.Net.Http.Functional.Tests.HttpClientHandlerTest_Headers.SendAsync_WithZeroLengthHeaderName_Throws() in /_/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs:line 413
--- End of stack trace from previous location ---
@karelz karelz added this to the 6.0.0 milestone Jul 26, 2021
@ghost
Copy link

ghost commented Jul 26, 2021

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

Issue Details

Failures 5/26-7/26 (incl. PRs):

Day Failures
7/25 2 Official run
7/26 (incomplete day) 1x PR

Test type: System.Net.Http.Functional.Tests.SocketsHttpHandlerTest_HttpClientHandlerTest_Headers_Http3_MsQuic

Failure:

System.Net.Quic.QuicConnectionAbortedException : Connection aborted by peer (256).

   at System.Net.Quic.Implementations.MsQuic.MsQuicStream.ShutdownCompleted(CancellationToken cancellationToken) in /_/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs:line 576
   at System.Net.Test.Common.Http3LoopbackStream.SendResponseBodyAsync(Byte[] content, Boolean isFinal) in /_/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs:line 274
   at System.Net.Test.Common.Http3LoopbackStream.SendResponseAsync(HttpStatusCode statusCode, IList`1 headers, String content, Boolean isFinal) in /_/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs:line 235
   at System.Net.Test.Common.Http3LoopbackConnection.HandleRequestAsync(HttpStatusCode statusCode, IList`1 headers, String content) in /_/src/libraries/Common/tests/System/Net/Http/Http3LoopbackConnection.cs:line 195
   at System.Net.Test.Common.Http3LoopbackServer.HandleRequestAsync(HttpStatusCode statusCode, IList`1 headers, String content) in /_/src/libraries/Common/tests/System/Net/Http/Http3LoopbackServer.cs:line 76
   at System.Net.Http.Functional.Tests.HttpClientHandlerTest_Headers.<>c.<<SendAsync_WithZeroLengthHeaderName_Throws>b__16_1>d.MoveNext() in /_/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs:line 425
--- End of stack trace from previous location ---
   at System.Threading.Tasks.TaskTimeoutExtensions.WhenAllOrAnyFailed(Task[] tasks) in /_/src/libraries/Common/tests/System/Threading/Tasks/TaskTimeoutExtensions.cs:line 63
   at System.Threading.Tasks.TaskTimeoutExtensions.WhenAllOrAnyFailed(Task[] tasks) in /_/src/libraries/Common/tests/System/Threading/Tasks/TaskTimeoutExtensions.cs:line 82
   at System.Net.Test.Common.LoopbackServerFactory.<>c__DisplayClass5_0.<<CreateClientAndServerAsync>b__0>d.MoveNext() in /_/src/libraries/Common/tests/System/Net/Http/GenericLoopbackServer.cs:line 39
--- End of stack trace from previous location ---
   at System.Net.Test.Common.Http3LoopbackServerFactory.CreateServerAsync(Func`3 funcAsync, Int32 millisecondsTimeout, GenericLoopbackOptions options) in /_/src/libraries/Common/tests/System/Net/Http/Http3LoopbackServer.cs:line 101
   at System.Net.Http.Functional.Tests.HttpClientHandlerTest_Headers.SendAsync_WithZeroLengthHeaderName_Throws() in /_/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs:line 413
--- End of stack trace from previous location ---
Author: karelz
Assignees: -
Labels:

area-System.Net.Http
Milestone: 6.0.0

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jul 26, 2021
@karelz karelz added test-run-core Test failures in .NET Core test runs and removed untriaged New issue has not been triaged by the area owner labels Jul 26, 2021
@karelz karelz mentioned this issue Jul 27, 2021
Closed
karelz added a commit that referenced this issue Jul 27, 2021
@karelz
Disable test SendAsync_WithZeroLengthHeaderName_Throws
c83f12b 
Test: System.Net.Http.Functional.Tests.SocketsHttpHandlerTest_HttpClientHandlerTest_Headers_Http3_MsQuic.SendAsync_WithZeroLengthHeaderName_Throws

Disabled test tracked by #56292
@karelz karelz mentioned this issue Jul 27, 2021
Merged
karelz added a commit that referenced this issue Jul 27, 2021
@karelz
Disable test SendAsync_WithZeroLengthHeaderName_Throws (#56363)
f32b55f 
Test: System.Net.Http.Functional.Tests.SocketsHttpHandlerTest_HttpClientHandlerTest_Headers_Http3_MsQuic.SendAsync_WithZeroLengthHeaderName_Throws

Disabled test tracked by #56292
@karelz karelz added the disabled-test The test is disabled in source code against the issue label Jul 27, 2021
@wfurt
Copy link
Member

wfurt commented Jul 28, 2021

This is easy to reproduce. The test actually expects failure e.g. IOException.
The problem is that we throw

      System.Net.Quic.QuicConnectionAbortedException : Connection aborted by peer (256).

My initial reaction was to add it to expected exceptions. But that seems wrong since Quic is not public so there would be no way for users to catch it.

It seems like the right action would be to catch all Quic exceptions and convert them to either HttpRequest or IOExceptions. Thoughts on this @geoffkizer @stephentoub ?

@stephentoub
Copy link
Member

so there would be no way for users to catch it

They can still catch it, just as the base Exception.

In what situation do these exceptions escape unwrapped to the caller? From HttpClient we typically expect all exceptions to be wrapped in an HttpRequestException, or if coming out of a Stream, in an IOException.

@wfurt
Copy link
Member

wfurt commented Jul 28, 2021

Looking at it again, it actually escapes at server side e.g. loopback

        --- End of stack trace from previous location ---
        /home/furt/github/wfurt-runtime/src/libraries/System.Net.Quic/src/System/Net/Quic/Implementations/MsQuic/MsQuicStream.cs(581,0): at System.Net.Quic.Implementations.MsQuic.MsQuicStream.ShutdownCompleted(CancellationToken cancellationToken)
        /home/furt/github/wfurt-runtime/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs(277,0): at System.Net.Test.Common.Http3LoopbackStream.SendResponseBodyAsync(Byte[] content, Boolean isFinal)
        /home/furt/github/wfurt-runtime/src/libraries/Common/tests/System/Net/Http/Http3LoopbackStream.cs(240,0): at System.Net.Test.Common.Http3LoopbackStream.SendResponseAsync(HttpStatusCode statusCode, IList`1 headers, String content, Boolean isFinal)
        /home/furt/github/wfurt-runtime/src/libraries/Common/tests/System/Net/Http/Http3LoopbackConnection.cs(190,0): at System.Net.Test.Common.Http3LoopbackConnection.HandleRequestAsync(HttpStatusCode statusCode, IList`1 headers, String content)
        /home/furt/github/wfurt-runtime/src/libraries/Common/tests/System/Net/Http/Http3LoopbackServer.cs(75,0): at System.Net.Test.Common.Http3LoopbackServer.HandleRequestAsync(HttpStatusCode statusCode, IList`1 headers, String content)
        /home/furt/github/wfurt-runtime/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Headers.cs(427,0): at System.Net.Http.Functional.Tests.HttpClientHandlerTest_Headers.<>c.<<SendAsync_WithZeroLengthHeaderName_Throws>b__16_1>d.MoveNext()
        --- End of stack trace from previous location ---

               async server =>
                {
                    // The client may detect the bad header and close the connection before we are done sending the response.
                    // So, eat any IOException that occurs here.
                    try
                    {
                        await server.HandleRequestAsync(headers: new[]
                        {
                            new HttpHeaderData("", "foo")
                        });
                    }
                    catch (IOException) { }
                });

So I guess less pressing but what the fix should be? Should Http3 loopback wrap or should we add the alternative Quic exceptions to tests as we see fit?

I'll keep eyes on the client side. I just go to moving up from Quic to focusing on HTTP tests.

@wfurt wfurt self-assigned this Jul 29, 2021
@wfurt wfurt mentioned this issue Aug 3, 2021
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 3, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 5, 2021
@geoffkizer geoffkizer mentioned this issue Aug 6, 2021
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Sep 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@wfurt wfurt

Labels
area-System.Net.Http disabled-test The test is disabled in source code against the issue test-run-core Test failures in .NET Core test runs
Projects
None yet
Milestone
6.0.0
Development

Successfully merging a pull request may close this issue.

handle QuicConnectionAbortedException wfurt/runtime
3 participants
@stephentoub @karelz @wfurt