From 030ba861fc3659bb987bcc598a6ab153fb51f58a Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 25 Aug 2023 12:56:12 +0700 Subject: [PATCH 1/5] swarm: return a more meaningful error when dialing QUIC draft-29 --- p2p/net/swarm/dial_test.go | 8 ++++++++ p2p/net/swarm/swarm_dial.go | 15 ++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/p2p/net/swarm/dial_test.go b/p2p/net/swarm/dial_test.go index 324a985391..2da04ed572 100644 --- a/p2p/net/swarm/dial_test.go +++ b/p2p/net/swarm/dial_test.go @@ -642,3 +642,11 @@ func TestDialSelf(t *testing.T) { _, err := s1.DialPeer(context.Background(), s1.LocalPeer()) require.ErrorIs(t, err, swarm.ErrDialToSelf, "expected error from self dial") } + +func TestDialQUICDraft29(t *testing.T) { + s := makeDialOnlySwarm(t) + id := testutil.RandPeerIDFatal(t) + s.Peerstore().AddAddr(id, ma.StringCast("/ip4/127.0.0.1/udp/1234/quic"), time.Hour) + _, err := s.DialPeer(context.Background(), id) + require.ErrorIs(t, err, swarm.ErrQUICDraft29) +} diff --git a/p2p/net/swarm/swarm_dial.go b/p2p/net/swarm/swarm_dial.go index e17c27353b..9948471258 100644 --- a/p2p/net/swarm/swarm_dial.go +++ b/p2p/net/swarm/swarm_dial.go @@ -14,8 +14,10 @@ import ( "github.com/libp2p/go-libp2p/core/peer" "github.com/libp2p/go-libp2p/core/peerstore" "github.com/libp2p/go-libp2p/core/transport" + ma "github.com/multiformats/go-multiaddr" madns "github.com/multiformats/go-multiaddr-dns" + mafmt "github.com/multiformats/go-multiaddr-fmt" manet "github.com/multiformats/go-multiaddr/net" ) @@ -63,6 +65,9 @@ var ( // ErrGaterDisallowedConnection is returned when the gater prevents us from // forming a connection with a peer. ErrGaterDisallowedConnection = errors.New("gater disallows connection to peer") + + // ErrQUICDraft29 is returned instead of an ErrNoTransport when attempting to dial a QUIC draft-29 address. + ErrQUICDraft29 = errors.New("QUIC draft-29 has been removed, QUIC (RFC 9000) is accessible with /quic-v1") ) // DialAttempts governs how many times a goroutine will try to dial a given peer. @@ -407,6 +412,8 @@ func (s *Swarm) nonProxyAddr(addr ma.Multiaddr) bool { return !t.Proxy() } +var quicDraft29DialMatcher = mafmt.And(mafmt.IP, mafmt.Base(ma.P_UDP), mafmt.Base(ma.P_QUIC)) + // filterKnownUndialables takes a list of multiaddrs, and removes those // that we definitely don't want to dial: addresses configured to be blocked, // IPv6 link-local addresses, addresses without a dial-capable transport, @@ -435,7 +442,13 @@ func (s *Swarm) filterKnownUndialables(p peer.ID, addrs []ma.Multiaddr) (goodAdd // filter addresses with no transport addrs = ma.FilterAddrs(addrs, func(a ma.Multiaddr) bool { if s.TransportForDialing(a) == nil { - addrErrs = append(addrErrs, TransportError{Address: a, Cause: ErrNoTransport}) + e := ErrNoTransport + // We used to support QUIC draft-29 for a long time. + // Provide a more useful error when attempting to dial a QUIC draft-29 address. + if quicDraft29DialMatcher.Matches(a) { + e = ErrQUICDraft29 + } + addrErrs = append(addrErrs, TransportError{Address: a, Cause: e}) return false } return true From 2ecd33ee275e5e1c9c7e8200e932c632bd0033cb Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 25 Aug 2023 02:31:47 -0700 Subject: [PATCH 2/5] Apply suggestions from code review Co-authored-by: Jorropo --- p2p/net/swarm/dial_test.go | 1 + p2p/net/swarm/swarm_dial.go | 14 +++++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/p2p/net/swarm/dial_test.go b/p2p/net/swarm/dial_test.go index 2da04ed572..a614183bba 100644 --- a/p2p/net/swarm/dial_test.go +++ b/p2p/net/swarm/dial_test.go @@ -649,4 +649,5 @@ func TestDialQUICDraft29(t *testing.T) { s.Peerstore().AddAddr(id, ma.StringCast("/ip4/127.0.0.1/udp/1234/quic"), time.Hour) _, err := s.DialPeer(context.Background(), id) require.ErrorIs(t, err, swarm.ErrQUICDraft29) + require.ErrorIs(t, err, swarm.ErrNoTransport) } diff --git a/p2p/net/swarm/swarm_dial.go b/p2p/net/swarm/swarm_dial.go index 9948471258..e48763e425 100644 --- a/p2p/net/swarm/swarm_dial.go +++ b/p2p/net/swarm/swarm_dial.go @@ -65,11 +65,19 @@ var ( // ErrGaterDisallowedConnection is returned when the gater prevents us from // forming a connection with a peer. ErrGaterDisallowedConnection = errors.New("gater disallows connection to peer") - - // ErrQUICDraft29 is returned instead of an ErrNoTransport when attempting to dial a QUIC draft-29 address. - ErrQUICDraft29 = errors.New("QUIC draft-29 has been removed, QUIC (RFC 9000) is accessible with /quic-v1") ) +// ErrQUICDraft29 wraps [ErrNoTransport] and provide a more meaningful error message while +type ErrQUICDraft29 struct{} + +func (ErrQUICDraft29) Error() string { + return "QUIC draft-29 has been removed, QUIC (RFC 9000) is accessible with /quic-v1" +} + +func (ErrQUICDraft29) Unwrap() error { + return ErrNoTransport +} + // DialAttempts governs how many times a goroutine will try to dial a given peer. // Note: this is down to one, as we have _too many dials_ atm. To add back in, // add loop back in Dial(.) From e80ec9d6c7258f2bc97e8c0a090fa3e288236c07 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 25 Aug 2023 02:32:26 -0700 Subject: [PATCH 3/5] Update p2p/net/swarm/swarm_dial.go --- p2p/net/swarm/swarm_dial.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/net/swarm/swarm_dial.go b/p2p/net/swarm/swarm_dial.go index e48763e425..6613b8b705 100644 --- a/p2p/net/swarm/swarm_dial.go +++ b/p2p/net/swarm/swarm_dial.go @@ -67,7 +67,7 @@ var ( ErrGaterDisallowedConnection = errors.New("gater disallows connection to peer") ) -// ErrQUICDraft29 wraps [ErrNoTransport] and provide a more meaningful error message while +// ErrQUICDraft29 wraps ErrNoTransport and provide a more meaningful error message type ErrQUICDraft29 struct{} func (ErrQUICDraft29) Error() string { From 827a185b24c23ec5ca813e5a3ad1867d91ea4b61 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 25 Aug 2023 16:34:53 +0700 Subject: [PATCH 4/5] fix --- p2p/net/swarm/dial_test.go | 2 +- p2p/net/swarm/swarm_dial.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/p2p/net/swarm/dial_test.go b/p2p/net/swarm/dial_test.go index a614183bba..3e7b09da43 100644 --- a/p2p/net/swarm/dial_test.go +++ b/p2p/net/swarm/dial_test.go @@ -648,6 +648,6 @@ func TestDialQUICDraft29(t *testing.T) { id := testutil.RandPeerIDFatal(t) s.Peerstore().AddAddr(id, ma.StringCast("/ip4/127.0.0.1/udp/1234/quic"), time.Hour) _, err := s.DialPeer(context.Background(), id) - require.ErrorIs(t, err, swarm.ErrQUICDraft29) + require.ErrorIs(t, err, swarm.ErrQUICDraft29{}) require.ErrorIs(t, err, swarm.ErrNoTransport) } diff --git a/p2p/net/swarm/swarm_dial.go b/p2p/net/swarm/swarm_dial.go index 6613b8b705..f403d8298a 100644 --- a/p2p/net/swarm/swarm_dial.go +++ b/p2p/net/swarm/swarm_dial.go @@ -71,11 +71,11 @@ var ( type ErrQUICDraft29 struct{} func (ErrQUICDraft29) Error() string { - return "QUIC draft-29 has been removed, QUIC (RFC 9000) is accessible with /quic-v1" + return "QUIC draft-29 has been removed, QUIC (RFC 9000) is accessible with /quic-v1" } func (ErrQUICDraft29) Unwrap() error { - return ErrNoTransport + return ErrNoTransport } // DialAttempts governs how many times a goroutine will try to dial a given peer. @@ -454,7 +454,7 @@ func (s *Swarm) filterKnownUndialables(p peer.ID, addrs []ma.Multiaddr) (goodAdd // We used to support QUIC draft-29 for a long time. // Provide a more useful error when attempting to dial a QUIC draft-29 address. if quicDraft29DialMatcher.Matches(a) { - e = ErrQUICDraft29 + e = ErrQUICDraft29{} } addrErrs = append(addrErrs, TransportError{Address: a, Cause: e}) return false From 263476a8751aa83bdc4ff2213fceab7ddbe16812 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 25 Aug 2023 16:37:21 +0700 Subject: [PATCH 5/5] better types --- p2p/net/swarm/dial_test.go | 2 +- p2p/net/swarm/swarm_dial.go | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/p2p/net/swarm/dial_test.go b/p2p/net/swarm/dial_test.go index 3e7b09da43..a614183bba 100644 --- a/p2p/net/swarm/dial_test.go +++ b/p2p/net/swarm/dial_test.go @@ -648,6 +648,6 @@ func TestDialQUICDraft29(t *testing.T) { id := testutil.RandPeerIDFatal(t) s.Peerstore().AddAddr(id, ma.StringCast("/ip4/127.0.0.1/udp/1234/quic"), time.Hour) _, err := s.DialPeer(context.Background(), id) - require.ErrorIs(t, err, swarm.ErrQUICDraft29{}) + require.ErrorIs(t, err, swarm.ErrQUICDraft29) require.ErrorIs(t, err, swarm.ErrNoTransport) } diff --git a/p2p/net/swarm/swarm_dial.go b/p2p/net/swarm/swarm_dial.go index f403d8298a..1e5638f379 100644 --- a/p2p/net/swarm/swarm_dial.go +++ b/p2p/net/swarm/swarm_dial.go @@ -68,13 +68,15 @@ var ( ) // ErrQUICDraft29 wraps ErrNoTransport and provide a more meaningful error message -type ErrQUICDraft29 struct{} +var ErrQUICDraft29 errQUICDraft29 -func (ErrQUICDraft29) Error() string { +type errQUICDraft29 struct{} + +func (errQUICDraft29) Error() string { return "QUIC draft-29 has been removed, QUIC (RFC 9000) is accessible with /quic-v1" } -func (ErrQUICDraft29) Unwrap() error { +func (errQUICDraft29) Unwrap() error { return ErrNoTransport } @@ -454,7 +456,7 @@ func (s *Swarm) filterKnownUndialables(p peer.ID, addrs []ma.Multiaddr) (goodAdd // We used to support QUIC draft-29 for a long time. // Provide a more useful error when attempting to dial a QUIC draft-29 address. if quicDraft29DialMatcher.Matches(a) { - e = ErrQUICDraft29{} + e = ErrQUICDraft29 } addrErrs = append(addrErrs, TransportError{Address: a, Cause: e}) return false