diff --git a/connection.go b/connection.go index 47f4ca99..e0adc80c 100644 --- a/connection.go +++ b/connection.go @@ -190,6 +190,7 @@ type connection struct { clientHelloWritten <-chan *wire.TransportParameters earlyConnReadyChan chan struct{} handshakeCompleteChan chan struct{} // is closed when the handshake completes + sentFirstPacket bool handshakeComplete bool handshakeConfirmed bool @@ -1519,6 +1520,12 @@ func (s *connection) handleCloseError(closeErr *closeError) { s.connIDGenerator.RemoveAll() return } + // Don't send out any CONNECTION_CLOSE if this is an error that occurred + // before we even sent out the first packet. + if s.perspective == protocol.PerspectiveClient && !s.sentFirstPacket { + s.connIDGenerator.RemoveAll() + return + } connClosePacket, err := s.sendConnectionClose(e) if err != nil { s.logger.Debugf("Error sending CONNECTION_CLOSE: %s", err) @@ -1760,6 +1767,7 @@ func (s *connection) sendPacket() (bool, error) { if err != nil || packet == nil { return false, err } + s.sentFirstPacket = true s.logCoalescedPacket(packet) for _, p := range packet.packets { if s.firstAckElicitingPacketAfterIdleSentTime.IsZero() && p.IsAckEliciting() { diff --git a/connection_test.go b/connection_test.go index 1677a6d2..c36681f7 100644 --- a/connection_test.go +++ b/connection_test.go @@ -2477,6 +2477,7 @@ var _ = Describe("Client Connection", func() { conn.packer = packer cryptoSetup = mocks.NewMockCryptoSetup(mockCtrl) conn.cryptoStreamHandler = cryptoSetup + conn.sentFirstPacket = true }) It("changes the connection ID when receiving the first packet from the server", func() { @@ -2568,6 +2569,25 @@ var _ = Describe("Client Connection", func() { Expect(conn.handleAckFrame(ack, protocol.Encryption1RTT)).To(Succeed()) }) + It("doesn't send a CONNECTION_CLOSE when no packet was sent", func() { + conn.sentFirstPacket = false + tracer.EXPECT().ClosedConnection(gomock.Any()) + tracer.EXPECT().Close() + running := make(chan struct{}) + cryptoSetup.EXPECT().RunHandshake().Do(func() { + close(running) + conn.closeLocal(errors.New("early error")) + }) + cryptoSetup.EXPECT().Close() + connRunner.EXPECT().Remove(gomock.Any()) + go func() { + defer GinkgoRecover() + conn.run() + }() + Eventually(running).Should(BeClosed()) + Eventually(areConnsRunning).Should(BeFalse()) + }) + Context("handling tokens", func() { var mockTokenStore *MockTokenStore diff --git a/integrationtests/self/handshake_test.go b/integrationtests/self/handshake_test.go index 8d3bea4d..dee162f7 100644 --- a/integrationtests/self/handshake_test.go +++ b/integrationtests/self/handshake_test.go @@ -12,6 +12,7 @@ import ( "github.com/lucas-clemente/quic-go" "github.com/lucas-clemente/quic-go/integrationtests/tools/israce" "github.com/lucas-clemente/quic-go/internal/protocol" + "github.com/lucas-clemente/quic-go/internal/qerr" "github.com/lucas-clemente/quic-go/logging" . "github.com/onsi/ginkgo" @@ -567,4 +568,37 @@ var _ = Describe("Handshake tests", func() { Expect(token.IsRetryToken).To(BeTrue()) }) }) + + It("doesn't send any packets when generating the ClientHello fails", func() { + ln, err := net.ListenUDP("udp", nil) + Expect(err).ToNot(HaveOccurred()) + done := make(chan struct{}) + packetChan := make(chan struct{}) + go func() { + defer GinkgoRecover() + defer close(done) + for { + _, _, err := ln.ReadFromUDP(make([]byte, protocol.MaxPacketBufferSize)) + if err != nil { + return + } + packetChan <- struct{}{} + } + }() + + tlsConf := getTLSClientConfig() + tlsConf.NextProtos = []string{""} + _, err = quic.DialAddr( + fmt.Sprintf("localhost:%d", ln.LocalAddr().(*net.UDPAddr).Port), + tlsConf, + nil, + ) + Expect(err).To(MatchError(&qerr.TransportError{ + ErrorCode: qerr.InternalError, + ErrorMessage: "tls: invalid NextProtos value", + })) + Consistently(packetChan).ShouldNot(Receive()) + ln.Close() + Eventually(done).Should(BeClosed()) + }) })