From b9f1a0149f6884304fc96ec60872f7ffd9b73a18 Mon Sep 17 00:00:00 2001 From: Nick Banks Date: Thu, 15 Jul 2021 08:49:32 -0500 Subject: [PATCH 1/3] Ignore Duplicate Stream Read Shutdowns --- src/core/stream_recv.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/core/stream_recv.c b/src/core/stream_recv.c index f1c10b72d7..89e0b604bd 100644 --- a/src/core/stream_recv.c +++ b/src/core/stream_recv.c @@ -53,6 +53,13 @@ QuicStreamRecvShutdown( goto Exit; } + if (Stream->Flags.SentStopSending) { + // + // We've already aborted locally. Just ignore any additional shutdowns. + // + goto Exit; + } + // // Disable all future receive events. // From f1aef4eededa416b5b7e67f2cd998ee98a4f0a72 Mon Sep 17 00:00:00 2001 From: Nick Banks Date: Thu, 15 Jul 2021 09:57:39 -0500 Subject: [PATCH 2/3] Add Tests --- src/test/bin/quic_gtest.h | 2 +- src/test/lib/EventTest.cpp | 139 ++++++++++++++++++++++++++++++++++++- 2 files changed, 139 insertions(+), 2 deletions(-) diff --git a/src/test/bin/quic_gtest.h b/src/test/bin/quic_gtest.h index 1d6f4d8982..d71e3684f0 100644 --- a/src/test/bin/quic_gtest.h +++ b/src/test/bin/quic_gtest.h @@ -655,7 +655,7 @@ struct ValidateStreamEventArgs { uint32_t Test; static ::std::vector Generate() { ::std::vector list; - for (uint32_t Test = 0; Test < 6; ++Test) + for (uint32_t Test = 0; Test < 7; ++Test) list.push_back({ Test }); return list; } diff --git a/src/test/lib/EventTest.cpp b/src/test/lib/EventTest.cpp index 468bff9ba7..6697aa36e6 100644 --- a/src/test/lib/EventTest.cpp +++ b/src/test/lib/EventTest.cpp @@ -71,6 +71,32 @@ struct StreamStartCompleteEventValidator : StreamEventValidator { } }; +struct StreamPeerRecvAbortEventValidator : StreamEventValidator { + QUIC_UINT62 ErrorCode; + StreamPeerRecvAbortEventValidator(QUIC_UINT62 errorcode, uint8_t actions = 0, bool optional = false) : + StreamEventValidator(QUIC_STREAM_EVENT_PEER_RECEIVE_ABORTED, actions, optional), + ErrorCode(errorcode) { } + virtual void Validate(_In_ HQUIC Stream, _Inout_ QUIC_STREAM_EVENT* Event) { + if (Event->Type != Type) { + if (!Optional) { + TEST_FAILURE("StreamEventValidator: Expected %u. Actual %u", Type, Event->Type); + } + return; + } + if (Event->PEER_RECEIVE_ABORTED.ErrorCode != ErrorCode) { + TEST_FAILURE("PeerRecvAbort mismatch: Expected %llu. Actual %llu", ErrorCode, Event->PEER_RECEIVE_ABORTED.ErrorCode); + return; + } + Success = true; + if (Actions & QUIC_EVENT_ACTION_SHUTDOWN_STREAM) { + MsQuic->StreamShutdown(Stream, QUIC_STREAM_SHUTDOWN_FLAG_GRACEFUL, 0); + } + if (Actions & QUIC_EVENT_ACTION_SHUTDOWN_CONNECTION) { + MsQuic->ConnectionShutdown(Stream, QUIC_CONNECTION_SHUTDOWN_FLAG_NONE, 0); + } + } +}; + struct StreamValidator { HQUIC Handle; StreamEventValidator** ExpectedEvents; @@ -1177,6 +1203,116 @@ QuicTestValidateStreamEvents6( } // Connections scope } +void +QuicTestValidateStreamEvents7( + _In_ MsQuicRegistration& Registration, + _In_ HQUIC Listener, + _In_ QuicAddr& ServerLocalAddr + ) +{ + TestScopeLogger ScopeLogger(__FUNCTION__); + + MsQuicSettings Settings; + Settings.SetPeerBidiStreamCount(1).SetMinimumMtu(1280).SetMaximumMtu(1280); + MsQuicConfiguration ServerConfiguration(Registration, "MsQuicTest", Settings, ServerSelfSignedCredConfig); + TEST_TRUE(ServerConfiguration.IsValid()); + + MsQuicConfiguration ClientConfiguration(Registration, "MsQuicTest", MsQuicSettings().SetMinimumMtu(1280).SetMaximumMtu(1280), MsQuicCredentialConfig()); + TEST_TRUE(ClientConfiguration.IsValid()); + + { // Connections scope + ConnValidator Client, Server(ServerConfiguration); + + MsQuic->SetContext(Listener, &Server); + + TEST_QUIC_SUCCEEDED( + MsQuic->ConnectionOpen( + Registration, + ConnValidatorCallback, + &Client, + &Client.Handle)); + + { // Stream scope + + StreamValidator ClientStream( + new(std::nothrow) StreamEventValidator* [4] { + new(std::nothrow) StreamStartCompleteEventValidator(), + new(std::nothrow) StreamEventValidator(QUIC_STREAM_EVENT_SEND_SHUTDOWN_COMPLETE, 0, true), + new(std::nothrow) StreamEventValidator(QUIC_STREAM_EVENT_SHUTDOWN_COMPLETE, QUIC_EVENT_ACTION_SHUTDOWN_CONNECTION), + nullptr + }); + StreamValidator ServerStream( + new(std::nothrow) StreamEventValidator* [6] { + new(std::nothrow) StreamPeerRecvAbortEventValidator(0), + new(std::nothrow) StreamEventValidator(QUIC_STREAM_EVENT_RECEIVE), + new(std::nothrow) StreamEventValidator(QUIC_STREAM_EVENT_PEER_SEND_SHUTDOWN), + new(std::nothrow) StreamEventValidator(QUIC_STREAM_EVENT_SEND_SHUTDOWN_COMPLETE), + new(std::nothrow) StreamEventValidator(QUIC_STREAM_EVENT_SHUTDOWN_COMPLETE), + nullptr + }); + + Client.SetExpectedEvents( + new(std::nothrow) ConnEventValidator* [7] { + new(std::nothrow) ConnEventValidator(QUIC_CONNECTION_EVENT_STREAMS_AVAILABLE), + new(std::nothrow) ConnEventValidator(QUIC_CONNECTION_EVENT_DATAGRAM_STATE_CHANGED), + new(std::nothrow) ConnEventValidator(QUIC_CONNECTION_EVENT_CONNECTED), + new(std::nothrow) ConnEventValidator(QUIC_CONNECTION_EVENT_STREAMS_AVAILABLE, 0, true), + new(std::nothrow) ConnEventValidator(QUIC_CONNECTION_EVENT_RESUMPTION_TICKET_RECEIVED, 0, true), // TODO - Schannel does resumption regardless + new(std::nothrow) ConnEventValidator(QUIC_CONNECTION_EVENT_SHUTDOWN_COMPLETE), + nullptr + }); + Server.SetExpectedEvents( + new(std::nothrow) ConnEventValidator* [6] { + new(std::nothrow) ConnEventValidator(QUIC_CONNECTION_EVENT_CONNECTED), + new(std::nothrow) NewStreamEventValidator(&ServerStream), + new(std::nothrow) ConnEventValidator(QUIC_CONNECTION_EVENT_SHUTDOWN_INITIATED_BY_PEER), + new(std::nothrow) ConnEventValidator(QUIC_CONNECTION_EVENT_SHUTDOWN_COMPLETE), + nullptr + }); + + TEST_QUIC_SUCCEEDED( + MsQuic->StreamOpen( + Client.Handle, + QUIC_STREAM_OPEN_FLAG_NONE, + StreamValidatorCallback, + &ClientStream, + &ClientStream.Handle)); + TEST_QUIC_SUCCEEDED( + MsQuic->StreamStart( + ClientStream.Handle, + QUIC_STREAM_START_FLAG_NONE)); + TEST_QUIC_SUCCEEDED( + MsQuic->StreamShutdown( + ClientStream.Handle, + QUIC_STREAM_SHUTDOWN_FLAG_GRACEFUL, + 0)); + TEST_QUIC_SUCCEEDED( + MsQuic->StreamShutdown( + ClientStream.Handle, + QUIC_STREAM_SHUTDOWN_FLAG_ABORT_RECEIVE, + 0)); + TEST_QUIC_SUCCEEDED( + MsQuic->StreamShutdown( + ClientStream.Handle, + QUIC_STREAM_SHUTDOWN_FLAG_ABORT_RECEIVE, + 0xFFFF)); + + TEST_QUIC_SUCCEEDED( + MsQuic->ConnectionStart( + Client.Handle, + ClientConfiguration, + QuicAddrGetFamily(&ServerLocalAddr.SockAddr), + QUIC_LOCALHOST_FOR_AF( + QuicAddrGetFamily(&ServerLocalAddr.SockAddr)), + ServerLocalAddr.GetPort())); + + TEST_TRUE(Client.Complete.WaitTimeout(2000)); + TEST_TRUE(Server.Complete.WaitTimeout(1000)); + + } // Stream scope + } // Connections scope +} + void QuicTestValidateStreamEvents(uint32_t Test) { MsQuicRegistration Registration(true); @@ -1200,7 +1336,8 @@ void QuicTestValidateStreamEvents(uint32_t Test) QuicTestValidateStreamEvents3, QuicTestValidateStreamEvents4, QuicTestValidateStreamEvents5, - QuicTestValidateStreamEvents6 + QuicTestValidateStreamEvents6, + QuicTestValidateStreamEvents7 }; Tests[Test](Registration, Listener, ServerLocalAddr); From 2182ea82cb3b20d41dcbd87fa9086beb42d31d4a Mon Sep 17 00:00:00 2001 From: Nick Banks Date: Thu, 15 Jul 2021 10:03:10 -0500 Subject: [PATCH 3/3] Improve test abstraction --- src/test/lib/EventTest.cpp | 42 ++++++++++---------------------------- 1 file changed, 11 insertions(+), 31 deletions(-) diff --git a/src/test/lib/EventTest.cpp b/src/test/lib/EventTest.cpp index 6697aa36e6..aa9c63f179 100644 --- a/src/test/lib/EventTest.cpp +++ b/src/test/lib/EventTest.cpp @@ -27,13 +27,16 @@ struct StreamEventValidator { uint8_t Actions; StreamEventValidator(QUIC_STREAM_EVENT_TYPE type, uint8_t actions = 0, bool optional = false) : Success(false), Optional(optional), Type(type), Actions(actions) { } - virtual void Validate(_In_ HQUIC Stream, _Inout_ QUIC_STREAM_EVENT* Event) { + void Validate(_In_ HQUIC Stream, _Inout_ QUIC_STREAM_EVENT* Event) { if (Event->Type != Type) { if (!Optional) { TEST_FAILURE("StreamEventValidator: Expected %u. Actual %u", Type, Event->Type); } return; } + if (!ValidateMore(Stream, Event)) { + return; + } Success = true; if (Actions & QUIC_EVENT_ACTION_SHUTDOWN_STREAM) { MsQuic->StreamShutdown(Stream, QUIC_STREAM_SHUTDOWN_FLAG_GRACEFUL, 0); @@ -42,6 +45,7 @@ struct StreamEventValidator { MsQuic->ConnectionShutdown(Stream, QUIC_CONNECTION_SHUTDOWN_FLAG_NONE, 0); } } + virtual bool ValidateMore(_In_ HQUIC, _Inout_ QUIC_STREAM_EVENT*) { return true; } virtual ~StreamEventValidator() { } }; @@ -50,24 +54,12 @@ struct StreamStartCompleteEventValidator : StreamEventValidator { StreamStartCompleteEventValidator(bool PeerAccepted = false, uint8_t actions = 0, bool optional = false) : StreamEventValidator(QUIC_STREAM_EVENT_START_COMPLETE, actions, optional), PeerAccepted(PeerAccepted ? TRUE : FALSE) { } - virtual void Validate(_In_ HQUIC Stream, _Inout_ QUIC_STREAM_EVENT* Event) { - if (Event->Type != Type) { - if (!Optional) { - TEST_FAILURE("StreamEventValidator: Expected %u. Actual %u", Type, Event->Type); - } - return; - } + virtual bool ValidateMore(_In_ HQUIC, _Inout_ QUIC_STREAM_EVENT* Event) { if (Event->START_COMPLETE.PeerAccepted != PeerAccepted) { TEST_FAILURE("PeerAccepted mismatch: Expected %u. Actual %u", PeerAccepted, Event->START_COMPLETE.PeerAccepted); - return; - } - Success = true; - if (Actions & QUIC_EVENT_ACTION_SHUTDOWN_STREAM) { - MsQuic->StreamShutdown(Stream, QUIC_STREAM_SHUTDOWN_FLAG_GRACEFUL, 0); - } - if (Actions & QUIC_EVENT_ACTION_SHUTDOWN_CONNECTION) { - MsQuic->ConnectionShutdown(Stream, QUIC_CONNECTION_SHUTDOWN_FLAG_NONE, 0); + return false; } + return true; } }; @@ -76,24 +68,12 @@ struct StreamPeerRecvAbortEventValidator : StreamEventValidator { StreamPeerRecvAbortEventValidator(QUIC_UINT62 errorcode, uint8_t actions = 0, bool optional = false) : StreamEventValidator(QUIC_STREAM_EVENT_PEER_RECEIVE_ABORTED, actions, optional), ErrorCode(errorcode) { } - virtual void Validate(_In_ HQUIC Stream, _Inout_ QUIC_STREAM_EVENT* Event) { - if (Event->Type != Type) { - if (!Optional) { - TEST_FAILURE("StreamEventValidator: Expected %u. Actual %u", Type, Event->Type); - } - return; - } + virtual bool ValidateMore(_In_ HQUIC, _Inout_ QUIC_STREAM_EVENT* Event) { if (Event->PEER_RECEIVE_ABORTED.ErrorCode != ErrorCode) { TEST_FAILURE("PeerRecvAbort mismatch: Expected %llu. Actual %llu", ErrorCode, Event->PEER_RECEIVE_ABORTED.ErrorCode); - return; - } - Success = true; - if (Actions & QUIC_EVENT_ACTION_SHUTDOWN_STREAM) { - MsQuic->StreamShutdown(Stream, QUIC_STREAM_SHUTDOWN_FLAG_GRACEFUL, 0); - } - if (Actions & QUIC_EVENT_ACTION_SHUTDOWN_CONNECTION) { - MsQuic->ConnectionShutdown(Stream, QUIC_CONNECTION_SHUTDOWN_FLAG_NONE, 0); + return false; } + return true; } };