From ede46e0eeafad55421952ef4c594c28df36105c9 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 19 Jan 2018 14:56:27 -0800 Subject: [PATCH 01/55] initial commit --- p2p/net/upgrader/conn.go | 17 +++++ p2p/net/upgrader/listener.go | 142 +++++++++++++++++++++++++++++++++++ p2p/net/upgrader/upgrader.go | 120 +++++++++++++++++++++++++++++ 3 files changed, 279 insertions(+) create mode 100644 p2p/net/upgrader/conn.go create mode 100644 p2p/net/upgrader/listener.go create mode 100644 p2p/net/upgrader/upgrader.go diff --git a/p2p/net/upgrader/conn.go b/p2p/net/upgrader/conn.go new file mode 100644 index 0000000000..a9f5f36bd7 --- /dev/null +++ b/p2p/net/upgrader/conn.go @@ -0,0 +1,17 @@ +package stream + +import ( + transport "github.com/libp2p/go-libp2p-transport" + smux "github.com/libp2p/go-stream-muxer" +) + +type transportConn struct { + smux.Conn + transport.ConnMultiaddrs + transport.ConnSecurity + transport transport.Transport +} + +func (t *transportConn) Transport() transport.Transport { + return t.transport +} diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go new file mode 100644 index 0000000000..cc8e16743c --- /dev/null +++ b/p2p/net/upgrader/listener.go @@ -0,0 +1,142 @@ +package stream + +import ( + "context" + "fmt" + "sync" + + logging "github.com/ipfs/go-log" + tec "github.com/jbenet/go-temp-err-catcher" + transport "github.com/libp2p/go-libp2p-transport" + manet "github.com/multiformats/go-multiaddr-net" +) + +var log = logging.Logger("stream-upgrader") + +type connErr struct { + conn transport.Conn + err error +} + +type listener struct { + manet.Listener + + transport transport.Transport + upgrader *Upgrader + + incoming chan transport.Conn + err error + + ticket chan struct{} + + ctx context.Context + cancel func() +} + +// Close closes the listener. +func (l *listener) Close() error { + // Do this first to try to get any relevent errors. + err := l.Listener.Close() + + l.cancel() + // Drain and wait. + for c := range l.incoming { + c.Close() + } + return err +} + +func (l *listener) handleIncoming() { + var wg sync.WaitGroup + defer func() { + // make sure we're closed + l.Listener.Close() + if l.err == nil { + l.err = fmt.Errorf("listener closed") + } + + wg.Wait() + close(l.incoming) + }() + + var catcher tec.TempErrCatcher + for { + + select { + case <-l.ticket: + case <-l.ctx.Done(): + return + } + + maconn, err := l.Listener.Accept() + if err != nil { + if catcher.IsTemporary(err) { + log.Infof("temporary accept error: %s", err) + continue + } + l.err = err + return + } + + log.Debugf("listener %s got connection: %s <---> %s", + l, + maconn.LocalMultiaddr(), + maconn.RemoteMultiaddr()) + + wg.Add(1) + go func() { + defer wg.Done() + + ctx, cancel := context.WithTimeout(l.ctx, transport.AcceptTimeout) + defer cancel() + + conn, err := l.upgrader.UpgradeInbound(ctx, l.transport, maconn) + if err != nil { + // Don't bother bubbling this up. We just failed + // to completely negotiate the connection. + log.Debugf("accept upgrade error: %s (%s <--> %s)", + err, + conn.LocalMultiaddr(), + conn.RemoteMultiaddr()) + return + } + + log.Debugf("listener %s accepted connection: %s (%s) <---> %s (%s)", + l, + conn.LocalMultiaddr(), + conn.LocalPeer(), + conn.RemoteMultiaddr(), + conn.RemotePeer()) + + select { + case l.incoming <- conn: + case <-ctx.Done(): + if l.ctx.Err() == nil { + // Listener *not* closed but the accept timeout expired. + log.Warningf("listener dropped connection due to slow accept") + } + // Wait on the context with a timeout. This way, + // if we stop accepting connections for some reason, + // we'll eventually close all the open ones + // instead of hanging onto them. + conn.Close() + } + }() + } +} + +// Accept accepts a connection. +func (l *listener) Accept() (transport.Conn, error) { + for { + select { + case l.ticket <- struct{}{}: + case c, ok := <-l.incoming: + if !ok { + return nil, l.err + } + return c, nil + } + } +} + +var _ transport.Listener = (*listener)(nil) diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go new file mode 100644 index 0000000000..3da10bf798 --- /dev/null +++ b/p2p/net/upgrader/upgrader.go @@ -0,0 +1,120 @@ +package stream + +import ( + "context" + "errors" + "fmt" + "net" + + pnet "github.com/libp2p/go-libp2p-interface-pnet" + peer "github.com/libp2p/go-libp2p-peer" + transport "github.com/libp2p/go-libp2p-transport" + filter "github.com/libp2p/go-maddr-filter" + smux "github.com/libp2p/go-stream-muxer" + ss "github.com/libp2p/go-stream-security" + manet "github.com/multiformats/go-multiaddr-net" +) + +// ErrNilPeer is returned when attempting to upgrade an outbound connection +// without specifying a peer ID. +var ErrNilPeer = errors.New("nil peer") + +// Upgrader is a multistream upgrader that can upgrade an underlying connection +// to a full transport connection (secure and multiplexed). +type Upgrader struct { + Protector pnet.Protector + Secure ss.Transport + Muxer smux.Transport + Filters *filter.Filters +} + +// UpgradeListener upgrades the passed multiaddr-net listener into a full libp2p-transport listener. +func (u *Upgrader) UpgradeListener(t transport.Transport, list manet.Listener) transport.Listener { + ctx, cancel := context.WithCancel(context.Background()) + l := &listener{ + Listener: list, + upgrader: u, + transport: t, + ticket: make(chan struct{}), + incoming: make(chan transport.Conn), + cancel: cancel, + ctx: ctx, + } + go l.handleIncoming() + return l +} + +// UpgradeOutbound upgrades the given outbound multiaddr-net connection into a +// full libp2p-transport connection. +func (u *Upgrader) UpgradeOutbound(ctx context.Context, t transport.Transport, maconn manet.Conn, p peer.ID) (transport.Conn, error) { + if p == "" { + return nil, ErrNilPeer + } + return u.upgrade(ctx, t, maconn, p) +} + +// UpgradeInbound upgrades the given inbound multiaddr-net connection into a +// full libp2p-transport connection. +func (u *Upgrader) UpgradeInbound(ctx context.Context, t transport.Transport, maconn manet.Conn) (transport.Conn, error) { + return u.upgrade(ctx, t, maconn, "") +} + +func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn manet.Conn, p peer.ID) (transport.Conn, error) { + if u.Filters != nil && u.Filters.AddrBlocked(maconn.RemoteMultiaddr()) { + log.Debugf("blocked connection from %s", maconn.RemoteMultiaddr()) + maconn.Close() + return nil, fmt.Errorf("blocked connection from %s", maconn.RemoteMultiaddr()) + } + + var conn net.Conn = maconn + if u.Protector != nil { + pconn, err := u.Protector.Protect(conn) + if err != nil { + conn.Close() + return nil, err + } + conn = pconn + } + sconn, err := u.setupSecurity(ctx, conn, p) + if err != nil { + conn.Close() + return nil, err + } + smconn, err := u.setupMuxer(ctx, sconn, p) + if err != nil { + conn.Close() + return nil, err + } + return &transportConn{ + Conn: smconn, + ConnMultiaddrs: maconn, + ConnSecurity: sconn, + transport: t, + }, nil +} + +func (u *Upgrader) setupSecurity(ctx context.Context, conn net.Conn, p peer.ID) (ss.Conn, error) { + if p == "" { + return u.Secure.SecureInbound(ctx, conn) + } + return u.Secure.SecureOutbound(ctx, conn, p) +} + +func (u *Upgrader) setupMuxer(ctx context.Context, conn net.Conn, p peer.ID) (smux.Conn, error) { + // TODO: The muxer should take a context. + done := make(chan struct{}) + + var smconn smux.Conn + var err error + go func() { + defer close(done) + smconn, err = u.Muxer.NewConn(conn, p == "") + }() + + select { + case <-done: + return smconn, err + case <-ctx.Done(): + return nil, ctx.Err() + } +} From 48221760c6128bcb1e86de789fb6b0ccba823f8e Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 19 Jan 2018 16:27:04 -0800 Subject: [PATCH 02/55] fix nil dereference --- p2p/net/upgrader/listener.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index cc8e16743c..3c7408955e 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -96,8 +96,8 @@ func (l *listener) handleIncoming() { // to completely negotiate the connection. log.Debugf("accept upgrade error: %s (%s <--> %s)", err, - conn.LocalMultiaddr(), - conn.RemoteMultiaddr()) + maconn.LocalMultiaddr(), + maconn.RemoteMultiaddr()) return } From cf38072bb6220b471503a4fd627568007e7bc21f Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 19 Jan 2018 16:48:12 -0800 Subject: [PATCH 03/55] nicer stringers --- p2p/net/upgrader/conn.go | 17 +++++++++++++++++ p2p/net/upgrader/listener.go | 14 ++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/p2p/net/upgrader/conn.go b/p2p/net/upgrader/conn.go index a9f5f36bd7..50d5a5560b 100644 --- a/p2p/net/upgrader/conn.go +++ b/p2p/net/upgrader/conn.go @@ -1,6 +1,8 @@ package stream import ( + "fmt" + transport "github.com/libp2p/go-libp2p-transport" smux "github.com/libp2p/go-stream-muxer" ) @@ -15,3 +17,18 @@ type transportConn struct { func (t *transportConn) Transport() transport.Transport { return t.transport } + +func (t *transportConn) String() string { + ts := "" + if s, ok := t.transport.(fmt.Stringer); ok { + ts = "[" + s.String() + "]" + } + return fmt.Sprintf( + " %s (%s)>", + ts, + t.LocalMultiaddr(), + t.LocalPeer(), + t.RemoteMultiaddr(), + t.RemotePeer(), + ) +} diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index 3c7408955e..c1d73ae570 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -101,12 +101,7 @@ func (l *listener) handleIncoming() { return } - log.Debugf("listener %s accepted connection: %s (%s) <---> %s (%s)", - l, - conn.LocalMultiaddr(), - conn.LocalPeer(), - conn.RemoteMultiaddr(), - conn.RemotePeer()) + log.Debugf("listener %s accepted connection: %s", l, conn) select { case l.incoming <- conn: @@ -139,4 +134,11 @@ func (l *listener) Accept() (transport.Conn, error) { } } +func (l *listener) String() string { + if s, ok := l.transport.(fmt.Stringer); ok { + return fmt.Sprintf("", s, l.Multiaddr()) + } + return fmt.Sprintf("", l.Multiaddr()) +} + var _ transport.Listener = (*listener)(nil) From 61cd627405e18bcd591d7861be051683265b612e Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Mon, 22 Jan 2018 00:05:13 -0800 Subject: [PATCH 04/55] keep accepting and negotiating connections until we have 16 ready and queued up Otherwise, we'll have an annoying saw-tooth pattern where we'll accept a bunch of connections, set them up in parallel, and then wait until we pass them all back up to the swarm before accepting any more. This commit allows us to queue up 16 ready connections before ceasing to accept any new connections. We'll still probably have a saw-tooth pattern under heavy load but it should be less pronounced (and we can improve the situation by upping the queue size. --- p2p/net/upgrader/listener.go | 30 +++++++++---------- p2p/net/upgrader/threshold.go | 56 +++++++++++++++++++++++++++++++++++ p2p/net/upgrader/upgrader.go | 5 +++- 3 files changed, 74 insertions(+), 17 deletions(-) create mode 100644 p2p/net/upgrader/threshold.go diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index c1d73ae570..72d129ced1 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -27,8 +27,11 @@ type listener struct { incoming chan transport.Conn err error - ticket chan struct{} + // Used for backpressure + threshold *threshold + // Canceling this context isn't sufficient to tear down the listener. + // Call close. ctx context.Context cancel func() } @@ -61,12 +64,10 @@ func (l *listener) handleIncoming() { var catcher tec.TempErrCatcher for { - - select { - case <-l.ticket: - case <-l.ctx.Done(): - return - } + // no need to wait on the context. + // Close will drain the incoming channel which will cause the + // threshold to drop. + l.threshold.Wait() maconn, err := l.Listener.Accept() if err != nil { @@ -103,6 +104,9 @@ func (l *listener) handleIncoming() { log.Debugf("listener %s accepted connection: %s", l, conn) + l.threshold.Acquire() + defer l.threshold.Release() + select { case l.incoming <- conn: case <-ctx.Done(): @@ -122,16 +126,10 @@ func (l *listener) handleIncoming() { // Accept accepts a connection. func (l *listener) Accept() (transport.Conn, error) { - for { - select { - case l.ticket <- struct{}{}: - case c, ok := <-l.incoming: - if !ok { - return nil, l.err - } - return c, nil - } + if c, ok := <-l.incoming; ok { + return c, nil } + return nil, l.err } func (l *listener) String() string { diff --git a/p2p/net/upgrader/threshold.go b/p2p/net/upgrader/threshold.go new file mode 100644 index 0000000000..091e35c075 --- /dev/null +++ b/p2p/net/upgrader/threshold.go @@ -0,0 +1,56 @@ +package stream + +import ( + "sync" +) + +func newThreshold(cutoff int) *threshold { + t := &threshold{ + threshold: cutoff, + } + t.cond.L = &t.mu + return t +} + +type threshold struct { + mu sync.Mutex + cond sync.Cond + + count int + threshold int +} + +func (t *threshold) Count() int { + t.mu.Lock() + defer t.mu.Unlock() + return t.count +} + +// Acquire increments the counter. It will not block. +func (t *threshold) Acquire() { + t.mu.Lock() + t.count++ + t.mu.Unlock() +} + +// Release decrements the counter. +func (t *threshold) Release() { + t.mu.Lock() + if t.count == 0 { + panic("negative count") + } + if t.threshold == t.count { + t.cond.Broadcast() + } + t.count-- + t.mu.Unlock() +} + +// Wait waits for the counter to drop below the threshold +func (t *threshold) Wait() { + t.mu.Lock() + for t.count > t.threshold { + t.cond.Wait() + } + t.mu.Unlock() +} diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 3da10bf798..c494ea2374 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -19,6 +19,9 @@ import ( // without specifying a peer ID. var ErrNilPeer = errors.New("nil peer") +// AcceptQueueLength is the number of connections to fully setup before not accepting any new connections +var AcceptQueueLength = 16 + // Upgrader is a multistream upgrader that can upgrade an underlying connection // to a full transport connection (secure and multiplexed). type Upgrader struct { @@ -35,7 +38,7 @@ func (u *Upgrader) UpgradeListener(t transport.Transport, list manet.Listener) t Listener: list, upgrader: u, transport: t, - ticket: make(chan struct{}), + threshold: newThreshold(AcceptQueueLength), incoming: make(chan transport.Conn), cancel: cancel, ctx: ctx, From a950fa7dafb8b761489e32a2ae99f657815b38bf Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Mon, 22 Jan 2018 09:16:28 -0800 Subject: [PATCH 05/55] fix comment on why we don't need to wait on the context Also, add a context canceled check to the loop just in case (accept could, if buggy, return temporary errors after it's closed). --- p2p/net/upgrader/listener.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index 72d129ced1..c74772f90b 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -63,12 +63,7 @@ func (l *listener) handleIncoming() { }() var catcher tec.TempErrCatcher - for { - // no need to wait on the context. - // Close will drain the incoming channel which will cause the - // threshold to drop. - l.threshold.Wait() - + for l.ctx.Err() == nil { maconn, err := l.Listener.Accept() if err != nil { if catcher.IsTemporary(err) { @@ -121,6 +116,10 @@ func (l *listener) handleIncoming() { conn.Close() } }() + + // The go routine above calls Release when the context is + // canceled so there's no need to wait on it here. + l.threshold.Wait() } } From cec0d9ff0cae84a411593f68f6f74dbb39e6b269 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Mon, 22 Jan 2018 09:21:46 -0800 Subject: [PATCH 06/55] remove unused count function from threshold --- p2p/net/upgrader/threshold.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/p2p/net/upgrader/threshold.go b/p2p/net/upgrader/threshold.go index 091e35c075..8185e3a53e 100644 --- a/p2p/net/upgrader/threshold.go +++ b/p2p/net/upgrader/threshold.go @@ -20,12 +20,6 @@ type threshold struct { threshold int } -func (t *threshold) Count() int { - t.mu.Lock() - defer t.mu.Unlock() - return t.count -} - // Acquire increments the counter. It will not block. func (t *threshold) Acquire() { t.mu.Lock() From be4a5f410f36edf99d92151afa47a8e48f954c1d Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 23 Jan 2018 23:31:15 +1100 Subject: [PATCH 07/55] fix off-by-one error in the threshold --- p2p/net/upgrader/threshold.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/net/upgrader/threshold.go b/p2p/net/upgrader/threshold.go index 8185e3a53e..2c278bd398 100644 --- a/p2p/net/upgrader/threshold.go +++ b/p2p/net/upgrader/threshold.go @@ -43,7 +43,7 @@ func (t *threshold) Release() { // Wait waits for the counter to drop below the threshold func (t *threshold) Wait() { t.mu.Lock() - for t.count > t.threshold { + for t.count >= t.threshold { t.cond.Wait() } t.mu.Unlock() From 90789da3cb30fce78ebd3f692b5cc4defd7ece6e Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 23 Jan 2018 23:41:58 +1100 Subject: [PATCH 08/55] add tests for the listener --- .../go_libp2p_stream_transport_suite_test.go | 13 + p2p/net/upgrader/listener_test.go | 254 ++++++++++++++++++ 2 files changed, 267 insertions(+) create mode 100644 p2p/net/upgrader/go_libp2p_stream_transport_suite_test.go create mode 100644 p2p/net/upgrader/listener_test.go diff --git a/p2p/net/upgrader/go_libp2p_stream_transport_suite_test.go b/p2p/net/upgrader/go_libp2p_stream_transport_suite_test.go new file mode 100644 index 0000000000..1a40574419 --- /dev/null +++ b/p2p/net/upgrader/go_libp2p_stream_transport_suite_test.go @@ -0,0 +1,13 @@ +package stream_test + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + + "testing" +) + +func TestGoLibp2pStreamTransport(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "GoLibp2pStreamTransport Suite") +} diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go new file mode 100644 index 0000000000..c2eaf6c72b --- /dev/null +++ b/p2p/net/upgrader/listener_test.go @@ -0,0 +1,254 @@ +package stream_test + +import ( + "context" + "errors" + "net" + "sync" + "time" + + peer "github.com/libp2p/go-libp2p-peer" + st "github.com/libp2p/go-libp2p-stream-transport" + tpt "github.com/libp2p/go-libp2p-transport" + smux "github.com/libp2p/go-stream-muxer" + ss "github.com/libp2p/go-stream-security" + tcp "github.com/libp2p/go-tcp-transport" + ma "github.com/multiformats/go-multiaddr" + yamux "github.com/whyrusleeping/go-smux-yamux" + + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +// negotiatingMuxer sets up a new yamux connection +// It makes sure that this happens at the same time for client and server. +type negotiatingMuxer struct{} + +func (m *negotiatingMuxer) NewConn(c net.Conn, isServer bool) (smux.Conn, error) { + var err error + // run a fake muxer negotiation + if isServer { + _, err = c.Write([]byte("setup")) + } else { + _, err = c.Read(make([]byte, 5)) + } + if err != nil { + return nil, err + } + return yamux.DefaultTransport.NewConn(c, isServer) +} + +// blockingMuxer blocks the muxer negotiation until the contain chan is closed +type blockingMuxer struct { + unblock chan struct{} +} + +var _ smux.Transport = &blockingMuxer{} + +func newBlockingMuxer() *blockingMuxer { return &blockingMuxer{unblock: make(chan struct{})} } +func (m *blockingMuxer) NewConn(c net.Conn, isServer bool) (smux.Conn, error) { + <-m.unblock + return (&negotiatingMuxer{}).NewConn(c, isServer) +} +func (m *blockingMuxer) Unblock() { close(m.unblock) } + +// errorMuxer is a muxer that errors while setting up +type errorMuxer struct{} + +var _ smux.Transport = &errorMuxer{} + +func (m *errorMuxer) NewConn(c net.Conn, isServer bool) (smux.Conn, error) { + return nil, errors.New("mux error") +} + +var _ = Describe("Listener", func() { + var ( + defaultUpgrader = &st.Upgrader{ + Secure: ss.NewInsecureTransport(peer.ID(1)), + Muxer: &negotiatingMuxer{}, + } + ) + + testConn := func(clientConn, serverConn tpt.Conn) { + cstr, err := clientConn.OpenStream() + ExpectWithOffset(0, err).ToNot(HaveOccurred()) + _, err = cstr.Write([]byte("foobar")) + ExpectWithOffset(0, err).ToNot(HaveOccurred()) + sstr, err := serverConn.AcceptStream() + ExpectWithOffset(0, err).ToNot(HaveOccurred()) + b := make([]byte, 6) + _, err = sstr.Read(b) + ExpectWithOffset(0, err).ToNot(HaveOccurred()) + ExpectWithOffset(0, b).To(Equal([]byte("foobar"))) + } + + createListener := func(upgrader *st.Upgrader) tpt.Listener { + addr, err := ma.NewMultiaddr("/ip4/0.0.0.0/tcp/0") + ExpectWithOffset(0, err).ToNot(HaveOccurred()) + ln, err := tcp.NewTCPTransport(upgrader).Listen(addr) + ExpectWithOffset(0, err).ToNot(HaveOccurred()) + return ln + } + + BeforeEach(func() { + tpt.AcceptTimeout = time.Hour + }) + + It("accepts a single connection", func() { + ln := createListener(defaultUpgrader) + cconn, err := tcp.NewTCPTransport(defaultUpgrader).Dial(context.Background(), ln.Multiaddr(), peer.ID(1)) + Expect(err).ToNot(HaveOccurred()) + sconn, err := ln.Accept() + Expect(err).ToNot(HaveOccurred()) + testConn(cconn, sconn) + }) + + It("accepts multiple connections", func() { + ln := createListener(defaultUpgrader) + const num = 10 + for i := 0; i < 10; i++ { + cconn, err := tcp.NewTCPTransport(defaultUpgrader).Dial(context.Background(), ln.Multiaddr(), peer.ID(1)) + Expect(err).ToNot(HaveOccurred()) + sconn, err := ln.Accept() + Expect(err).ToNot(HaveOccurred()) + testConn(cconn, sconn) + } + }) + + It("closes connections if they are not accepted", func() { + const timeout = 200 * time.Millisecond + tpt.AcceptTimeout = timeout + ln := createListener(defaultUpgrader) + conn, err := tcp.NewTCPTransport(defaultUpgrader).Dial(context.Background(), ln.Multiaddr(), peer.ID(2)) + Expect(err).ToNot(HaveOccurred()) + done := make(chan struct{}) + go func() { + defer GinkgoRecover() + str, err := conn.OpenStream() + Expect(err).ToNot(HaveOccurred()) + // start a Read. It will block until the connection is closed + str.Read([]byte{0}) + close(done) + }() + Consistently(done, timeout/2).ShouldNot(BeClosed()) + Eventually(done, timeout).Should(BeClosed()) + }) + + It("doesn't accept connections that fail to setup", func() { + upgrader := &st.Upgrader{ + Secure: ss.NewInsecureTransport(peer.ID(1)), + Muxer: &errorMuxer{}, + } + ln := createListener(upgrader) + done := make(chan struct{}) + go func() { + defer GinkgoRecover() + _, _ = ln.Accept() + close(done) + }() + _, _ = tcp.NewTCPTransport(upgrader).Dial(context.Background(), ln.Multiaddr(), peer.ID(2)) + Consistently(done).ShouldNot(BeClosed()) + // make the goroutine return + ln.Close() + Eventually(done).Should(BeClosed()) + }) + + Context("concurrency", func() { + It("sets up connections concurrently", func() { + num := 3 * st.AcceptQueueLength + bm := newBlockingMuxer() + upgrader := &st.Upgrader{ + Secure: ss.NewInsecureTransport(peer.ID(1)), + Muxer: bm, + } + ln := createListener(upgrader) + accepted := make(chan tpt.Conn, num) + go func() { + defer GinkgoRecover() + for { + conn, err := ln.Accept() + if err != nil { + return + } + accepted <- conn + } + }() + var wg sync.WaitGroup + // start num dials, which all block while setting up the muxer + for i := 0; i < num; i++ { + wg.Add(1) + go func() { + defer GinkgoRecover() + _, err := tcp.NewTCPTransport(upgrader).Dial(context.Background(), ln.Multiaddr(), peer.ID(2)) + Expect(err).ToNot(HaveOccurred()) + wg.Done() + }() + } + // the dials are still blocked, so we shouldn't have any connection available yet + Consistently(accepted).Should(BeEmpty()) + bm.Unblock() // make all dials succeed + Eventually(accepted).Should(HaveLen(num)) + wg.Wait() + }) + + It("stops setting up when the more than AcceptQueueLength connections are waiting to get accepted", func() { + ln := createListener(defaultUpgrader) + // setup AcceptQueueLength connections, but don't accept any of them + dialed := make(chan struct{}, 10*st.AcceptQueueLength) // used as a thread-safe counter + for i := 0; i < st.AcceptQueueLength; i++ { + go func() { + defer GinkgoRecover() + _, err := tcp.NewTCPTransport(defaultUpgrader).Dial(context.Background(), ln.Multiaddr(), peer.ID(2)) + Expect(err).ToNot(HaveOccurred()) + dialed <- struct{}{} + }() + } + Eventually(dialed).Should(HaveLen(st.AcceptQueueLength)) + // dial a new connection. This connection should not complete setup, since the queue is full + go func() { + defer GinkgoRecover() + _, err := tcp.NewTCPTransport(defaultUpgrader).Dial(context.Background(), ln.Multiaddr(), peer.ID(2)) + Expect(err).ToNot(HaveOccurred()) + dialed <- struct{}{} + }() + Consistently(dialed).Should(HaveLen(st.AcceptQueueLength)) + // accept a single connection. Now the new connection should be set up, and fill the queue again + _, err := ln.Accept() + Expect(err).ToNot(HaveOccurred()) + Eventually(dialed).Should(HaveLen(st.AcceptQueueLength + 1)) + }) + }) + + Context("closing", func() { + It("unblocks Accept when it is closed", func() { + ln := createListener(defaultUpgrader) + done := make(chan struct{}) + go func() { + defer GinkgoRecover() + _, err := ln.Accept() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("use of closed network connection")) + close(done) + }() + Consistently(done).ShouldNot(BeClosed()) + Expect(ln.Close()).To(Succeed()) + Eventually(done).Should(BeClosed()) + }) + + It("doesn't accept new connections when it is closed", func() { + ln := createListener(defaultUpgrader) + Expect(ln.Close()).To(Succeed()) + _, err := tcp.NewTCPTransport(defaultUpgrader).Dial(context.Background(), ln.Multiaddr(), peer.ID(1)) + Expect(err).To(HaveOccurred()) + }) + + It("closes incoming connections that have not yet been accepted", func() { + ln := createListener(defaultUpgrader) + conn, err := tcp.NewTCPTransport(defaultUpgrader).Dial(context.Background(), ln.Multiaddr(), peer.ID(2)) + Expect(conn.IsClosed()).To(BeFalse()) + Expect(err).ToNot(HaveOccurred()) + Expect(ln.Close()).To(Succeed()) + Eventually(conn.IsClosed).Should(BeTrue()) + }) + }) +}) From 516978c8494484ccf8588a5b9cc7da668f1462ea Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Tue, 23 Jan 2018 23:34:12 +1100 Subject: [PATCH 09/55] wait after accepting a new connection if the queue is full --- p2p/net/upgrader/listener.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index c74772f90b..18005593aa 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -74,6 +74,10 @@ func (l *listener) handleIncoming() { return } + // The go routine above calls Release when the context is + // canceled so there's no need to wait on it here. + l.threshold.Wait() + log.Debugf("listener %s got connection: %s <---> %s", l, maconn.LocalMultiaddr(), @@ -116,10 +120,6 @@ func (l *listener) handleIncoming() { conn.Close() } }() - - // The go routine above calls Release when the context is - // canceled so there's no need to wait on it here. - l.threshold.Wait() } } From 1317fb9f54a89e1dfadf813f8b2478672c24fc93 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 28 Jan 2018 17:56:38 -0800 Subject: [PATCH 10/55] complete rename --- p2p/net/upgrader/upgrader.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index c494ea2374..392d65da03 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -11,7 +11,7 @@ import ( transport "github.com/libp2p/go-libp2p-transport" filter "github.com/libp2p/go-maddr-filter" smux "github.com/libp2p/go-stream-muxer" - ss "github.com/libp2p/go-stream-security" + ss "github.com/libp2p/go-conn-security" manet "github.com/multiformats/go-multiaddr-net" ) From dc4b4aef0cc6a31d7bc60dee82128bbb19ad86f1 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 28 Jan 2018 18:57:37 -0800 Subject: [PATCH 11/55] fix tests for rename --- p2p/net/upgrader/listener_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index c2eaf6c72b..7be94667a0 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -8,10 +8,10 @@ import ( "time" peer "github.com/libp2p/go-libp2p-peer" - st "github.com/libp2p/go-libp2p-stream-transport" + st "github.com/libp2p/go-libp2p-transport-upgrader" tpt "github.com/libp2p/go-libp2p-transport" smux "github.com/libp2p/go-stream-muxer" - ss "github.com/libp2p/go-stream-security" + ss "github.com/libp2p/go-conn-security" tcp "github.com/libp2p/go-tcp-transport" ma "github.com/multiformats/go-multiaddr" yamux "github.com/whyrusleeping/go-smux-yamux" From 0db3c0aa8861d80e8e91d21401be2400e33b0e0d Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Thu, 15 Feb 2018 12:15:41 -0800 Subject: [PATCH 12/55] fix for interface move --- p2p/net/upgrader/conn.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/p2p/net/upgrader/conn.go b/p2p/net/upgrader/conn.go index 50d5a5560b..48e4814166 100644 --- a/p2p/net/upgrader/conn.go +++ b/p2p/net/upgrader/conn.go @@ -3,14 +3,15 @@ package stream import ( "fmt" + inet "github.com/libp2p/go-libp2p-net" transport "github.com/libp2p/go-libp2p-transport" smux "github.com/libp2p/go-stream-muxer" ) type transportConn struct { smux.Conn - transport.ConnMultiaddrs - transport.ConnSecurity + inet.ConnMultiaddrs + inet.ConnSecurity transport transport.Transport } From 605544869cb72af1d514e701e85e0030f3cbf6c4 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Thu, 15 Feb 2018 12:16:22 -0800 Subject: [PATCH 13/55] fix for moved insecure security transport --- p2p/net/upgrader/listener_test.go | 10 +++++----- p2p/net/upgrader/upgrader.go | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index 7be94667a0..45418053d7 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -7,11 +7,11 @@ import ( "sync" "time" + insecure "github.com/libp2p/go-conn-security/insecure" peer "github.com/libp2p/go-libp2p-peer" - st "github.com/libp2p/go-libp2p-transport-upgrader" tpt "github.com/libp2p/go-libp2p-transport" + st "github.com/libp2p/go-libp2p-transport-upgrader" smux "github.com/libp2p/go-stream-muxer" - ss "github.com/libp2p/go-conn-security" tcp "github.com/libp2p/go-tcp-transport" ma "github.com/multiformats/go-multiaddr" yamux "github.com/whyrusleeping/go-smux-yamux" @@ -64,7 +64,7 @@ func (m *errorMuxer) NewConn(c net.Conn, isServer bool) (smux.Conn, error) { var _ = Describe("Listener", func() { var ( defaultUpgrader = &st.Upgrader{ - Secure: ss.NewInsecureTransport(peer.ID(1)), + Secure: insecure.New(peer.ID(1)), Muxer: &negotiatingMuxer{}, } ) @@ -136,7 +136,7 @@ var _ = Describe("Listener", func() { It("doesn't accept connections that fail to setup", func() { upgrader := &st.Upgrader{ - Secure: ss.NewInsecureTransport(peer.ID(1)), + Secure: insecure.New(peer.ID(1)), Muxer: &errorMuxer{}, } ln := createListener(upgrader) @@ -158,7 +158,7 @@ var _ = Describe("Listener", func() { num := 3 * st.AcceptQueueLength bm := newBlockingMuxer() upgrader := &st.Upgrader{ - Secure: ss.NewInsecureTransport(peer.ID(1)), + Secure: insecure.New(peer.ID(1)), Muxer: bm, } ln := createListener(upgrader) diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 392d65da03..7bc8fd7675 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -6,12 +6,12 @@ import ( "fmt" "net" + ss "github.com/libp2p/go-conn-security" pnet "github.com/libp2p/go-libp2p-interface-pnet" peer "github.com/libp2p/go-libp2p-peer" transport "github.com/libp2p/go-libp2p-transport" filter "github.com/libp2p/go-maddr-filter" smux "github.com/libp2p/go-stream-muxer" - ss "github.com/libp2p/go-conn-security" manet "github.com/multiformats/go-multiaddr-net" ) From 6fbbb671e114db456181288f89bac9fc37d96138 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Thu, 15 Feb 2018 19:57:41 -0800 Subject: [PATCH 14/55] check if connection is closed before returning it from Accept It could have been sitting around for a while. --- p2p/net/upgrader/listener.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index 18005593aa..1a4ddd57b8 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -125,8 +125,11 @@ func (l *listener) handleIncoming() { // Accept accepts a connection. func (l *listener) Accept() (transport.Conn, error) { - if c, ok := <-l.incoming; ok { - return c, nil + for c := range l.incoming { + // Could have been sitting there for a while. + if !c.IsClosed() { + return c, nil + } } return nil, l.err } From 07e87ba7ae4cc75f7497ce6e68a3955068721602 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Thu, 8 Mar 2018 21:29:16 -0800 Subject: [PATCH 15/55] ensure we force use of the libp2p protector if ForcePrivateNetwork is set --- p2p/net/upgrader/upgrader.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 7bc8fd7675..bfbfa7d017 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -77,6 +77,10 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma return nil, err } conn = pconn + } else if pnet.ForcePrivateNetwork { + log.Error("tried to dial with no Private Network Protector but usage" + + " of Private Networks is forced by the enviroment") + return nil, pnet.ErrNotInPrivateNetwork } sconn, err := u.setupSecurity(ctx, conn, p) if err != nil { From 32b0643327821ba8b63adf9766808a8fb5865d5d Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 9 Mar 2018 10:44:29 -0800 Subject: [PATCH 16/55] better document handleIncoming --- p2p/net/upgrader/listener.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index 1a4ddd57b8..fa26825fd5 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -49,6 +49,15 @@ func (l *listener) Close() error { return err } +// handles inbound connections. +// +// This function does a few interesting things that should be noted: +// +// 1. It logs and discards temporary/transient errors (errors with a Temporary() +// function that returns true). +// 2. It stops accepting new connections once AcceptQueueLength connections have +// been fully negotiated but not accepted. This gives us a basic backpressure +// mechanism while still allowing us to negotiate connections in parallel. func (l *listener) handleIncoming() { var wg sync.WaitGroup defer func() { @@ -66,6 +75,7 @@ func (l *listener) handleIncoming() { for l.ctx.Err() == nil { maconn, err := l.Listener.Accept() if err != nil { + // Note: function may pause the accept loop. if catcher.IsTemporary(err) { log.Infof("temporary accept error: %s", err) continue @@ -74,7 +84,7 @@ func (l *listener) handleIncoming() { return } - // The go routine above calls Release when the context is + // The go routine below calls Release when the context is // canceled so there's no need to wait on it here. l.threshold.Wait() @@ -103,6 +113,11 @@ func (l *listener) handleIncoming() { log.Debugf("listener %s accepted connection: %s", l, conn) + // This records the fact that the connection has been + // setup and is waiting to be accepted. This call + // *never* blocks, even if we go over the threshold. It + // simply ensures that calls to Wait block while we're + // over the threshold. l.threshold.Acquire() defer l.threshold.Release() From dbf3f835e261a2ad06100d0ca45bbdb78b809fa9 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 9 Mar 2018 16:46:51 -0800 Subject: [PATCH 17/55] remove dependency on the tcp transport fixes #7 --- p2p/net/upgrader/listener_test.go | 32 +++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index 45418053d7..e15ca0db6d 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -12,8 +12,8 @@ import ( tpt "github.com/libp2p/go-libp2p-transport" st "github.com/libp2p/go-libp2p-transport-upgrader" smux "github.com/libp2p/go-stream-muxer" - tcp "github.com/libp2p/go-tcp-transport" ma "github.com/multiformats/go-multiaddr" + manet "github.com/multiformats/go-multiaddr-net" yamux "github.com/whyrusleeping/go-smux-yamux" . "github.com/onsi/ginkgo" @@ -85,9 +85,17 @@ var _ = Describe("Listener", func() { createListener := func(upgrader *st.Upgrader) tpt.Listener { addr, err := ma.NewMultiaddr("/ip4/0.0.0.0/tcp/0") ExpectWithOffset(0, err).ToNot(HaveOccurred()) - ln, err := tcp.NewTCPTransport(upgrader).Listen(addr) + ln, err := manet.Listen(addr) ExpectWithOffset(0, err).ToNot(HaveOccurred()) - return ln + return upgrader.UpgradeListener(nil, ln) + } + + dial := func(upgrader *st.Upgrader, raddr ma.Multiaddr, p peer.ID) (tpt.Conn, error) { + macon, err := manet.Dial(raddr) + if err != nil { + return nil, err + } + return upgrader.UpgradeOutbound(context.Background(), nil, macon, p) } BeforeEach(func() { @@ -96,7 +104,7 @@ var _ = Describe("Listener", func() { It("accepts a single connection", func() { ln := createListener(defaultUpgrader) - cconn, err := tcp.NewTCPTransport(defaultUpgrader).Dial(context.Background(), ln.Multiaddr(), peer.ID(1)) + cconn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(1)) Expect(err).ToNot(HaveOccurred()) sconn, err := ln.Accept() Expect(err).ToNot(HaveOccurred()) @@ -107,7 +115,7 @@ var _ = Describe("Listener", func() { ln := createListener(defaultUpgrader) const num = 10 for i := 0; i < 10; i++ { - cconn, err := tcp.NewTCPTransport(defaultUpgrader).Dial(context.Background(), ln.Multiaddr(), peer.ID(1)) + cconn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(1)) Expect(err).ToNot(HaveOccurred()) sconn, err := ln.Accept() Expect(err).ToNot(HaveOccurred()) @@ -119,7 +127,7 @@ var _ = Describe("Listener", func() { const timeout = 200 * time.Millisecond tpt.AcceptTimeout = timeout ln := createListener(defaultUpgrader) - conn, err := tcp.NewTCPTransport(defaultUpgrader).Dial(context.Background(), ln.Multiaddr(), peer.ID(2)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) Expect(err).ToNot(HaveOccurred()) done := make(chan struct{}) go func() { @@ -146,7 +154,7 @@ var _ = Describe("Listener", func() { _, _ = ln.Accept() close(done) }() - _, _ = tcp.NewTCPTransport(upgrader).Dial(context.Background(), ln.Multiaddr(), peer.ID(2)) + _, _ = dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) Consistently(done).ShouldNot(BeClosed()) // make the goroutine return ln.Close() @@ -179,7 +187,7 @@ var _ = Describe("Listener", func() { wg.Add(1) go func() { defer GinkgoRecover() - _, err := tcp.NewTCPTransport(upgrader).Dial(context.Background(), ln.Multiaddr(), peer.ID(2)) + _, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) Expect(err).ToNot(HaveOccurred()) wg.Done() }() @@ -198,7 +206,7 @@ var _ = Describe("Listener", func() { for i := 0; i < st.AcceptQueueLength; i++ { go func() { defer GinkgoRecover() - _, err := tcp.NewTCPTransport(defaultUpgrader).Dial(context.Background(), ln.Multiaddr(), peer.ID(2)) + _, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) Expect(err).ToNot(HaveOccurred()) dialed <- struct{}{} }() @@ -207,7 +215,7 @@ var _ = Describe("Listener", func() { // dial a new connection. This connection should not complete setup, since the queue is full go func() { defer GinkgoRecover() - _, err := tcp.NewTCPTransport(defaultUpgrader).Dial(context.Background(), ln.Multiaddr(), peer.ID(2)) + _, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) Expect(err).ToNot(HaveOccurred()) dialed <- struct{}{} }() @@ -238,13 +246,13 @@ var _ = Describe("Listener", func() { It("doesn't accept new connections when it is closed", func() { ln := createListener(defaultUpgrader) Expect(ln.Close()).To(Succeed()) - _, err := tcp.NewTCPTransport(defaultUpgrader).Dial(context.Background(), ln.Multiaddr(), peer.ID(1)) + _, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(1)) Expect(err).To(HaveOccurred()) }) It("closes incoming connections that have not yet been accepted", func() { ln := createListener(defaultUpgrader) - conn, err := tcp.NewTCPTransport(defaultUpgrader).Dial(context.Background(), ln.Multiaddr(), peer.ID(2)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) Expect(conn.IsClosed()).To(BeFalse()) Expect(err).ToNot(HaveOccurred()) Expect(ln.Close()).To(Succeed()) From 66b1a470e5b36bc01ebf2ef496109df735e875c6 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 9 Mar 2018 16:48:23 -0800 Subject: [PATCH 18/55] use mplex for tests instead of yamux --- p2p/net/upgrader/listener_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index e15ca0db6d..b216c462e3 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -14,13 +14,13 @@ import ( smux "github.com/libp2p/go-stream-muxer" ma "github.com/multiformats/go-multiaddr" manet "github.com/multiformats/go-multiaddr-net" - yamux "github.com/whyrusleeping/go-smux-yamux" + mplex "github.com/whyrusleeping/go-smux-multiplex" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" ) -// negotiatingMuxer sets up a new yamux connection +// negotiatingMuxer sets up a new mplex connection // It makes sure that this happens at the same time for client and server. type negotiatingMuxer struct{} @@ -35,7 +35,7 @@ func (m *negotiatingMuxer) NewConn(c net.Conn, isServer bool) (smux.Conn, error) if err != nil { return nil, err } - return yamux.DefaultTransport.NewConn(c, isServer) + return mplex.DefaultTransport.NewConn(c, isServer) } // blockingMuxer blocks the muxer negotiation until the contain chan is closed From 8ceb85678fac58ed84786086f9d7182f0ae652cc Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Tue, 8 Jan 2019 16:05:55 -0800 Subject: [PATCH 19/55] annotate errors --- p2p/net/upgrader/upgrader.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index bfbfa7d017..1c901b27dc 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -74,7 +74,7 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma pconn, err := u.Protector.Protect(conn) if err != nil { conn.Close() - return nil, err + return nil, fmt.Errorf("failed to setup private network protector: %s", err) } conn = pconn } else if pnet.ForcePrivateNetwork { @@ -85,12 +85,12 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma sconn, err := u.setupSecurity(ctx, conn, p) if err != nil { conn.Close() - return nil, err + return nil, fmt.Errorf("failed to negotiate security protocol: %s", err) } smconn, err := u.setupMuxer(ctx, sconn, p) if err != nil { conn.Close() - return nil, err + return nil, fmt.Errorf("failed to negotiate security stream multiplexer: %s", err) } return &transportConn{ Conn: smconn, From e87a4e322b4b1c2e89fedf709d1748aec4cfb7ba Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Thu, 25 Apr 2019 19:31:56 -0700 Subject: [PATCH 20/55] improve correctness of closing connections on failure may be related to https://github.com/ipfs/go-ipfs/issues/6197 (but I can't find one) --- p2p/net/upgrader/upgrader.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 1c901b27dc..eacbb3dee8 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -89,7 +89,7 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma } smconn, err := u.setupMuxer(ctx, sconn, p) if err != nil { - conn.Close() + sconn.Close() return nil, fmt.Errorf("failed to negotiate security stream multiplexer: %s", err) } return &transportConn{ @@ -122,6 +122,10 @@ func (u *Upgrader) setupMuxer(ctx context.Context, conn net.Conn, p peer.ID) (sm case <-done: return smconn, err case <-ctx.Done(): + // interrupt this process + conn.Close() + // wait to finish + <-done return nil, ctx.Err() } } From f418d2c15b38464a805da3514faab9ccab5c3c7b Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 26 Apr 2019 01:32:43 -0700 Subject: [PATCH 21/55] test: cleanup after tests Otherwise, setting global variables races with running goroutines. --- p2p/net/upgrader/listener_test.go | 74 +++++++++++++++++++++++-------- 1 file changed, 56 insertions(+), 18 deletions(-) diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index b216c462e3..969b98d238 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -104,6 +104,7 @@ var _ = Describe("Listener", func() { It("accepts a single connection", func() { ln := createListener(defaultUpgrader) + defer ln.Close() cconn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(1)) Expect(err).ToNot(HaveOccurred()) sconn, err := ln.Accept() @@ -113,6 +114,7 @@ var _ = Describe("Listener", func() { It("accepts multiple connections", func() { ln := createListener(defaultUpgrader) + defer ln.Close() const num = 10 for i := 0; i < 10; i++ { cconn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(1)) @@ -127,11 +129,15 @@ var _ = Describe("Listener", func() { const timeout = 200 * time.Millisecond tpt.AcceptTimeout = timeout ln := createListener(defaultUpgrader) + defer ln.Close() conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) - Expect(err).ToNot(HaveOccurred()) + if !Expect(err).ToNot(HaveOccurred()) { + return + } done := make(chan struct{}) go func() { defer GinkgoRecover() + defer conn.Close() str, err := conn.OpenStream() Expect(err).ToNot(HaveOccurred()) // start a Read. It will block until the connection is closed @@ -151,10 +157,16 @@ var _ = Describe("Listener", func() { done := make(chan struct{}) go func() { defer GinkgoRecover() - _, _ = ln.Accept() + conn, err := ln.Accept() + if !Expect(err).To(HaveOccurred()) { + conn.Close() + } close(done) }() - _, _ = dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + if !Expect(err).To(HaveOccurred()) { + conn.Close() + } Consistently(done).ShouldNot(BeClosed()) // make the goroutine return ln.Close() @@ -178,6 +190,7 @@ var _ = Describe("Listener", func() { if err != nil { return } + conn.Close() accepted <- conn } }() @@ -187,8 +200,14 @@ var _ = Describe("Listener", func() { wg.Add(1) go func() { defer GinkgoRecover() - _, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) - Expect(err).ToNot(HaveOccurred()) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + if Expect(err).ToNot(HaveOccurred()) { + stream, err := conn.AcceptStream() // wait for conn to be accepted. + if !Expect(err).To(HaveOccurred()) { + stream.Close() + } + conn.Close() + } wg.Done() }() } @@ -201,29 +220,40 @@ var _ = Describe("Listener", func() { It("stops setting up when the more than AcceptQueueLength connections are waiting to get accepted", func() { ln := createListener(defaultUpgrader) + defer ln.Close() + // setup AcceptQueueLength connections, but don't accept any of them - dialed := make(chan struct{}, 10*st.AcceptQueueLength) // used as a thread-safe counter + dialed := make(chan tpt.Conn, 10*st.AcceptQueueLength) // used as a thread-safe counter for i := 0; i < st.AcceptQueueLength; i++ { go func() { defer GinkgoRecover() - _, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) Expect(err).ToNot(HaveOccurred()) - dialed <- struct{}{} + dialed <- conn }() } Eventually(dialed).Should(HaveLen(st.AcceptQueueLength)) // dial a new connection. This connection should not complete setup, since the queue is full go func() { defer GinkgoRecover() - _, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) Expect(err).ToNot(HaveOccurred()) - dialed <- struct{}{} + dialed <- conn }() Consistently(dialed).Should(HaveLen(st.AcceptQueueLength)) // accept a single connection. Now the new connection should be set up, and fill the queue again - _, err := ln.Accept() - Expect(err).ToNot(HaveOccurred()) + conn, err := ln.Accept() + if Expect(err).ToNot(HaveOccurred()) { + conn.Close() + } Eventually(dialed).Should(HaveLen(st.AcceptQueueLength + 1)) + + // Cleanup + for i := 0; i < st.AcceptQueueLength+1; i++ { + if c := <-dialed; c != nil { + c.Close() + } + } }) }) @@ -233,9 +263,12 @@ var _ = Describe("Listener", func() { done := make(chan struct{}) go func() { defer GinkgoRecover() - _, err := ln.Accept() - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("use of closed network connection")) + conn, err := ln.Accept() + if Expect(err).To(HaveOccurred()) { + Expect(err.Error()).To(ContainSubstring("use of closed network connection")) + } else { + conn.Close() + } close(done) }() Consistently(done).ShouldNot(BeClosed()) @@ -246,15 +279,20 @@ var _ = Describe("Listener", func() { It("doesn't accept new connections when it is closed", func() { ln := createListener(defaultUpgrader) Expect(ln.Close()).To(Succeed()) - _, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(1)) - Expect(err).To(HaveOccurred()) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(1)) + if !Expect(err).To(HaveOccurred()) { + conn.Close() + } }) It("closes incoming connections that have not yet been accepted", func() { ln := createListener(defaultUpgrader) conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + if !Expect(err).ToNot(HaveOccurred()) { + ln.Close() + return + } Expect(conn.IsClosed()).To(BeFalse()) - Expect(err).ToNot(HaveOccurred()) Expect(ln.Close()).To(Succeed()) Eventually(conn.IsClosed).Should(BeTrue()) }) From 67b7b275cfad17c45434329289ca13d56014b05b Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Tue, 21 May 2019 18:56:06 -0700 Subject: [PATCH 22/55] dep: import go-libp2p-mplex into the libp2p org --- p2p/net/upgrader/listener_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index 969b98d238..6d98f0850e 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -14,7 +14,7 @@ import ( smux "github.com/libp2p/go-stream-muxer" ma "github.com/multiformats/go-multiaddr" manet "github.com/multiformats/go-multiaddr-net" - mplex "github.com/whyrusleeping/go-smux-multiplex" + mplex "github.com/libp2p/go-libp2p-mplex" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" From 57d79cc31d23cc5aed7e32493d5f234300e1c260 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 24 May 2019 15:33:02 -0700 Subject: [PATCH 23/55] Consolidate abstractions and core types into go-libp2p-core (#28) --- p2p/net/upgrader/conn.go | 12 +++---- p2p/net/upgrader/listener.go | 11 ++----- p2p/net/upgrader/listener_test.go | 52 +++++++++++++++---------------- p2p/net/upgrader/upgrader.go | 31 +++++++++--------- 4 files changed, 51 insertions(+), 55 deletions(-) diff --git a/p2p/net/upgrader/conn.go b/p2p/net/upgrader/conn.go index 48e4814166..4551a0f915 100644 --- a/p2p/net/upgrader/conn.go +++ b/p2p/net/upgrader/conn.go @@ -3,15 +3,15 @@ package stream import ( "fmt" - inet "github.com/libp2p/go-libp2p-net" - transport "github.com/libp2p/go-libp2p-transport" - smux "github.com/libp2p/go-stream-muxer" + mux "github.com/libp2p/go-libp2p-core/mux" + network "github.com/libp2p/go-libp2p-core/network" + transport "github.com/libp2p/go-libp2p-core/transport" ) type transportConn struct { - smux.Conn - inet.ConnMultiaddrs - inet.ConnSecurity + mux.MuxedConn + network.ConnMultiaddrs + network.ConnSecurity transport transport.Transport } diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index fa26825fd5..dc45325966 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -7,24 +7,19 @@ import ( logging "github.com/ipfs/go-log" tec "github.com/jbenet/go-temp-err-catcher" - transport "github.com/libp2p/go-libp2p-transport" + transport "github.com/libp2p/go-libp2p-core/transport" manet "github.com/multiformats/go-multiaddr-net" ) var log = logging.Logger("stream-upgrader") -type connErr struct { - conn transport.Conn - err error -} - type listener struct { manet.Listener transport transport.Transport upgrader *Upgrader - incoming chan transport.Conn + incoming chan transport.CapableConn err error // Used for backpressure @@ -139,7 +134,7 @@ func (l *listener) handleIncoming() { } // Accept accepts a connection. -func (l *listener) Accept() (transport.Conn, error) { +func (l *listener) Accept() (transport.CapableConn, error) { for c := range l.incoming { // Could have been sitting there for a while. if !c.IsClosed() { diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index 6d98f0850e..f3d89f4b3f 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -7,14 +7,14 @@ import ( "sync" "time" - insecure "github.com/libp2p/go-conn-security/insecure" - peer "github.com/libp2p/go-libp2p-peer" - tpt "github.com/libp2p/go-libp2p-transport" + core "github.com/libp2p/go-libp2p-core" + mux "github.com/libp2p/go-libp2p-core/mux" + insecure "github.com/libp2p/go-libp2p-core/sec/insecure" + tpt "github.com/libp2p/go-libp2p-core/transport" + mplex "github.com/libp2p/go-libp2p-mplex" st "github.com/libp2p/go-libp2p-transport-upgrader" - smux "github.com/libp2p/go-stream-muxer" ma "github.com/multiformats/go-multiaddr" manet "github.com/multiformats/go-multiaddr-net" - mplex "github.com/libp2p/go-libp2p-mplex" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -24,7 +24,7 @@ import ( // It makes sure that this happens at the same time for client and server. type negotiatingMuxer struct{} -func (m *negotiatingMuxer) NewConn(c net.Conn, isServer bool) (smux.Conn, error) { +func (m *negotiatingMuxer) NewConn(c net.Conn, isServer bool) (mux.MuxedConn, error) { var err error // run a fake muxer negotiation if isServer { @@ -43,10 +43,10 @@ type blockingMuxer struct { unblock chan struct{} } -var _ smux.Transport = &blockingMuxer{} +var _ mux.Multiplexer = &blockingMuxer{} func newBlockingMuxer() *blockingMuxer { return &blockingMuxer{unblock: make(chan struct{})} } -func (m *blockingMuxer) NewConn(c net.Conn, isServer bool) (smux.Conn, error) { +func (m *blockingMuxer) NewConn(c net.Conn, isServer bool) (mux.MuxedConn, error) { <-m.unblock return (&negotiatingMuxer{}).NewConn(c, isServer) } @@ -55,21 +55,21 @@ func (m *blockingMuxer) Unblock() { close(m.unblock) } // errorMuxer is a muxer that errors while setting up type errorMuxer struct{} -var _ smux.Transport = &errorMuxer{} +var _ mux.Multiplexer = &errorMuxer{} -func (m *errorMuxer) NewConn(c net.Conn, isServer bool) (smux.Conn, error) { +func (m *errorMuxer) NewConn(c net.Conn, isServer bool) (mux.MuxedConn, error) { return nil, errors.New("mux error") } var _ = Describe("Listener", func() { var ( defaultUpgrader = &st.Upgrader{ - Secure: insecure.New(peer.ID(1)), + Secure: insecure.New(core.PeerID(1)), Muxer: &negotiatingMuxer{}, } ) - testConn := func(clientConn, serverConn tpt.Conn) { + testConn := func(clientConn, serverConn tpt.CapableConn) { cstr, err := clientConn.OpenStream() ExpectWithOffset(0, err).ToNot(HaveOccurred()) _, err = cstr.Write([]byte("foobar")) @@ -90,7 +90,7 @@ var _ = Describe("Listener", func() { return upgrader.UpgradeListener(nil, ln) } - dial := func(upgrader *st.Upgrader, raddr ma.Multiaddr, p peer.ID) (tpt.Conn, error) { + dial := func(upgrader *st.Upgrader, raddr ma.Multiaddr, p core.PeerID) (tpt.CapableConn, error) { macon, err := manet.Dial(raddr) if err != nil { return nil, err @@ -105,7 +105,7 @@ var _ = Describe("Listener", func() { It("accepts a single connection", func() { ln := createListener(defaultUpgrader) defer ln.Close() - cconn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(1)) + cconn, err := dial(defaultUpgrader, ln.Multiaddr(), core.PeerID(1)) Expect(err).ToNot(HaveOccurred()) sconn, err := ln.Accept() Expect(err).ToNot(HaveOccurred()) @@ -117,7 +117,7 @@ var _ = Describe("Listener", func() { defer ln.Close() const num = 10 for i := 0; i < 10; i++ { - cconn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(1)) + cconn, err := dial(defaultUpgrader, ln.Multiaddr(), core.PeerID(1)) Expect(err).ToNot(HaveOccurred()) sconn, err := ln.Accept() Expect(err).ToNot(HaveOccurred()) @@ -130,7 +130,7 @@ var _ = Describe("Listener", func() { tpt.AcceptTimeout = timeout ln := createListener(defaultUpgrader) defer ln.Close() - conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), core.PeerID(2)) if !Expect(err).ToNot(HaveOccurred()) { return } @@ -150,7 +150,7 @@ var _ = Describe("Listener", func() { It("doesn't accept connections that fail to setup", func() { upgrader := &st.Upgrader{ - Secure: insecure.New(peer.ID(1)), + Secure: insecure.New(core.PeerID(1)), Muxer: &errorMuxer{}, } ln := createListener(upgrader) @@ -163,7 +163,7 @@ var _ = Describe("Listener", func() { } close(done) }() - conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), core.PeerID(2)) if !Expect(err).To(HaveOccurred()) { conn.Close() } @@ -178,11 +178,11 @@ var _ = Describe("Listener", func() { num := 3 * st.AcceptQueueLength bm := newBlockingMuxer() upgrader := &st.Upgrader{ - Secure: insecure.New(peer.ID(1)), + Secure: insecure.New(core.PeerID(1)), Muxer: bm, } ln := createListener(upgrader) - accepted := make(chan tpt.Conn, num) + accepted := make(chan tpt.CapableConn, num) go func() { defer GinkgoRecover() for { @@ -200,7 +200,7 @@ var _ = Describe("Listener", func() { wg.Add(1) go func() { defer GinkgoRecover() - conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), core.PeerID(2)) if Expect(err).ToNot(HaveOccurred()) { stream, err := conn.AcceptStream() // wait for conn to be accepted. if !Expect(err).To(HaveOccurred()) { @@ -223,11 +223,11 @@ var _ = Describe("Listener", func() { defer ln.Close() // setup AcceptQueueLength connections, but don't accept any of them - dialed := make(chan tpt.Conn, 10*st.AcceptQueueLength) // used as a thread-safe counter + dialed := make(chan tpt.CapableConn, 10*st.AcceptQueueLength) // used as a thread-safe counter for i := 0; i < st.AcceptQueueLength; i++ { go func() { defer GinkgoRecover() - conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), core.PeerID(2)) Expect(err).ToNot(HaveOccurred()) dialed <- conn }() @@ -236,7 +236,7 @@ var _ = Describe("Listener", func() { // dial a new connection. This connection should not complete setup, since the queue is full go func() { defer GinkgoRecover() - conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), core.PeerID(2)) Expect(err).ToNot(HaveOccurred()) dialed <- conn }() @@ -279,7 +279,7 @@ var _ = Describe("Listener", func() { It("doesn't accept new connections when it is closed", func() { ln := createListener(defaultUpgrader) Expect(ln.Close()).To(Succeed()) - conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(1)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), core.PeerID(1)) if !Expect(err).To(HaveOccurred()) { conn.Close() } @@ -287,7 +287,7 @@ var _ = Describe("Listener", func() { It("closes incoming connections that have not yet been accepted", func() { ln := createListener(defaultUpgrader) - conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), core.PeerID(2)) if !Expect(err).ToNot(HaveOccurred()) { ln.Close() return diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index eacbb3dee8..d68ae8cd8d 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -6,12 +6,12 @@ import ( "fmt" "net" - ss "github.com/libp2p/go-conn-security" - pnet "github.com/libp2p/go-libp2p-interface-pnet" - peer "github.com/libp2p/go-libp2p-peer" - transport "github.com/libp2p/go-libp2p-transport" + core "github.com/libp2p/go-libp2p-core" + mux "github.com/libp2p/go-libp2p-core/mux" + pnet "github.com/libp2p/go-libp2p-core/pnet" + sec "github.com/libp2p/go-libp2p-core/sec" + transport "github.com/libp2p/go-libp2p-core/transport" filter "github.com/libp2p/go-maddr-filter" - smux "github.com/libp2p/go-stream-muxer" manet "github.com/multiformats/go-multiaddr-net" ) @@ -26,8 +26,8 @@ var AcceptQueueLength = 16 // to a full transport connection (secure and multiplexed). type Upgrader struct { Protector pnet.Protector - Secure ss.Transport - Muxer smux.Transport + Secure sec.SecureTransport + Muxer mux.Multiplexer Filters *filter.Filters } @@ -39,7 +39,7 @@ func (u *Upgrader) UpgradeListener(t transport.Transport, list manet.Listener) t upgrader: u, transport: t, threshold: newThreshold(AcceptQueueLength), - incoming: make(chan transport.Conn), + incoming: make(chan transport.CapableConn), cancel: cancel, ctx: ctx, } @@ -49,7 +49,7 @@ func (u *Upgrader) UpgradeListener(t transport.Transport, list manet.Listener) t // UpgradeOutbound upgrades the given outbound multiaddr-net connection into a // full libp2p-transport connection. -func (u *Upgrader) UpgradeOutbound(ctx context.Context, t transport.Transport, maconn manet.Conn, p peer.ID) (transport.Conn, error) { +func (u *Upgrader) UpgradeOutbound(ctx context.Context, t transport.Transport, maconn manet.Conn, p core.PeerID) (transport.CapableConn, error) { if p == "" { return nil, ErrNilPeer } @@ -58,11 +58,11 @@ func (u *Upgrader) UpgradeOutbound(ctx context.Context, t transport.Transport, m // UpgradeInbound upgrades the given inbound multiaddr-net connection into a // full libp2p-transport connection. -func (u *Upgrader) UpgradeInbound(ctx context.Context, t transport.Transport, maconn manet.Conn) (transport.Conn, error) { +func (u *Upgrader) UpgradeInbound(ctx context.Context, t transport.Transport, maconn manet.Conn) (transport.CapableConn, error) { return u.upgrade(ctx, t, maconn, "") } -func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn manet.Conn, p peer.ID) (transport.Conn, error) { +func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn manet.Conn, p core.PeerID) (transport.CapableConn, error) { if u.Filters != nil && u.Filters.AddrBlocked(maconn.RemoteMultiaddr()) { log.Debugf("blocked connection from %s", maconn.RemoteMultiaddr()) maconn.Close() @@ -78,6 +78,7 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma } conn = pconn } else if pnet.ForcePrivateNetwork { + conn.Close() log.Error("tried to dial with no Private Network Protector but usage" + " of Private Networks is forced by the enviroment") return nil, pnet.ErrNotInPrivateNetwork @@ -93,25 +94,25 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma return nil, fmt.Errorf("failed to negotiate security stream multiplexer: %s", err) } return &transportConn{ - Conn: smconn, + MuxedConn: smconn, ConnMultiaddrs: maconn, ConnSecurity: sconn, transport: t, }, nil } -func (u *Upgrader) setupSecurity(ctx context.Context, conn net.Conn, p peer.ID) (ss.Conn, error) { +func (u *Upgrader) setupSecurity(ctx context.Context, conn net.Conn, p core.PeerID) (sec.SecureConn, error) { if p == "" { return u.Secure.SecureInbound(ctx, conn) } return u.Secure.SecureOutbound(ctx, conn, p) } -func (u *Upgrader) setupMuxer(ctx context.Context, conn net.Conn, p peer.ID) (smux.Conn, error) { +func (u *Upgrader) setupMuxer(ctx context.Context, conn net.Conn, p core.PeerID) (mux.MuxedConn, error) { // TODO: The muxer should take a context. done := make(chan struct{}) - var smconn smux.Conn + var smconn mux.MuxedConn var err error go func() { defer close(done) From 9bcb09e28a5953d22098f0146e366349a15d93fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Kripalani?= Date: Sun, 26 May 2019 12:39:19 +0100 Subject: [PATCH 24/55] complete the migration to consolidated types. (#23) --- p2p/net/upgrader/conn.go | 6 +++--- p2p/net/upgrader/listener.go | 8 +++++++- p2p/net/upgrader/listener_test.go | 32 +++++++++++++++---------------- p2p/net/upgrader/upgrader.go | 20 +++++++++---------- 4 files changed, 36 insertions(+), 30 deletions(-) diff --git a/p2p/net/upgrader/conn.go b/p2p/net/upgrader/conn.go index 4551a0f915..2098b151e7 100644 --- a/p2p/net/upgrader/conn.go +++ b/p2p/net/upgrader/conn.go @@ -3,9 +3,9 @@ package stream import ( "fmt" - mux "github.com/libp2p/go-libp2p-core/mux" - network "github.com/libp2p/go-libp2p-core/network" - transport "github.com/libp2p/go-libp2p-core/transport" + "github.com/libp2p/go-libp2p-core/mux" + "github.com/libp2p/go-libp2p-core/network" + "github.com/libp2p/go-libp2p-core/transport" ) type transportConn struct { diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index dc45325966..f792c991be 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -5,14 +5,20 @@ import ( "fmt" "sync" + "github.com/libp2p/go-libp2p-core/transport" + logging "github.com/ipfs/go-log" tec "github.com/jbenet/go-temp-err-catcher" - transport "github.com/libp2p/go-libp2p-core/transport" manet "github.com/multiformats/go-multiaddr-net" ) var log = logging.Logger("stream-upgrader") +type connErr struct { + conn transport.CapableConn + err error +} + type listener struct { manet.Listener diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index f3d89f4b3f..c5d9719cc9 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -7,9 +7,9 @@ import ( "sync" "time" - core "github.com/libp2p/go-libp2p-core" - mux "github.com/libp2p/go-libp2p-core/mux" - insecure "github.com/libp2p/go-libp2p-core/sec/insecure" + "github.com/libp2p/go-libp2p-core/mux" + "github.com/libp2p/go-libp2p-core/peer" + "github.com/libp2p/go-libp2p-core/sec/insecure" tpt "github.com/libp2p/go-libp2p-core/transport" mplex "github.com/libp2p/go-libp2p-mplex" st "github.com/libp2p/go-libp2p-transport-upgrader" @@ -64,7 +64,7 @@ func (m *errorMuxer) NewConn(c net.Conn, isServer bool) (mux.MuxedConn, error) { var _ = Describe("Listener", func() { var ( defaultUpgrader = &st.Upgrader{ - Secure: insecure.New(core.PeerID(1)), + Secure: insecure.New(peer.ID(1)), Muxer: &negotiatingMuxer{}, } ) @@ -90,7 +90,7 @@ var _ = Describe("Listener", func() { return upgrader.UpgradeListener(nil, ln) } - dial := func(upgrader *st.Upgrader, raddr ma.Multiaddr, p core.PeerID) (tpt.CapableConn, error) { + dial := func(upgrader *st.Upgrader, raddr ma.Multiaddr, p peer.ID) (tpt.CapableConn, error) { macon, err := manet.Dial(raddr) if err != nil { return nil, err @@ -105,7 +105,7 @@ var _ = Describe("Listener", func() { It("accepts a single connection", func() { ln := createListener(defaultUpgrader) defer ln.Close() - cconn, err := dial(defaultUpgrader, ln.Multiaddr(), core.PeerID(1)) + cconn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(1)) Expect(err).ToNot(HaveOccurred()) sconn, err := ln.Accept() Expect(err).ToNot(HaveOccurred()) @@ -117,7 +117,7 @@ var _ = Describe("Listener", func() { defer ln.Close() const num = 10 for i := 0; i < 10; i++ { - cconn, err := dial(defaultUpgrader, ln.Multiaddr(), core.PeerID(1)) + cconn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(1)) Expect(err).ToNot(HaveOccurred()) sconn, err := ln.Accept() Expect(err).ToNot(HaveOccurred()) @@ -130,7 +130,7 @@ var _ = Describe("Listener", func() { tpt.AcceptTimeout = timeout ln := createListener(defaultUpgrader) defer ln.Close() - conn, err := dial(defaultUpgrader, ln.Multiaddr(), core.PeerID(2)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) if !Expect(err).ToNot(HaveOccurred()) { return } @@ -150,7 +150,7 @@ var _ = Describe("Listener", func() { It("doesn't accept connections that fail to setup", func() { upgrader := &st.Upgrader{ - Secure: insecure.New(core.PeerID(1)), + Secure: insecure.New(peer.ID(1)), Muxer: &errorMuxer{}, } ln := createListener(upgrader) @@ -163,7 +163,7 @@ var _ = Describe("Listener", func() { } close(done) }() - conn, err := dial(defaultUpgrader, ln.Multiaddr(), core.PeerID(2)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) if !Expect(err).To(HaveOccurred()) { conn.Close() } @@ -178,7 +178,7 @@ var _ = Describe("Listener", func() { num := 3 * st.AcceptQueueLength bm := newBlockingMuxer() upgrader := &st.Upgrader{ - Secure: insecure.New(core.PeerID(1)), + Secure: insecure.New(peer.ID(1)), Muxer: bm, } ln := createListener(upgrader) @@ -200,7 +200,7 @@ var _ = Describe("Listener", func() { wg.Add(1) go func() { defer GinkgoRecover() - conn, err := dial(defaultUpgrader, ln.Multiaddr(), core.PeerID(2)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) if Expect(err).ToNot(HaveOccurred()) { stream, err := conn.AcceptStream() // wait for conn to be accepted. if !Expect(err).To(HaveOccurred()) { @@ -227,7 +227,7 @@ var _ = Describe("Listener", func() { for i := 0; i < st.AcceptQueueLength; i++ { go func() { defer GinkgoRecover() - conn, err := dial(defaultUpgrader, ln.Multiaddr(), core.PeerID(2)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) Expect(err).ToNot(HaveOccurred()) dialed <- conn }() @@ -236,7 +236,7 @@ var _ = Describe("Listener", func() { // dial a new connection. This connection should not complete setup, since the queue is full go func() { defer GinkgoRecover() - conn, err := dial(defaultUpgrader, ln.Multiaddr(), core.PeerID(2)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) Expect(err).ToNot(HaveOccurred()) dialed <- conn }() @@ -279,7 +279,7 @@ var _ = Describe("Listener", func() { It("doesn't accept new connections when it is closed", func() { ln := createListener(defaultUpgrader) Expect(ln.Close()).To(Succeed()) - conn, err := dial(defaultUpgrader, ln.Multiaddr(), core.PeerID(1)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(1)) if !Expect(err).To(HaveOccurred()) { conn.Close() } @@ -287,7 +287,7 @@ var _ = Describe("Listener", func() { It("closes incoming connections that have not yet been accepted", func() { ln := createListener(defaultUpgrader) - conn, err := dial(defaultUpgrader, ln.Multiaddr(), core.PeerID(2)) + conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) if !Expect(err).ToNot(HaveOccurred()) { ln.Close() return diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index d68ae8cd8d..5e3854b0c1 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -6,11 +6,12 @@ import ( "fmt" "net" - core "github.com/libp2p/go-libp2p-core" - mux "github.com/libp2p/go-libp2p-core/mux" - pnet "github.com/libp2p/go-libp2p-core/pnet" - sec "github.com/libp2p/go-libp2p-core/sec" - transport "github.com/libp2p/go-libp2p-core/transport" + "github.com/libp2p/go-libp2p-core/mux" + "github.com/libp2p/go-libp2p-core/peer" + "github.com/libp2p/go-libp2p-core/pnet" + "github.com/libp2p/go-libp2p-core/sec" + "github.com/libp2p/go-libp2p-core/transport" + filter "github.com/libp2p/go-maddr-filter" manet "github.com/multiformats/go-multiaddr-net" ) @@ -49,7 +50,7 @@ func (u *Upgrader) UpgradeListener(t transport.Transport, list manet.Listener) t // UpgradeOutbound upgrades the given outbound multiaddr-net connection into a // full libp2p-transport connection. -func (u *Upgrader) UpgradeOutbound(ctx context.Context, t transport.Transport, maconn manet.Conn, p core.PeerID) (transport.CapableConn, error) { +func (u *Upgrader) UpgradeOutbound(ctx context.Context, t transport.Transport, maconn manet.Conn, p peer.ID) (transport.CapableConn, error) { if p == "" { return nil, ErrNilPeer } @@ -62,7 +63,7 @@ func (u *Upgrader) UpgradeInbound(ctx context.Context, t transport.Transport, ma return u.upgrade(ctx, t, maconn, "") } -func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn manet.Conn, p core.PeerID) (transport.CapableConn, error) { +func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn manet.Conn, p peer.ID) (transport.CapableConn, error) { if u.Filters != nil && u.Filters.AddrBlocked(maconn.RemoteMultiaddr()) { log.Debugf("blocked connection from %s", maconn.RemoteMultiaddr()) maconn.Close() @@ -78,7 +79,6 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma } conn = pconn } else if pnet.ForcePrivateNetwork { - conn.Close() log.Error("tried to dial with no Private Network Protector but usage" + " of Private Networks is forced by the enviroment") return nil, pnet.ErrNotInPrivateNetwork @@ -101,14 +101,14 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma }, nil } -func (u *Upgrader) setupSecurity(ctx context.Context, conn net.Conn, p core.PeerID) (sec.SecureConn, error) { +func (u *Upgrader) setupSecurity(ctx context.Context, conn net.Conn, p peer.ID) (sec.SecureConn, error) { if p == "" { return u.Secure.SecureInbound(ctx, conn) } return u.Secure.SecureOutbound(ctx, conn, p) } -func (u *Upgrader) setupMuxer(ctx context.Context, conn net.Conn, p core.PeerID) (mux.MuxedConn, error) { +func (u *Upgrader) setupMuxer(ctx context.Context, conn net.Conn, p peer.ID) (mux.MuxedConn, error) { // TODO: The muxer should take a context. done := make(chan struct{}) From f0810711a1696ec33979427f8f425e96d34540b2 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 6 Sep 2019 22:37:05 -0700 Subject: [PATCH 25/55] fix an incorrect error message "security stream multiplexer" is the worst sort of confusing. --- p2p/net/upgrader/upgrader.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 5e3854b0c1..69fb90bb22 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -91,7 +91,7 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma smconn, err := u.setupMuxer(ctx, sconn, p) if err != nil { sconn.Close() - return nil, fmt.Errorf("failed to negotiate security stream multiplexer: %s", err) + return nil, fmt.Errorf("failed to negotiate stream multiplexer: %s", err) } return &transportConn{ MuxedConn: smconn, From 1c3506645167e6885bc81630dfc8ce714d4275d3 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 20 Feb 2020 12:20:42 +0700 Subject: [PATCH 26/55] use the ipnet.PSK instead of the ipnet.Protector for private networks --- p2p/net/upgrader/upgrader.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 69fb90bb22..a464df5c48 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -8,9 +8,10 @@ import ( "github.com/libp2p/go-libp2p-core/mux" "github.com/libp2p/go-libp2p-core/peer" - "github.com/libp2p/go-libp2p-core/pnet" + ipnet "github.com/libp2p/go-libp2p-core/pnet" "github.com/libp2p/go-libp2p-core/sec" "github.com/libp2p/go-libp2p-core/transport" + "github.com/libp2p/go-libp2p-pnet" filter "github.com/libp2p/go-maddr-filter" manet "github.com/multiformats/go-multiaddr-net" @@ -26,10 +27,10 @@ var AcceptQueueLength = 16 // Upgrader is a multistream upgrader that can upgrade an underlying connection // to a full transport connection (secure and multiplexed). type Upgrader struct { - Protector pnet.Protector - Secure sec.SecureTransport - Muxer mux.Multiplexer - Filters *filter.Filters + PSK ipnet.PSK + Secure sec.SecureTransport + Muxer mux.Multiplexer + Filters *filter.Filters } // UpgradeListener upgrades the passed multiaddr-net listener into a full libp2p-transport listener. @@ -71,17 +72,17 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma } var conn net.Conn = maconn - if u.Protector != nil { - pconn, err := u.Protector.Protect(conn) + if u.PSK != nil { + pconn, err := pnet.NewProtectedConn(u.PSK, conn) if err != nil { conn.Close() return nil, fmt.Errorf("failed to setup private network protector: %s", err) } conn = pconn - } else if pnet.ForcePrivateNetwork { + } else if ipnet.ForcePrivateNetwork { log.Error("tried to dial with no Private Network Protector but usage" + " of Private Networks is forced by the enviroment") - return nil, pnet.ErrNotInPrivateNetwork + return nil, ipnet.ErrNotInPrivateNetwork } sconn, err := u.setupSecurity(ctx, conn, p) if err != nil { From b19703ddfe5bca58d59e983f053ef42d3435bf6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Kripalani?= Date: Wed, 13 May 2020 17:40:44 +0100 Subject: [PATCH 27/55] simplify tests by removing ginkgo/gomega. (#60) --- p2p/net/upgrader/listener_test.go | 531 +++++++++++++++++------------- p2p/net/upgrader/upgrader.go | 3 + 2 files changed, 312 insertions(+), 222 deletions(-) diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index c5d9719cc9..419877d13a 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -3,21 +3,24 @@ package stream_test import ( "context" "errors" + "io" "net" "sync" + "testing" "time" "github.com/libp2p/go-libp2p-core/mux" "github.com/libp2p/go-libp2p-core/peer" "github.com/libp2p/go-libp2p-core/sec/insecure" - tpt "github.com/libp2p/go-libp2p-core/transport" + "github.com/libp2p/go-libp2p-core/transport" + mplex "github.com/libp2p/go-libp2p-mplex" - st "github.com/libp2p/go-libp2p-transport-upgrader" ma "github.com/multiformats/go-multiaddr" manet "github.com/multiformats/go-multiaddr-net" - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" + st "github.com/libp2p/go-libp2p-transport-upgrader" + + "github.com/stretchr/testify/require" ) // negotiatingMuxer sets up a new mplex connection @@ -45,12 +48,18 @@ type blockingMuxer struct { var _ mux.Multiplexer = &blockingMuxer{} -func newBlockingMuxer() *blockingMuxer { return &blockingMuxer{unblock: make(chan struct{})} } +func newBlockingMuxer() *blockingMuxer { + return &blockingMuxer{unblock: make(chan struct{})} +} + func (m *blockingMuxer) NewConn(c net.Conn, isServer bool) (mux.MuxedConn, error) { <-m.unblock return (&negotiatingMuxer{}).NewConn(c, isServer) } -func (m *blockingMuxer) Unblock() { close(m.unblock) } + +func (m *blockingMuxer) Unblock() { + close(m.unblock) +} // errorMuxer is a muxer that errors while setting up type errorMuxer struct{} @@ -61,240 +70,318 @@ func (m *errorMuxer) NewConn(c net.Conn, isServer bool) (mux.MuxedConn, error) { return nil, errors.New("mux error") } -var _ = Describe("Listener", func() { - var ( - defaultUpgrader = &st.Upgrader{ - Secure: insecure.New(peer.ID(1)), - Muxer: &negotiatingMuxer{}, - } - ) +var ( + defaultUpgrader = &st.Upgrader{ + Secure: insecure.New(peer.ID(1)), + Muxer: &negotiatingMuxer{}, + } +) + +func init() { + transport.AcceptTimeout = 1 * time.Hour +} - testConn := func(clientConn, serverConn tpt.CapableConn) { - cstr, err := clientConn.OpenStream() - ExpectWithOffset(0, err).ToNot(HaveOccurred()) - _, err = cstr.Write([]byte("foobar")) - ExpectWithOffset(0, err).ToNot(HaveOccurred()) - sstr, err := serverConn.AcceptStream() - ExpectWithOffset(0, err).ToNot(HaveOccurred()) - b := make([]byte, 6) - _, err = sstr.Read(b) - ExpectWithOffset(0, err).ToNot(HaveOccurred()) - ExpectWithOffset(0, b).To(Equal([]byte("foobar"))) +func testConn(t *testing.T, clientConn, serverConn transport.CapableConn) { + t.Helper() + require := require.New(t) + + cstr, err := clientConn.OpenStream() + require.NoError(err) + + _, err = cstr.Write([]byte("foobar")) + require.NoError(err) + + sstr, err := serverConn.AcceptStream() + require.NoError(err) + + b := make([]byte, 6) + _, err = sstr.Read(b) + require.NoError(err) + require.Equal([]byte("foobar"), b) +} + +func dial(t *testing.T, upgrader *st.Upgrader, raddr ma.Multiaddr, p peer.ID) (transport.CapableConn, error) { + t.Helper() + + macon, err := manet.Dial(raddr) + if err != nil { + return nil, err } - createListener := func(upgrader *st.Upgrader) tpt.Listener { - addr, err := ma.NewMultiaddr("/ip4/0.0.0.0/tcp/0") - ExpectWithOffset(0, err).ToNot(HaveOccurred()) - ln, err := manet.Listen(addr) - ExpectWithOffset(0, err).ToNot(HaveOccurred()) - return upgrader.UpgradeListener(nil, ln) + return upgrader.UpgradeOutbound(context.Background(), nil, macon, p) +} + +func createListener(t *testing.T, upgrader *st.Upgrader) transport.Listener { + t.Helper() + require := require.New(t) + + addr, err := ma.NewMultiaddr("/ip4/0.0.0.0/tcp/0") + require.NoError(err) + + ln, err := manet.Listen(addr) + require.NoError(err) + + return upgrader.UpgradeListener(nil, ln) +} + +func TestAcceptSingleConn(t *testing.T) { + require := require.New(t) + + ln := createListener(t, defaultUpgrader) + defer ln.Close() + + cconn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(1)) + require.NoError(err) + + sconn, err := ln.Accept() + require.NoError(err) + + testConn(t, cconn, sconn) +} + +func TestAcceptMultipleConns(t *testing.T) { + require := require.New(t) + + ln := createListener(t, defaultUpgrader) + defer ln.Close() + + var toClose []io.Closer + defer func() { + for _, c := range toClose { + _ = c.Close() + } + }() + + for i := 0; i < 10; i++ { + cconn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(1)) + require.NoError(err) + toClose = append(toClose, cconn) + + sconn, err := ln.Accept() + require.NoError(err) + toClose = append(toClose, sconn) + + testConn(t, cconn, sconn) } +} - dial := func(upgrader *st.Upgrader, raddr ma.Multiaddr, p peer.ID) (tpt.CapableConn, error) { - macon, err := manet.Dial(raddr) +func TestConnectionsClosedIfNotAccepted(t *testing.T) { + require := require.New(t) + + const timeout = 200 * time.Millisecond + transport.AcceptTimeout = timeout + defer func() { transport.AcceptTimeout = 1 * time.Hour }() + + ln := createListener(t, defaultUpgrader) + defer ln.Close() + + conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + require.NoError(err) + + errCh := make(chan error) + go func() { + defer conn.Close() + str, err := conn.OpenStream() if err != nil { - return nil, err + errCh <- err + return } - return upgrader.UpgradeOutbound(context.Background(), nil, macon, p) + // start a Read. It will block until the connection is closed + _, _ = str.Read([]byte{0}) + errCh <- nil + }() + + time.Sleep(timeout / 2) + select { + case err := <-errCh: + t.Fatalf("connection closed earlier than expected. expected nothing on channel, got: %v", err) + default: } - BeforeEach(func() { - tpt.AcceptTimeout = time.Hour - }) + time.Sleep(timeout) + require.Nil(<-errCh) +} - It("accepts a single connection", func() { - ln := createListener(defaultUpgrader) - defer ln.Close() - cconn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(1)) - Expect(err).ToNot(HaveOccurred()) - sconn, err := ln.Accept() - Expect(err).ToNot(HaveOccurred()) - testConn(cconn, sconn) - }) - - It("accepts multiple connections", func() { - ln := createListener(defaultUpgrader) - defer ln.Close() - const num = 10 - for i := 0; i < 10; i++ { - cconn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(1)) - Expect(err).ToNot(HaveOccurred()) - sconn, err := ln.Accept() - Expect(err).ToNot(HaveOccurred()) - testConn(cconn, sconn) - } - }) - - It("closes connections if they are not accepted", func() { - const timeout = 200 * time.Millisecond - tpt.AcceptTimeout = timeout - ln := createListener(defaultUpgrader) - defer ln.Close() - conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) - if !Expect(err).ToNot(HaveOccurred()) { - return +func TestFailedUpgradeOnListen(t *testing.T) { + require := require.New(t) + + upgrader := &st.Upgrader{ + Secure: insecure.New(peer.ID(1)), + Muxer: &errorMuxer{}, + } + + ln := createListener(t, upgrader) + defer ln.Close() + + errCh := make(chan error) + go func() { + _, err := ln.Accept() + errCh <- err + }() + + _, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + require.Error(err) + + // close the listener. + ln.Close() + require.Error(<-errCh) +} + +func TestListenerClose(t *testing.T) { + require := require.New(t) + + ln := createListener(t, defaultUpgrader) + + errCh := make(chan error) + go func() { + _, err := ln.Accept() + errCh <- err + }() + + select { + case err := <-errCh: + t.Fatalf("connection closed earlier than expected. expected nothing on channel, got: %v", err) + case <-time.After(200 * time.Millisecond): + // nothing in 200ms. + } + + // unblocks Accept when it is closed. + err := ln.Close() + require.NoError(err) + err = <-errCh + require.Error(err) + require.Contains(err.Error(), "use of closed network connection") + + // doesn't accept new connections when it is closed + _, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(1)) + require.Error(err) +} + +func TestListenerCloseClosesQueued(t *testing.T) { + require := require.New(t) + + ln := createListener(t, defaultUpgrader) + + var conns []transport.CapableConn + for i := 0; i < 10; i++ { + conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(i)) + require.NoError(err) + conns = append(conns, conn) + } + + // wait for all the dials to happen. + time.Sleep(500 * time.Millisecond) + + // all the connections are opened. + for _, c := range conns { + require.False(c.IsClosed()) + } + + // expect that all the connections will be closed. + err := ln.Close() + require.NoError(err) + + // all the connections are closed. + require.Eventually(func() bool { + for _, c := range conns { + if !c.IsClosed() { + return false + } } - done := make(chan struct{}) - go func() { - defer GinkgoRecover() - defer conn.Close() - str, err := conn.OpenStream() - Expect(err).ToNot(HaveOccurred()) - // start a Read. It will block until the connection is closed - str.Read([]byte{0}) - close(done) - }() - Consistently(done, timeout/2).ShouldNot(BeClosed()) - Eventually(done, timeout).Should(BeClosed()) - }) + return true + }, 3*time.Second, 100*time.Millisecond) - It("doesn't accept connections that fail to setup", func() { - upgrader := &st.Upgrader{ + for _, c := range conns { + _ = c.Close() + } +} + +func TestConcurrentAccept(t *testing.T) { + var ( + require = require.New(t) + num = 3 * st.AcceptQueueLength + blockingMuxer = newBlockingMuxer() + upgrader = &st.Upgrader{ Secure: insecure.New(peer.ID(1)), - Muxer: &errorMuxer{}, + Muxer: blockingMuxer, } - ln := createListener(upgrader) - done := make(chan struct{}) - go func() { - defer GinkgoRecover() + ) + + ln := createListener(t, upgrader) + defer ln.Close() + + accepted := make(chan transport.CapableConn, num) + go func() { + for { conn, err := ln.Accept() - if !Expect(err).To(HaveOccurred()) { - conn.Close() + if err != nil { + return } - close(done) - }() - conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) - if !Expect(err).To(HaveOccurred()) { - conn.Close() + _ = conn.Close() + accepted <- conn } - Consistently(done).ShouldNot(BeClosed()) - // make the goroutine return - ln.Close() - Eventually(done).Should(BeClosed()) - }) - - Context("concurrency", func() { - It("sets up connections concurrently", func() { - num := 3 * st.AcceptQueueLength - bm := newBlockingMuxer() - upgrader := &st.Upgrader{ - Secure: insecure.New(peer.ID(1)), - Muxer: bm, - } - ln := createListener(upgrader) - accepted := make(chan tpt.CapableConn, num) - go func() { - defer GinkgoRecover() - for { - conn, err := ln.Accept() - if err != nil { - return - } - conn.Close() - accepted <- conn - } - }() - var wg sync.WaitGroup - // start num dials, which all block while setting up the muxer - for i := 0; i < num; i++ { - wg.Add(1) - go func() { - defer GinkgoRecover() - conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) - if Expect(err).ToNot(HaveOccurred()) { - stream, err := conn.AcceptStream() // wait for conn to be accepted. - if !Expect(err).To(HaveOccurred()) { - stream.Close() - } - conn.Close() - } - wg.Done() - }() - } - // the dials are still blocked, so we shouldn't have any connection available yet - Consistently(accepted).Should(BeEmpty()) - bm.Unblock() // make all dials succeed - Eventually(accepted).Should(HaveLen(num)) - wg.Wait() - }) - - It("stops setting up when the more than AcceptQueueLength connections are waiting to get accepted", func() { - ln := createListener(defaultUpgrader) - defer ln.Close() - - // setup AcceptQueueLength connections, but don't accept any of them - dialed := make(chan tpt.CapableConn, 10*st.AcceptQueueLength) // used as a thread-safe counter - for i := 0; i < st.AcceptQueueLength; i++ { - go func() { - defer GinkgoRecover() - conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) - Expect(err).ToNot(HaveOccurred()) - dialed <- conn - }() - } - Eventually(dialed).Should(HaveLen(st.AcceptQueueLength)) - // dial a new connection. This connection should not complete setup, since the queue is full - go func() { - defer GinkgoRecover() - conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) - Expect(err).ToNot(HaveOccurred()) - dialed <- conn - }() - Consistently(dialed).Should(HaveLen(st.AcceptQueueLength)) - // accept a single connection. Now the new connection should be set up, and fill the queue again - conn, err := ln.Accept() - if Expect(err).ToNot(HaveOccurred()) { - conn.Close() - } - Eventually(dialed).Should(HaveLen(st.AcceptQueueLength + 1)) + }() - // Cleanup - for i := 0; i < st.AcceptQueueLength+1; i++ { - if c := <-dialed; c != nil { - c.Close() - } - } - }) - }) - - Context("closing", func() { - It("unblocks Accept when it is closed", func() { - ln := createListener(defaultUpgrader) - done := make(chan struct{}) - go func() { - defer GinkgoRecover() - conn, err := ln.Accept() - if Expect(err).To(HaveOccurred()) { - Expect(err.Error()).To(ContainSubstring("use of closed network connection")) - } else { - conn.Close() - } - close(done) - }() - Consistently(done).ShouldNot(BeClosed()) - Expect(ln.Close()).To(Succeed()) - Eventually(done).Should(BeClosed()) - }) - - It("doesn't accept new connections when it is closed", func() { - ln := createListener(defaultUpgrader) - Expect(ln.Close()).To(Succeed()) - conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(1)) - if !Expect(err).To(HaveOccurred()) { - conn.Close() - } - }) + // start num dials, which all block while setting up the muxer + errCh := make(chan error, num) + var wg sync.WaitGroup + for i := 0; i < num; i++ { + wg.Add(1) + go func() { + defer wg.Done() - It("closes incoming connections that have not yet been accepted", func() { - ln := createListener(defaultUpgrader) - conn, err := dial(defaultUpgrader, ln.Multiaddr(), peer.ID(2)) - if !Expect(err).ToNot(HaveOccurred()) { - ln.Close() + conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + if err != nil { + errCh <- err return } - Expect(conn.IsClosed()).To(BeFalse()) - Expect(ln.Close()).To(Succeed()) - Eventually(conn.IsClosed).Should(BeTrue()) - }) - }) -}) + defer conn.Close() + + _, err = conn.AcceptStream() // wait for conn to be accepted. + errCh <- err + }() + } + + time.Sleep(200 * time.Millisecond) + // the dials are still blocked, so we shouldn't have any connection available yet + require.Empty(accepted) + blockingMuxer.Unblock() // make all dials succeed + require.Eventually(func() bool { return len(accepted) == num }, 3*time.Second, 100*time.Millisecond) + wg.Wait() +} + +func TestAcceptQueueBacklogged(t *testing.T) { + require := require.New(t) + + ln := createListener(t, defaultUpgrader) + defer ln.Close() + + // setup AcceptQueueLength connections, but don't accept any of them + errCh := make(chan error, st.AcceptQueueLength+1) + doDial := func() { + conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + errCh <- err + if conn != nil { + _ = conn.Close() + } + } + + for i := 0; i < st.AcceptQueueLength; i++ { + go doDial() + } + + require.Eventually(func() bool { return len(errCh) == st.AcceptQueueLength }, 2*time.Second, 100*time.Millisecond) + + // dial a new connection. This connection should not complete setup, since the queue is full + go doDial() + + time.Sleep(500 * time.Millisecond) + require.Len(errCh, st.AcceptQueueLength) + + // accept a single connection. Now the new connection should be set up, and fill the queue again + conn, err := ln.Accept() + require.NoError(err) + _ = conn.Close() + + require.Eventually(func() bool { return len(errCh) == st.AcceptQueueLength+1 }, 2*time.Second, 100*time.Millisecond) +} diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index a464df5c48..4b529b4440 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -84,16 +84,19 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma " of Private Networks is forced by the enviroment") return nil, ipnet.ErrNotInPrivateNetwork } + sconn, err := u.setupSecurity(ctx, conn, p) if err != nil { conn.Close() return nil, fmt.Errorf("failed to negotiate security protocol: %s", err) } + smconn, err := u.setupMuxer(ctx, sconn, p) if err != nil { sconn.Close() return nil, fmt.Errorf("failed to negotiate stream multiplexer: %s", err) } + return &transportConn{ MuxedConn: smconn, ConnMultiaddrs: maconn, From 59da2c11b4c7d6c38e1bdfe2d08003727973df98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Kripalani?= Date: Wed, 13 May 2020 19:22:23 +0100 Subject: [PATCH 28/55] remove leftover ginkgo test file. (#61) --- .../go_libp2p_stream_transport_suite_test.go | 13 ------------- 1 file changed, 13 deletions(-) delete mode 100644 p2p/net/upgrader/go_libp2p_stream_transport_suite_test.go diff --git a/p2p/net/upgrader/go_libp2p_stream_transport_suite_test.go b/p2p/net/upgrader/go_libp2p_stream_transport_suite_test.go deleted file mode 100644 index 1a40574419..0000000000 --- a/p2p/net/upgrader/go_libp2p_stream_transport_suite_test.go +++ /dev/null @@ -1,13 +0,0 @@ -package stream_test - -import ( - . "github.com/onsi/ginkgo" - . "github.com/onsi/gomega" - - "testing" -) - -func TestGoLibp2pStreamTransport(t *testing.T) { - RegisterFailHandler(Fail) - RunSpecs(t, "GoLibp2pStreamTransport Suite") -} From bf33d1ed49902736683f1f7290cc987dc55d165f Mon Sep 17 00:00:00 2001 From: Aarsh Shah Date: Fri, 15 May 2020 18:24:52 +0530 Subject: [PATCH 29/55] call the connection gater when accepting connections and after crypto handshake (#55) --- p2p/net/upgrader/gater_test.go | 60 +++++++++++++ p2p/net/upgrader/listener.go | 11 +++ p2p/net/upgrader/listener_test.go | 135 ++++++++++-------------------- p2p/net/upgrader/upgrader.go | 38 +++++---- p2p/net/upgrader/upgrader_test.go | 135 ++++++++++++++++++++++++++++++ 5 files changed, 274 insertions(+), 105 deletions(-) create mode 100644 p2p/net/upgrader/gater_test.go create mode 100644 p2p/net/upgrader/upgrader_test.go diff --git a/p2p/net/upgrader/gater_test.go b/p2p/net/upgrader/gater_test.go new file mode 100644 index 0000000000..858bd9e564 --- /dev/null +++ b/p2p/net/upgrader/gater_test.go @@ -0,0 +1,60 @@ +package stream_test + +import ( + "sync" + + "github.com/libp2p/go-libp2p-core/connmgr" + "github.com/libp2p/go-libp2p-core/control" + "github.com/libp2p/go-libp2p-core/network" + "github.com/libp2p/go-libp2p-core/peer" + + ma "github.com/multiformats/go-multiaddr" +) + +type testGater struct { + sync.Mutex + + blockAccept, blockSecured bool +} + +var _ connmgr.ConnectionGater = (*testGater)(nil) + +func (t *testGater) BlockAccept(block bool) { + t.Lock() + defer t.Unlock() + + t.blockAccept = block +} + +func (t *testGater) BlockSecured(block bool) { + t.Lock() + defer t.Unlock() + + t.blockSecured = block +} + +func (t *testGater) InterceptPeerDial(p peer.ID) (allow bool) { + panic("not implemented") +} + +func (t *testGater) InterceptAddrDial(id peer.ID, multiaddr ma.Multiaddr) (allow bool) { + panic("not implemented") +} + +func (t *testGater) InterceptAccept(multiaddrs network.ConnMultiaddrs) (allow bool) { + t.Lock() + defer t.Unlock() + + return !t.blockAccept +} + +func (t *testGater) InterceptSecured(direction network.Direction, id peer.ID, multiaddrs network.ConnMultiaddrs) (allow bool) { + t.Lock() + defer t.Unlock() + + return !t.blockSecured +} + +func (t *testGater) InterceptUpgraded(conn network.Conn) (allow bool, reason control.DisconnectReason) { + panic("not implemented") +} diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index f792c991be..2f3cd8e954 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -85,6 +85,17 @@ func (l *listener) handleIncoming() { return } + // gate the connection if applicable + if l.upgrader.ConnGater != nil && !l.upgrader.ConnGater.InterceptAccept(maconn) { + log.Debugf("gater blocked incoming connection on local addr %s from %s", + maconn.LocalMultiaddr(), maconn.RemoteMultiaddr()) + + if err := maconn.Close(); err != nil { + log.Warnf("failed to incoming connection rejected by gater; err: %s", err) + } + continue + } + // The go routine below calls Release when the context is // canceled so there's no need to wait on it here. l.threshold.Wait() diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index 419877d13a..a351aa47fe 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -1,20 +1,15 @@ package stream_test import ( - "context" - "errors" "io" - "net" "sync" "testing" "time" - "github.com/libp2p/go-libp2p-core/mux" "github.com/libp2p/go-libp2p-core/peer" "github.com/libp2p/go-libp2p-core/sec/insecure" "github.com/libp2p/go-libp2p-core/transport" - mplex "github.com/libp2p/go-libp2p-mplex" ma "github.com/multiformats/go-multiaddr" manet "github.com/multiformats/go-multiaddr-net" @@ -23,94 +18,10 @@ import ( "github.com/stretchr/testify/require" ) -// negotiatingMuxer sets up a new mplex connection -// It makes sure that this happens at the same time for client and server. -type negotiatingMuxer struct{} - -func (m *negotiatingMuxer) NewConn(c net.Conn, isServer bool) (mux.MuxedConn, error) { - var err error - // run a fake muxer negotiation - if isServer { - _, err = c.Write([]byte("setup")) - } else { - _, err = c.Read(make([]byte, 5)) - } - if err != nil { - return nil, err - } - return mplex.DefaultTransport.NewConn(c, isServer) -} - -// blockingMuxer blocks the muxer negotiation until the contain chan is closed -type blockingMuxer struct { - unblock chan struct{} -} - -var _ mux.Multiplexer = &blockingMuxer{} - -func newBlockingMuxer() *blockingMuxer { - return &blockingMuxer{unblock: make(chan struct{})} -} - -func (m *blockingMuxer) NewConn(c net.Conn, isServer bool) (mux.MuxedConn, error) { - <-m.unblock - return (&negotiatingMuxer{}).NewConn(c, isServer) -} - -func (m *blockingMuxer) Unblock() { - close(m.unblock) -} - -// errorMuxer is a muxer that errors while setting up -type errorMuxer struct{} - -var _ mux.Multiplexer = &errorMuxer{} - -func (m *errorMuxer) NewConn(c net.Conn, isServer bool) (mux.MuxedConn, error) { - return nil, errors.New("mux error") -} - -var ( - defaultUpgrader = &st.Upgrader{ - Secure: insecure.New(peer.ID(1)), - Muxer: &negotiatingMuxer{}, - } -) - func init() { transport.AcceptTimeout = 1 * time.Hour } -func testConn(t *testing.T, clientConn, serverConn transport.CapableConn) { - t.Helper() - require := require.New(t) - - cstr, err := clientConn.OpenStream() - require.NoError(err) - - _, err = cstr.Write([]byte("foobar")) - require.NoError(err) - - sstr, err := serverConn.AcceptStream() - require.NoError(err) - - b := make([]byte, 6) - _, err = sstr.Read(b) - require.NoError(err) - require.Equal([]byte("foobar"), b) -} - -func dial(t *testing.T, upgrader *st.Upgrader, raddr ma.Multiaddr, p peer.ID) (transport.CapableConn, error) { - t.Helper() - - macon, err := manet.Dial(raddr) - if err != nil { - return nil, err - } - - return upgrader.UpgradeOutbound(context.Background(), nil, macon, p) -} - func createListener(t *testing.T, upgrader *st.Upgrader) transport.Listener { t.Helper() require := require.New(t) @@ -385,3 +296,49 @@ func TestAcceptQueueBacklogged(t *testing.T) { require.Eventually(func() bool { return len(errCh) == st.AcceptQueueLength+1 }, 2*time.Second, 100*time.Millisecond) } + +func TestListenerConnectionGater(t *testing.T) { + require := require.New(t) + + testGater := &testGater{} + upgrader := *defaultUpgrader + upgrader.ConnGater = testGater + + ln := createListener(t, &upgrader) + defer ln.Close() + + // no gating. + conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(0)) + require.NoError(err) + require.False(conn.IsClosed()) + _ = conn.Close() + + // rejecting after handshake. + testGater.BlockSecured(true) + testGater.BlockAccept(false) + conn, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(0)) + require.Error(err) + require.Nil(conn) + + // rejecting on accept will trigger first. + testGater.BlockSecured(true) + testGater.BlockAccept(true) + conn, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(0)) + require.Error(err) + require.Nil(conn) + + // rejecting only on acceptance. + testGater.BlockSecured(false) + testGater.BlockAccept(true) + conn, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(0)) + require.Error(err) + require.Nil(conn) + + // back to normal + testGater.BlockSecured(false) + testGater.BlockAccept(false) + conn, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(0)) + require.NoError(err) + require.False(conn.IsClosed()) + _ = conn.Close() +} diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 4b529b4440..0c75c58125 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -4,8 +4,10 @@ import ( "context" "errors" "fmt" + "github.com/libp2p/go-libp2p-core/network" "net" + "github.com/libp2p/go-libp2p-core/connmgr" "github.com/libp2p/go-libp2p-core/mux" "github.com/libp2p/go-libp2p-core/peer" ipnet "github.com/libp2p/go-libp2p-core/pnet" @@ -13,7 +15,6 @@ import ( "github.com/libp2p/go-libp2p-core/transport" "github.com/libp2p/go-libp2p-pnet" - filter "github.com/libp2p/go-maddr-filter" manet "github.com/multiformats/go-multiaddr-net" ) @@ -27,10 +28,10 @@ var AcceptQueueLength = 16 // Upgrader is a multistream upgrader that can upgrade an underlying connection // to a full transport connection (secure and multiplexed). type Upgrader struct { - PSK ipnet.PSK - Secure sec.SecureTransport - Muxer mux.Multiplexer - Filters *filter.Filters + PSK ipnet.PSK + Secure sec.SecureTransport + Muxer mux.Multiplexer + ConnGater connmgr.ConnectionGater } // UpgradeListener upgrades the passed multiaddr-net listener into a full libp2p-transport listener. @@ -55,22 +56,16 @@ func (u *Upgrader) UpgradeOutbound(ctx context.Context, t transport.Transport, m if p == "" { return nil, ErrNilPeer } - return u.upgrade(ctx, t, maconn, p) + return u.upgrade(ctx, t, maconn, p, network.DirOutbound) } // UpgradeInbound upgrades the given inbound multiaddr-net connection into a // full libp2p-transport connection. func (u *Upgrader) UpgradeInbound(ctx context.Context, t transport.Transport, maconn manet.Conn) (transport.CapableConn, error) { - return u.upgrade(ctx, t, maconn, "") + return u.upgrade(ctx, t, maconn, "", network.DirInbound) } -func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn manet.Conn, p peer.ID) (transport.CapableConn, error) { - if u.Filters != nil && u.Filters.AddrBlocked(maconn.RemoteMultiaddr()) { - log.Debugf("blocked connection from %s", maconn.RemoteMultiaddr()) - maconn.Close() - return nil, fmt.Errorf("blocked connection from %s", maconn.RemoteMultiaddr()) - } - +func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn manet.Conn, p peer.ID, dir network.Direction) (transport.CapableConn, error) { var conn net.Conn = maconn if u.PSK != nil { pconn, err := pnet.NewProtectedConn(u.PSK, conn) @@ -91,18 +86,29 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma return nil, fmt.Errorf("failed to negotiate security protocol: %s", err) } + // call the connection gater, if one is registered. + if u.ConnGater != nil && !u.ConnGater.InterceptSecured(dir, sconn.RemotePeer(), maconn) { + if err := maconn.Close(); err != nil { + log.Errorf("failed to close connection with peer %s and addr %s; err: %s", + p.Pretty(), maconn.RemoteMultiaddr(), err) + } + return nil, fmt.Errorf("gater rejected connection with peer %s and addr %s with direction %d", + sconn.RemotePeer().Pretty(), maconn.RemoteMultiaddr(), dir) + } + smconn, err := u.setupMuxer(ctx, sconn, p) if err != nil { sconn.Close() return nil, fmt.Errorf("failed to negotiate stream multiplexer: %s", err) } - return &transportConn{ + tc := &transportConn{ MuxedConn: smconn, ConnMultiaddrs: maconn, ConnSecurity: sconn, transport: t, - }, nil + } + return tc, nil } func (u *Upgrader) setupSecurity(ctx context.Context, conn net.Conn, p peer.ID) (sec.SecureConn, error) { diff --git a/p2p/net/upgrader/upgrader_test.go b/p2p/net/upgrader/upgrader_test.go new file mode 100644 index 0000000000..5d90848aad --- /dev/null +++ b/p2p/net/upgrader/upgrader_test.go @@ -0,0 +1,135 @@ +package stream_test + +import ( + "context" + "errors" + "net" + "testing" + + "github.com/libp2p/go-libp2p-core/mux" + "github.com/libp2p/go-libp2p-core/peer" + "github.com/libp2p/go-libp2p-core/sec/insecure" + "github.com/libp2p/go-libp2p-core/transport" + + mplex "github.com/libp2p/go-libp2p-mplex" + ma "github.com/multiformats/go-multiaddr" + manet "github.com/multiformats/go-multiaddr-net" + + "github.com/stretchr/testify/require" + + st "github.com/libp2p/go-libp2p-transport-upgrader" +) + +var ( + defaultUpgrader = &st.Upgrader{ + Secure: insecure.New(peer.ID(1)), + Muxer: &negotiatingMuxer{}, + } +) + +// negotiatingMuxer sets up a new mplex connection +// It makes sure that this happens at the same time for client and server. +type negotiatingMuxer struct{} + +func (m *negotiatingMuxer) NewConn(c net.Conn, isServer bool) (mux.MuxedConn, error) { + var err error + // run a fake muxer negotiation + if isServer { + _, err = c.Write([]byte("setup")) + } else { + _, err = c.Read(make([]byte, 5)) + } + if err != nil { + return nil, err + } + return mplex.DefaultTransport.NewConn(c, isServer) +} + +// blockingMuxer blocks the muxer negotiation until the contain chan is closed +type blockingMuxer struct { + unblock chan struct{} +} + +var _ mux.Multiplexer = &blockingMuxer{} + +func newBlockingMuxer() *blockingMuxer { + return &blockingMuxer{unblock: make(chan struct{})} +} + +func (m *blockingMuxer) NewConn(c net.Conn, isServer bool) (mux.MuxedConn, error) { + <-m.unblock + return (&negotiatingMuxer{}).NewConn(c, isServer) +} + +func (m *blockingMuxer) Unblock() { + close(m.unblock) +} + +// errorMuxer is a muxer that errors while setting up +type errorMuxer struct{} + +var _ mux.Multiplexer = &errorMuxer{} + +func (m *errorMuxer) NewConn(c net.Conn, isServer bool) (mux.MuxedConn, error) { + return nil, errors.New("mux error") +} + +func testConn(t *testing.T, clientConn, serverConn transport.CapableConn) { + t.Helper() + require := require.New(t) + + cstr, err := clientConn.OpenStream() + require.NoError(err) + + _, err = cstr.Write([]byte("foobar")) + require.NoError(err) + + sstr, err := serverConn.AcceptStream() + require.NoError(err) + + b := make([]byte, 6) + _, err = sstr.Read(b) + require.NoError(err) + require.Equal([]byte("foobar"), b) +} + +func dial(t *testing.T, upgrader *st.Upgrader, raddr ma.Multiaddr, p peer.ID) (transport.CapableConn, error) { + t.Helper() + + macon, err := manet.Dial(raddr) + if err != nil { + return nil, err + } + return upgrader.UpgradeOutbound(context.Background(), nil, macon, p) +} + +func TestOutboundConnectionGating(t *testing.T) { + require := require.New(t) + + ln := createListener(t, defaultUpgrader) + defer ln.Close() + + testGater := &testGater{} + upgrader := *defaultUpgrader + upgrader.ConnGater = testGater + + conn, err := dial(t, &upgrader, ln.Multiaddr(), peer.ID(2)) + require.NoError(err) + require.NotNil(conn) + _ = conn.Close() + + // blocking accepts doesn't affect the dialling side, only the listener. + testGater.BlockAccept(true) + conn, err = dial(t, &upgrader, ln.Multiaddr(), peer.ID(2)) + require.NoError(err) + require.NotNil(conn) + _ = conn.Close() + + // now let's block all connections after being secured. + testGater.BlockSecured(true) + conn, err = dial(t, &upgrader, ln.Multiaddr(), peer.ID(2)) + require.Error(err) + require.Contains(err.Error(), "gater rejected connection") + require.Nil(conn) + +} From d3651207f04a40902337c0848900541ceb36b041 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 17 Dec 2020 16:38:21 +0700 Subject: [PATCH 30/55] fix int to string conversion in tests, update Go version on CI --- p2p/net/upgrader/listener_test.go | 31 ++++++++++++++++--------------- p2p/net/upgrader/upgrader_test.go | 8 ++++---- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index a351aa47fe..a6b80ef8d6 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -1,6 +1,7 @@ package stream_test import ( + "fmt" "io" "sync" "testing" @@ -41,7 +42,7 @@ func TestAcceptSingleConn(t *testing.T) { ln := createListener(t, defaultUpgrader) defer ln.Close() - cconn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(1)) + cconn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("1")) require.NoError(err) sconn, err := ln.Accept() @@ -64,7 +65,7 @@ func TestAcceptMultipleConns(t *testing.T) { }() for i := 0; i < 10; i++ { - cconn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(1)) + cconn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("1")) require.NoError(err) toClose = append(toClose, cconn) @@ -86,7 +87,7 @@ func TestConnectionsClosedIfNotAccepted(t *testing.T) { ln := createListener(t, defaultUpgrader) defer ln.Close() - conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("2")) require.NoError(err) errCh := make(chan error) @@ -117,7 +118,7 @@ func TestFailedUpgradeOnListen(t *testing.T) { require := require.New(t) upgrader := &st.Upgrader{ - Secure: insecure.New(peer.ID(1)), + Secure: insecure.New(peer.ID("1")), Muxer: &errorMuxer{}, } @@ -130,7 +131,7 @@ func TestFailedUpgradeOnListen(t *testing.T) { errCh <- err }() - _, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + _, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("2")) require.Error(err) // close the listener. @@ -164,7 +165,7 @@ func TestListenerClose(t *testing.T) { require.Contains(err.Error(), "use of closed network connection") // doesn't accept new connections when it is closed - _, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(1)) + _, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("1")) require.Error(err) } @@ -175,7 +176,7 @@ func TestListenerCloseClosesQueued(t *testing.T) { var conns []transport.CapableConn for i := 0; i < 10; i++ { - conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(i)) + conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(fmt.Sprintf("%d", i))) require.NoError(err) conns = append(conns, conn) } @@ -213,7 +214,7 @@ func TestConcurrentAccept(t *testing.T) { num = 3 * st.AcceptQueueLength blockingMuxer = newBlockingMuxer() upgrader = &st.Upgrader{ - Secure: insecure.New(peer.ID(1)), + Secure: insecure.New(peer.ID("1")), Muxer: blockingMuxer, } ) @@ -241,7 +242,7 @@ func TestConcurrentAccept(t *testing.T) { go func() { defer wg.Done() - conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("2")) if err != nil { errCh <- err return @@ -270,7 +271,7 @@ func TestAcceptQueueBacklogged(t *testing.T) { // setup AcceptQueueLength connections, but don't accept any of them errCh := make(chan error, st.AcceptQueueLength+1) doDial := func() { - conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(2)) + conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("2")) errCh <- err if conn != nil { _ = conn.Close() @@ -308,7 +309,7 @@ func TestListenerConnectionGater(t *testing.T) { defer ln.Close() // no gating. - conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(0)) + conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("0")) require.NoError(err) require.False(conn.IsClosed()) _ = conn.Close() @@ -316,28 +317,28 @@ func TestListenerConnectionGater(t *testing.T) { // rejecting after handshake. testGater.BlockSecured(true) testGater.BlockAccept(false) - conn, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(0)) + conn, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("0")) require.Error(err) require.Nil(conn) // rejecting on accept will trigger first. testGater.BlockSecured(true) testGater.BlockAccept(true) - conn, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(0)) + conn, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("0")) require.Error(err) require.Nil(conn) // rejecting only on acceptance. testGater.BlockSecured(false) testGater.BlockAccept(true) - conn, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(0)) + conn, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("0")) require.Error(err) require.Nil(conn) // back to normal testGater.BlockSecured(false) testGater.BlockAccept(false) - conn, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(0)) + conn, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("0")) require.NoError(err) require.False(conn.IsClosed()) _ = conn.Close() diff --git a/p2p/net/upgrader/upgrader_test.go b/p2p/net/upgrader/upgrader_test.go index 5d90848aad..d784bd7903 100644 --- a/p2p/net/upgrader/upgrader_test.go +++ b/p2p/net/upgrader/upgrader_test.go @@ -22,7 +22,7 @@ import ( var ( defaultUpgrader = &st.Upgrader{ - Secure: insecure.New(peer.ID(1)), + Secure: insecure.New(peer.ID("1")), Muxer: &negotiatingMuxer{}, } ) @@ -113,21 +113,21 @@ func TestOutboundConnectionGating(t *testing.T) { upgrader := *defaultUpgrader upgrader.ConnGater = testGater - conn, err := dial(t, &upgrader, ln.Multiaddr(), peer.ID(2)) + conn, err := dial(t, &upgrader, ln.Multiaddr(), peer.ID("2")) require.NoError(err) require.NotNil(conn) _ = conn.Close() // blocking accepts doesn't affect the dialling side, only the listener. testGater.BlockAccept(true) - conn, err = dial(t, &upgrader, ln.Multiaddr(), peer.ID(2)) + conn, err = dial(t, &upgrader, ln.Multiaddr(), peer.ID("2")) require.NoError(err) require.NotNil(conn) _ = conn.Close() // now let's block all connections after being secured. testGater.BlockSecured(true) - conn, err = dial(t, &upgrader, ln.Multiaddr(), peer.ID(2)) + conn, err = dial(t, &upgrader, ln.Multiaddr(), peer.ID("2")) require.Error(err) require.Contains(err.Error(), "gater rejected connection") require.Nil(conn) From 508be7ea0b5523ecb6dc45f5ac743c241d238014 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 17 Dec 2020 16:40:31 +0700 Subject: [PATCH 31/55] pass contexts to OpenStream in tests --- p2p/net/upgrader/listener_test.go | 3 ++- p2p/net/upgrader/upgrader_test.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index a6b80ef8d6..08205887c9 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -1,6 +1,7 @@ package stream_test import ( + "context" "fmt" "io" "sync" @@ -93,7 +94,7 @@ func TestConnectionsClosedIfNotAccepted(t *testing.T) { errCh := make(chan error) go func() { defer conn.Close() - str, err := conn.OpenStream() + str, err := conn.OpenStream(context.Background()) if err != nil { errCh <- err return diff --git a/p2p/net/upgrader/upgrader_test.go b/p2p/net/upgrader/upgrader_test.go index d784bd7903..f1960a6273 100644 --- a/p2p/net/upgrader/upgrader_test.go +++ b/p2p/net/upgrader/upgrader_test.go @@ -78,7 +78,7 @@ func testConn(t *testing.T, clientConn, serverConn transport.CapableConn) { t.Helper() require := require.New(t) - cstr, err := clientConn.OpenStream() + cstr, err := clientConn.OpenStream(context.Background()) require.NoError(err) _, err = cstr.Write([]byte("foobar")) From 4453d7a5e04f9b7c32b8e122d3fbf09685db201f Mon Sep 17 00:00:00 2001 From: vyzo Date: Wed, 17 Feb 2021 09:36:57 +0200 Subject: [PATCH 32/55] Implement support for simultaneous open (#25) * implement support for simultaneous open Co-authored-by: aarshkshah1992 --- p2p/net/upgrader/listener_test.go | 20 ++++++++++++++++++-- p2p/net/upgrader/upgrader.go | 12 ++++++------ p2p/net/upgrader/upgrader_test.go | 2 +- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index 08205887c9..611186b4df 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -4,11 +4,13 @@ import ( "context" "fmt" "io" + "net" "sync" "testing" "time" "github.com/libp2p/go-libp2p-core/peer" + "github.com/libp2p/go-libp2p-core/sec" "github.com/libp2p/go-libp2p-core/sec/insecure" "github.com/libp2p/go-libp2p-core/transport" @@ -24,6 +26,20 @@ func init() { transport.AcceptTimeout = 1 * time.Hour } +type MuxAdapter struct { + tpt sec.SecureTransport +} + +func (mux *MuxAdapter) SecureInbound(ctx context.Context, insecure net.Conn) (sec.SecureConn, bool, error) { + sconn, err := mux.tpt.SecureInbound(ctx, insecure) + return sconn, true, err +} + +func (mux *MuxAdapter) SecureOutbound(ctx context.Context, insecure net.Conn, p peer.ID) (sec.SecureConn, bool, error) { + sconn, err := mux.tpt.SecureOutbound(ctx, insecure, p) + return sconn, false, err +} + func createListener(t *testing.T, upgrader *st.Upgrader) transport.Listener { t.Helper() require := require.New(t) @@ -119,7 +135,7 @@ func TestFailedUpgradeOnListen(t *testing.T) { require := require.New(t) upgrader := &st.Upgrader{ - Secure: insecure.New(peer.ID("1")), + Secure: &MuxAdapter{tpt: insecure.New(peer.ID("1"))}, Muxer: &errorMuxer{}, } @@ -215,7 +231,7 @@ func TestConcurrentAccept(t *testing.T) { num = 3 * st.AcceptQueueLength blockingMuxer = newBlockingMuxer() upgrader = &st.Upgrader{ - Secure: insecure.New(peer.ID("1")), + Secure: &MuxAdapter{tpt: insecure.New(peer.ID("1"))}, Muxer: blockingMuxer, } ) diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 0c75c58125..55427e17a4 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -29,7 +29,7 @@ var AcceptQueueLength = 16 // to a full transport connection (secure and multiplexed). type Upgrader struct { PSK ipnet.PSK - Secure sec.SecureTransport + Secure sec.SecureMuxer Muxer mux.Multiplexer ConnGater connmgr.ConnectionGater } @@ -80,7 +80,7 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma return nil, ipnet.ErrNotInPrivateNetwork } - sconn, err := u.setupSecurity(ctx, conn, p) + sconn, server, err := u.setupSecurity(ctx, conn, p) if err != nil { conn.Close() return nil, fmt.Errorf("failed to negotiate security protocol: %s", err) @@ -96,7 +96,7 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma sconn.RemotePeer().Pretty(), maconn.RemoteMultiaddr(), dir) } - smconn, err := u.setupMuxer(ctx, sconn, p) + smconn, err := u.setupMuxer(ctx, sconn, server) if err != nil { sconn.Close() return nil, fmt.Errorf("failed to negotiate stream multiplexer: %s", err) @@ -111,14 +111,14 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma return tc, nil } -func (u *Upgrader) setupSecurity(ctx context.Context, conn net.Conn, p peer.ID) (sec.SecureConn, error) { +func (u *Upgrader) setupSecurity(ctx context.Context, conn net.Conn, p peer.ID) (sec.SecureConn, bool, error) { if p == "" { return u.Secure.SecureInbound(ctx, conn) } return u.Secure.SecureOutbound(ctx, conn, p) } -func (u *Upgrader) setupMuxer(ctx context.Context, conn net.Conn, p peer.ID) (mux.MuxedConn, error) { +func (u *Upgrader) setupMuxer(ctx context.Context, conn net.Conn, server bool) (mux.MuxedConn, error) { // TODO: The muxer should take a context. done := make(chan struct{}) @@ -126,7 +126,7 @@ func (u *Upgrader) setupMuxer(ctx context.Context, conn net.Conn, p peer.ID) (mu var err error go func() { defer close(done) - smconn, err = u.Muxer.NewConn(conn, p == "") + smconn, err = u.Muxer.NewConn(conn, server) }() select { diff --git a/p2p/net/upgrader/upgrader_test.go b/p2p/net/upgrader/upgrader_test.go index f1960a6273..edcac27327 100644 --- a/p2p/net/upgrader/upgrader_test.go +++ b/p2p/net/upgrader/upgrader_test.go @@ -22,7 +22,7 @@ import ( var ( defaultUpgrader = &st.Upgrader{ - Secure: insecure.New(peer.ID("1")), + Secure: &MuxAdapter{tpt: insecure.New(peer.ID("1"))}, Muxer: &negotiatingMuxer{}, } ) From 2397d98fe400d280d8b8205362363bc6fc4d8a5f Mon Sep 17 00:00:00 2001 From: vyzo Date: Thu, 4 Feb 2021 13:46:47 +0200 Subject: [PATCH 33/55] thread underlying conn Stat to allow transports to propagate stat information --- p2p/net/upgrader/conn.go | 5 +++++ p2p/net/upgrader/upgrader.go | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/p2p/net/upgrader/conn.go b/p2p/net/upgrader/conn.go index 2098b151e7..a4ee0cb905 100644 --- a/p2p/net/upgrader/conn.go +++ b/p2p/net/upgrader/conn.go @@ -13,6 +13,7 @@ type transportConn struct { network.ConnMultiaddrs network.ConnSecurity transport transport.Transport + stat network.Stat } func (t *transportConn) Transport() transport.Transport { @@ -33,3 +34,7 @@ func (t *transportConn) String() string { t.RemotePeer(), ) } + +func (t *transportConn) Stat() network.Stat { + return t.stat +} diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 55427e17a4..26b879c5bc 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -66,6 +66,11 @@ func (u *Upgrader) UpgradeInbound(ctx context.Context, t transport.Transport, ma } func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn manet.Conn, p peer.ID, dir network.Direction) (transport.CapableConn, error) { + var stat network.Stat + if cs, ok := maconn.(network.ConnStat); ok { + stat = cs.Stat() + } + var conn net.Conn = maconn if u.PSK != nil { pconn, err := pnet.NewProtectedConn(u.PSK, conn) @@ -107,6 +112,7 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma ConnMultiaddrs: maconn, ConnSecurity: sconn, transport: t, + stat: stat, } return tc, nil } From b743906cf6a1580a63e9c920a61175a794fa1571 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 25 Feb 2021 13:32:16 +0800 Subject: [PATCH 34/55] stop using the deprecated go-multiaddr-net --- p2p/net/upgrader/listener.go | 5 ++--- p2p/net/upgrader/listener_test.go | 5 ++--- p2p/net/upgrader/upgrader.go | 5 ++--- p2p/net/upgrader/upgrader_test.go | 6 +++--- 4 files changed, 9 insertions(+), 12 deletions(-) diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index 2f3cd8e954..f9d23f6235 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -5,11 +5,10 @@ import ( "fmt" "sync" - "github.com/libp2p/go-libp2p-core/transport" - logging "github.com/ipfs/go-log" tec "github.com/jbenet/go-temp-err-catcher" - manet "github.com/multiformats/go-multiaddr-net" + "github.com/libp2p/go-libp2p-core/transport" + manet "github.com/multiformats/go-multiaddr/net" ) var log = logging.Logger("stream-upgrader") diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index 611186b4df..7b4c3ea743 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -14,10 +14,9 @@ import ( "github.com/libp2p/go-libp2p-core/sec/insecure" "github.com/libp2p/go-libp2p-core/transport" - ma "github.com/multiformats/go-multiaddr" - manet "github.com/multiformats/go-multiaddr-net" - st "github.com/libp2p/go-libp2p-transport-upgrader" + ma "github.com/multiformats/go-multiaddr" + manet "github.com/multiformats/go-multiaddr/net" "github.com/stretchr/testify/require" ) diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 26b879c5bc..8623481555 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -4,18 +4,17 @@ import ( "context" "errors" "fmt" - "github.com/libp2p/go-libp2p-core/network" "net" "github.com/libp2p/go-libp2p-core/connmgr" "github.com/libp2p/go-libp2p-core/mux" + "github.com/libp2p/go-libp2p-core/network" "github.com/libp2p/go-libp2p-core/peer" ipnet "github.com/libp2p/go-libp2p-core/pnet" "github.com/libp2p/go-libp2p-core/sec" "github.com/libp2p/go-libp2p-core/transport" "github.com/libp2p/go-libp2p-pnet" - - manet "github.com/multiformats/go-multiaddr-net" + manet "github.com/multiformats/go-multiaddr/net" ) // ErrNilPeer is returned when attempting to upgrade an outbound connection diff --git a/p2p/net/upgrader/upgrader_test.go b/p2p/net/upgrader/upgrader_test.go index edcac27327..74eb8d2df2 100644 --- a/p2p/net/upgrader/upgrader_test.go +++ b/p2p/net/upgrader/upgrader_test.go @@ -13,11 +13,11 @@ import ( mplex "github.com/libp2p/go-libp2p-mplex" ma "github.com/multiformats/go-multiaddr" - manet "github.com/multiformats/go-multiaddr-net" - - "github.com/stretchr/testify/require" + manet "github.com/multiformats/go-multiaddr/net" st "github.com/libp2p/go-libp2p-transport-upgrader" + + "github.com/stretchr/testify/require" ) var ( From 76f49368e8bd2b94565bf0fd0a10343229a392a7 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 25 Feb 2021 14:31:34 +0800 Subject: [PATCH 35/55] don't listen on all interfaces in tests --- p2p/net/upgrader/listener_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index 7b4c3ea743..00ed294851 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -43,7 +43,7 @@ func createListener(t *testing.T, upgrader *st.Upgrader) transport.Listener { t.Helper() require := require.New(t) - addr, err := ma.NewMultiaddr("/ip4/0.0.0.0/tcp/0") + addr, err := ma.NewMultiaddr("/ip4/127.0.0.1/tcp/0") require.NoError(err) ln, err := manet.Listen(addr) From 8739d56d41f4d2ffbdafc7227a65c53f2ce79e60 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 23 Apr 2021 10:52:30 +0700 Subject: [PATCH 36/55] fix staticcheck --- p2p/net/upgrader/listener.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index f9d23f6235..f03e42d8dd 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -13,11 +13,6 @@ import ( var log = logging.Logger("stream-upgrader") -type connErr struct { - conn transport.CapableConn - err error -} - type listener struct { manet.Listener @@ -137,7 +132,7 @@ func (l *listener) handleIncoming() { case <-ctx.Done(): if l.ctx.Err() == nil { // Listener *not* closed but the accept timeout expired. - log.Warningf("listener dropped connection due to slow accept") + log.Warn("listener dropped connection due to slow accept") } // Wait on the context with a timeout. This way, // if we stop accepting connections for some reason, From 3ac4a61cde47c8a2b2a880747d7dbecec1632f25 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 1 Jul 2021 17:06:45 -0700 Subject: [PATCH 37/55] fix typo in error message --- p2p/net/upgrader/upgrader.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 8623481555..de5de08b8f 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -79,8 +79,7 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma } conn = pconn } else if ipnet.ForcePrivateNetwork { - log.Error("tried to dial with no Private Network Protector but usage" + - " of Private Networks is forced by the enviroment") + log.Error("tried to dial with no Private Network Protector but usage of Private Networks is forced by the environment") return nil, ipnet.ErrNotInPrivateNetwork } From fc8779cf925eafdc7464aad95424394aa833a40d Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 26 Jul 2021 21:25:04 +0200 Subject: [PATCH 38/55] chore: update deps --- p2p/net/upgrader/listener_test.go | 84 ++++++++++++++----------------- p2p/net/upgrader/upgrader_test.go | 32 ++++++------ 2 files changed, 56 insertions(+), 60 deletions(-) diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index 00ed294851..73946865ef 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -2,7 +2,6 @@ package stream_test import ( "context" - "fmt" "io" "net" "sync" @@ -11,9 +10,7 @@ import ( "github.com/libp2p/go-libp2p-core/peer" "github.com/libp2p/go-libp2p-core/sec" - "github.com/libp2p/go-libp2p-core/sec/insecure" "github.com/libp2p/go-libp2p-core/transport" - st "github.com/libp2p/go-libp2p-transport-upgrader" ma "github.com/multiformats/go-multiaddr" manet "github.com/multiformats/go-multiaddr/net" @@ -41,24 +38,21 @@ func (mux *MuxAdapter) SecureOutbound(ctx context.Context, insecure net.Conn, p func createListener(t *testing.T, upgrader *st.Upgrader) transport.Listener { t.Helper() - require := require.New(t) - addr, err := ma.NewMultiaddr("/ip4/127.0.0.1/tcp/0") - require.NoError(err) - + require.NoError(t, err) ln, err := manet.Listen(addr) - require.NoError(err) - + require.NoError(t, err) return upgrader.UpgradeListener(nil, ln) } func TestAcceptSingleConn(t *testing.T) { require := require.New(t) - ln := createListener(t, defaultUpgrader) + id, upgrader := createUpgrader(t) + ln := createListener(t, upgrader) defer ln.Close() - cconn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("1")) + cconn, err := dial(t, upgrader, ln.Multiaddr(), id) require.NoError(err) sconn, err := ln.Accept() @@ -70,7 +64,8 @@ func TestAcceptSingleConn(t *testing.T) { func TestAcceptMultipleConns(t *testing.T) { require := require.New(t) - ln := createListener(t, defaultUpgrader) + id, upgrader := createUpgrader(t) + ln := createListener(t, upgrader) defer ln.Close() var toClose []io.Closer @@ -81,7 +76,7 @@ func TestAcceptMultipleConns(t *testing.T) { }() for i := 0; i < 10; i++ { - cconn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("1")) + cconn, err := dial(t, upgrader, ln.Multiaddr(), id) require.NoError(err) toClose = append(toClose, cconn) @@ -100,10 +95,11 @@ func TestConnectionsClosedIfNotAccepted(t *testing.T) { transport.AcceptTimeout = timeout defer func() { transport.AcceptTimeout = 1 * time.Hour }() - ln := createListener(t, defaultUpgrader) + id, upgrader := createUpgrader(t) + ln := createListener(t, upgrader) defer ln.Close() - conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("2")) + conn, err := dial(t, upgrader, ln.Multiaddr(), id) require.NoError(err) errCh := make(chan error) @@ -133,11 +129,8 @@ func TestConnectionsClosedIfNotAccepted(t *testing.T) { func TestFailedUpgradeOnListen(t *testing.T) { require := require.New(t) - upgrader := &st.Upgrader{ - Secure: &MuxAdapter{tpt: insecure.New(peer.ID("1"))}, - Muxer: &errorMuxer{}, - } - + id, upgrader := createUpgrader(t) + upgrader.Muxer = &errorMuxer{} ln := createListener(t, upgrader) defer ln.Close() @@ -147,7 +140,7 @@ func TestFailedUpgradeOnListen(t *testing.T) { errCh <- err }() - _, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("2")) + _, err := dial(t, upgrader, ln.Multiaddr(), id) require.Error(err) // close the listener. @@ -158,7 +151,8 @@ func TestFailedUpgradeOnListen(t *testing.T) { func TestListenerClose(t *testing.T) { require := require.New(t) - ln := createListener(t, defaultUpgrader) + _, upgrader := createUpgrader(t) + ln := createListener(t, upgrader) errCh := make(chan error) go func() { @@ -181,18 +175,19 @@ func TestListenerClose(t *testing.T) { require.Contains(err.Error(), "use of closed network connection") // doesn't accept new connections when it is closed - _, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("1")) + _, err = dial(t, upgrader, ln.Multiaddr(), peer.ID("1")) require.Error(err) } func TestListenerCloseClosesQueued(t *testing.T) { require := require.New(t) - ln := createListener(t, defaultUpgrader) + id, upgrader := createUpgrader(t) + ln := createListener(t, upgrader) var conns []transport.CapableConn for i := 0; i < 10; i++ { - conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID(fmt.Sprintf("%d", i))) + conn, err := dial(t, upgrader, ln.Multiaddr(), id) require.NoError(err) conns = append(conns, conn) } @@ -225,15 +220,11 @@ func TestListenerCloseClosesQueued(t *testing.T) { } func TestConcurrentAccept(t *testing.T) { - var ( - require = require.New(t) - num = 3 * st.AcceptQueueLength - blockingMuxer = newBlockingMuxer() - upgrader = &st.Upgrader{ - Secure: &MuxAdapter{tpt: insecure.New(peer.ID("1"))}, - Muxer: blockingMuxer, - } - ) + var num = 3 * st.AcceptQueueLength + + id, upgrader := createUpgrader(t) + blockingMuxer := newBlockingMuxer() + upgrader.Muxer = blockingMuxer ln := createListener(t, upgrader) defer ln.Close() @@ -258,7 +249,7 @@ func TestConcurrentAccept(t *testing.T) { go func() { defer wg.Done() - conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("2")) + conn, err := dial(t, upgrader, ln.Multiaddr(), id) if err != nil { errCh <- err return @@ -272,22 +263,23 @@ func TestConcurrentAccept(t *testing.T) { time.Sleep(200 * time.Millisecond) // the dials are still blocked, so we shouldn't have any connection available yet - require.Empty(accepted) + require.Empty(t, accepted) blockingMuxer.Unblock() // make all dials succeed - require.Eventually(func() bool { return len(accepted) == num }, 3*time.Second, 100*time.Millisecond) + require.Eventually(t, func() bool { return len(accepted) == num }, 3*time.Second, 100*time.Millisecond) wg.Wait() } func TestAcceptQueueBacklogged(t *testing.T) { require := require.New(t) - ln := createListener(t, defaultUpgrader) + id, upgrader := createUpgrader(t) + ln := createListener(t, upgrader) defer ln.Close() // setup AcceptQueueLength connections, but don't accept any of them errCh := make(chan error, st.AcceptQueueLength+1) doDial := func() { - conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("2")) + conn, err := dial(t, upgrader, ln.Multiaddr(), id) errCh <- err if conn != nil { _ = conn.Close() @@ -318,14 +310,14 @@ func TestListenerConnectionGater(t *testing.T) { require := require.New(t) testGater := &testGater{} - upgrader := *defaultUpgrader + id, upgrader := createUpgrader(t) upgrader.ConnGater = testGater - ln := createListener(t, &upgrader) + ln := createListener(t, upgrader) defer ln.Close() // no gating. - conn, err := dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("0")) + conn, err := dial(t, upgrader, ln.Multiaddr(), id) require.NoError(err) require.False(conn.IsClosed()) _ = conn.Close() @@ -333,28 +325,28 @@ func TestListenerConnectionGater(t *testing.T) { // rejecting after handshake. testGater.BlockSecured(true) testGater.BlockAccept(false) - conn, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("0")) + conn, err = dial(t, upgrader, ln.Multiaddr(), peer.ID("invalid")) require.Error(err) require.Nil(conn) // rejecting on accept will trigger first. testGater.BlockSecured(true) testGater.BlockAccept(true) - conn, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("0")) + conn, err = dial(t, upgrader, ln.Multiaddr(), peer.ID("invalid")) require.Error(err) require.Nil(conn) // rejecting only on acceptance. testGater.BlockSecured(false) testGater.BlockAccept(true) - conn, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("0")) + conn, err = dial(t, upgrader, ln.Multiaddr(), peer.ID("invalid")) require.Error(err) require.Nil(conn) // back to normal testGater.BlockSecured(false) testGater.BlockAccept(false) - conn, err = dial(t, defaultUpgrader, ln.Multiaddr(), peer.ID("0")) + conn, err = dial(t, upgrader, ln.Multiaddr(), id) require.NoError(err) require.False(conn.IsClosed()) _ = conn.Close() diff --git a/p2p/net/upgrader/upgrader_test.go b/p2p/net/upgrader/upgrader_test.go index 74eb8d2df2..795a0112bc 100644 --- a/p2p/net/upgrader/upgrader_test.go +++ b/p2p/net/upgrader/upgrader_test.go @@ -6,26 +6,30 @@ import ( "net" "testing" + "github.com/libp2p/go-libp2p-core/crypto" "github.com/libp2p/go-libp2p-core/mux" "github.com/libp2p/go-libp2p-core/peer" "github.com/libp2p/go-libp2p-core/sec/insecure" + "github.com/libp2p/go-libp2p-core/test" "github.com/libp2p/go-libp2p-core/transport" - mplex "github.com/libp2p/go-libp2p-mplex" + st "github.com/libp2p/go-libp2p-transport-upgrader" ma "github.com/multiformats/go-multiaddr" manet "github.com/multiformats/go-multiaddr/net" - st "github.com/libp2p/go-libp2p-transport-upgrader" - "github.com/stretchr/testify/require" ) -var ( - defaultUpgrader = &st.Upgrader{ - Secure: &MuxAdapter{tpt: insecure.New(peer.ID("1"))}, +func createUpgrader(t *testing.T) (peer.ID, *st.Upgrader) { + priv, _, err := test.RandTestKeyPair(crypto.Ed25519, 256) + require.NoError(t, err) + id, err := peer.IDFromPrivateKey(priv) + require.NoError(t, err) + return id, &st.Upgrader{ + Secure: &MuxAdapter{tpt: insecure.NewWithIdentity(id, priv)}, Muxer: &negotiatingMuxer{}, } -) +} // negotiatingMuxer sets up a new mplex connection // It makes sure that this happens at the same time for client and server. @@ -106,28 +110,28 @@ func dial(t *testing.T, upgrader *st.Upgrader, raddr ma.Multiaddr, p peer.ID) (t func TestOutboundConnectionGating(t *testing.T) { require := require.New(t) - ln := createListener(t, defaultUpgrader) + id, upgrader := createUpgrader(t) + ln := createListener(t, upgrader) defer ln.Close() testGater := &testGater{} - upgrader := *defaultUpgrader - upgrader.ConnGater = testGater - - conn, err := dial(t, &upgrader, ln.Multiaddr(), peer.ID("2")) + _, dialUpgrader := createUpgrader(t) + dialUpgrader.ConnGater = testGater + conn, err := dial(t, dialUpgrader, ln.Multiaddr(), id) require.NoError(err) require.NotNil(conn) _ = conn.Close() // blocking accepts doesn't affect the dialling side, only the listener. testGater.BlockAccept(true) - conn, err = dial(t, &upgrader, ln.Multiaddr(), peer.ID("2")) + conn, err = dial(t, dialUpgrader, ln.Multiaddr(), id) require.NoError(err) require.NotNil(conn) _ = conn.Close() // now let's block all connections after being secured. testGater.BlockSecured(true) - conn, err = dial(t, &upgrader, ln.Multiaddr(), peer.ID("2")) + conn, err = dial(t, dialUpgrader, ln.Multiaddr(), id) require.Error(err) require.Contains(err.Error(), "gater rejected connection") require.Nil(conn) From e267d49e21d5ffdb6b6ebf8d5c08f068c6201061 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 5 Sep 2021 15:56:11 +0100 Subject: [PATCH 39/55] add the peer ID to SecureInbound --- p2p/net/upgrader/listener.go | 6 ++++-- p2p/net/upgrader/listener_test.go | 7 +++++-- p2p/net/upgrader/upgrader.go | 25 ++++++++++++++----------- p2p/net/upgrader/upgrader_test.go | 4 +++- 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index f03e42d8dd..d25a656548 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -5,9 +5,11 @@ import ( "fmt" "sync" + "github.com/libp2p/go-libp2p-core/network" + "github.com/libp2p/go-libp2p-core/transport" + logging "github.com/ipfs/go-log" tec "github.com/jbenet/go-temp-err-catcher" - "github.com/libp2p/go-libp2p-core/transport" manet "github.com/multiformats/go-multiaddr/net" ) @@ -106,7 +108,7 @@ func (l *listener) handleIncoming() { ctx, cancel := context.WithTimeout(l.ctx, transport.AcceptTimeout) defer cancel() - conn, err := l.upgrader.UpgradeInbound(ctx, l.transport, maconn) + conn, err := l.upgrader.Upgrade(ctx, l.transport, maconn, network.DirInbound, "") if err != nil { // Don't bother bubbling this up. We just failed // to completely negotiate the connection. diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index 73946865ef..38d0d91d89 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -12,6 +12,7 @@ import ( "github.com/libp2p/go-libp2p-core/sec" "github.com/libp2p/go-libp2p-core/transport" st "github.com/libp2p/go-libp2p-transport-upgrader" + ma "github.com/multiformats/go-multiaddr" manet "github.com/multiformats/go-multiaddr/net" @@ -26,8 +27,10 @@ type MuxAdapter struct { tpt sec.SecureTransport } -func (mux *MuxAdapter) SecureInbound(ctx context.Context, insecure net.Conn) (sec.SecureConn, bool, error) { - sconn, err := mux.tpt.SecureInbound(ctx, insecure) +var _ sec.SecureMuxer = &MuxAdapter{} + +func (mux *MuxAdapter) SecureInbound(ctx context.Context, insecure net.Conn, p peer.ID) (sec.SecureConn, bool, error) { + sconn, err := mux.tpt.SecureInbound(ctx, insecure, p) return sconn, true, err } diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index de5de08b8f..22e7825911 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -13,7 +13,7 @@ import ( ipnet "github.com/libp2p/go-libp2p-core/pnet" "github.com/libp2p/go-libp2p-core/sec" "github.com/libp2p/go-libp2p-core/transport" - "github.com/libp2p/go-libp2p-pnet" + pnet "github.com/libp2p/go-libp2p-pnet" manet "github.com/multiformats/go-multiaddr/net" ) @@ -51,20 +51,23 @@ func (u *Upgrader) UpgradeListener(t transport.Transport, list manet.Listener) t // UpgradeOutbound upgrades the given outbound multiaddr-net connection into a // full libp2p-transport connection. +// Deprecated: use Upgrade instead. func (u *Upgrader) UpgradeOutbound(ctx context.Context, t transport.Transport, maconn manet.Conn, p peer.ID) (transport.CapableConn, error) { - if p == "" { - return nil, ErrNilPeer - } - return u.upgrade(ctx, t, maconn, p, network.DirOutbound) + return u.Upgrade(ctx, t, maconn, network.DirOutbound, p) } // UpgradeInbound upgrades the given inbound multiaddr-net connection into a // full libp2p-transport connection. +// Deprecated: use Upgrade instead. func (u *Upgrader) UpgradeInbound(ctx context.Context, t transport.Transport, maconn manet.Conn) (transport.CapableConn, error) { - return u.upgrade(ctx, t, maconn, "", network.DirInbound) + return u.Upgrade(ctx, t, maconn, network.DirInbound, "") } -func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn manet.Conn, p peer.ID, dir network.Direction) (transport.CapableConn, error) { +// Upgrade upgrades the multiaddr/net connection into a full libp2p-transport connection. +func (u *Upgrader) Upgrade(ctx context.Context, t transport.Transport, maconn manet.Conn, dir network.Direction, p peer.ID) (transport.CapableConn, error) { + if dir == network.DirOutbound && p == "" { + return nil, ErrNilPeer + } var stat network.Stat if cs, ok := maconn.(network.ConnStat); ok { stat = cs.Stat() @@ -83,7 +86,7 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma return nil, ipnet.ErrNotInPrivateNetwork } - sconn, server, err := u.setupSecurity(ctx, conn, p) + sconn, server, err := u.setupSecurity(ctx, conn, p, dir) if err != nil { conn.Close() return nil, fmt.Errorf("failed to negotiate security protocol: %s", err) @@ -115,9 +118,9 @@ func (u *Upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma return tc, nil } -func (u *Upgrader) setupSecurity(ctx context.Context, conn net.Conn, p peer.ID) (sec.SecureConn, bool, error) { - if p == "" { - return u.Secure.SecureInbound(ctx, conn) +func (u *Upgrader) setupSecurity(ctx context.Context, conn net.Conn, p peer.ID, dir network.Direction) (sec.SecureConn, bool, error) { + if dir == network.DirInbound { + return u.Secure.SecureInbound(ctx, conn, p) } return u.Secure.SecureOutbound(ctx, conn, p) } diff --git a/p2p/net/upgrader/upgrader_test.go b/p2p/net/upgrader/upgrader_test.go index 795a0112bc..4bebf25e18 100644 --- a/p2p/net/upgrader/upgrader_test.go +++ b/p2p/net/upgrader/upgrader_test.go @@ -8,12 +8,14 @@ import ( "github.com/libp2p/go-libp2p-core/crypto" "github.com/libp2p/go-libp2p-core/mux" + "github.com/libp2p/go-libp2p-core/network" "github.com/libp2p/go-libp2p-core/peer" "github.com/libp2p/go-libp2p-core/sec/insecure" "github.com/libp2p/go-libp2p-core/test" "github.com/libp2p/go-libp2p-core/transport" mplex "github.com/libp2p/go-libp2p-mplex" st "github.com/libp2p/go-libp2p-transport-upgrader" + ma "github.com/multiformats/go-multiaddr" manet "github.com/multiformats/go-multiaddr/net" @@ -104,7 +106,7 @@ func dial(t *testing.T, upgrader *st.Upgrader, raddr ma.Multiaddr, p peer.ID) (t if err != nil { return nil, err } - return upgrader.UpgradeOutbound(context.Background(), nil, macon, p) + return upgrader.Upgrade(context.Background(), nil, macon, network.DirOutbound, p) } func TestOutboundConnectionGating(t *testing.T) { From 289fcae846deda126510d76edf80fc39b5b3527b Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 8 Sep 2021 12:59:29 +0100 Subject: [PATCH 40/55] increase timeout in TestConnectionsClosedIfNotAccepted on CI --- p2p/net/upgrader/listener_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index 73946865ef..c557dbd07b 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -4,6 +4,7 @@ import ( "context" "io" "net" + "os" "sync" "testing" "time" @@ -91,9 +92,13 @@ func TestAcceptMultipleConns(t *testing.T) { func TestConnectionsClosedIfNotAccepted(t *testing.T) { require := require.New(t) - const timeout = 200 * time.Millisecond + var timeout = 100 * time.Millisecond + if os.Getenv("CI") != "" { + timeout = 500 * time.Millisecond + } + origAcceptTimeout := transport.AcceptTimeout transport.AcceptTimeout = timeout - defer func() { transport.AcceptTimeout = 1 * time.Hour }() + t.Cleanup(func() { transport.AcceptTimeout = origAcceptTimeout }) id, upgrader := createUpgrader(t) ln := createListener(t, upgrader) From 893c9a40e3cada5af10f5e1fb3b4b7d747193312 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 22 Sep 2021 12:04:36 +0100 Subject: [PATCH 41/55] chore: update go-log --- p2p/net/upgrader/listener.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index d25a656548..9178fe33a4 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -8,7 +8,7 @@ import ( "github.com/libp2p/go-libp2p-core/network" "github.com/libp2p/go-libp2p-core/transport" - logging "github.com/ipfs/go-log" + logging "github.com/ipfs/go-log/v2" tec "github.com/jbenet/go-temp-err-catcher" manet "github.com/multiformats/go-multiaddr/net" ) From 85de7f0dfb0cf4b90914b5c1f8d1deb461c7f3fa Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Fri, 10 Dec 2021 15:11:23 +0400 Subject: [PATCH 42/55] use the new network.ConnStats --- p2p/net/upgrader/conn.go | 4 ++-- p2p/net/upgrader/upgrader.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/p2p/net/upgrader/conn.go b/p2p/net/upgrader/conn.go index a4ee0cb905..4b8c0bcecd 100644 --- a/p2p/net/upgrader/conn.go +++ b/p2p/net/upgrader/conn.go @@ -13,7 +13,7 @@ type transportConn struct { network.ConnMultiaddrs network.ConnSecurity transport transport.Transport - stat network.Stat + stat network.ConnStats } func (t *transportConn) Transport() transport.Transport { @@ -35,6 +35,6 @@ func (t *transportConn) String() string { ) } -func (t *transportConn) Stat() network.Stat { +func (t *transportConn) Stat() network.ConnStats { return t.stat } diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 22e7825911..ff669c3f74 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -68,7 +68,7 @@ func (u *Upgrader) Upgrade(ctx context.Context, t transport.Transport, maconn ma if dir == network.DirOutbound && p == "" { return nil, ErrNilPeer } - var stat network.Stat + var stat network.ConnStats if cs, ok := maconn.(network.ConnStat); ok { stat = cs.Stat() } From fe89a27a3fc26b3f32760cf86a73ddb49b0738d1 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 12 Dec 2021 16:29:19 +0400 Subject: [PATCH 43/55] fix flaky TestAcceptQueueBacklogged test --- p2p/net/upgrader/listener_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index 1d339d2166..b47fd91fad 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -6,6 +6,7 @@ import ( "net" "os" "sync" + "sync/atomic" "testing" "time" @@ -285,33 +286,32 @@ func TestAcceptQueueBacklogged(t *testing.T) { defer ln.Close() // setup AcceptQueueLength connections, but don't accept any of them - errCh := make(chan error, st.AcceptQueueLength+1) + var counter int32 // to be used atomically doDial := func() { conn, err := dial(t, upgrader, ln.Multiaddr(), id) - errCh <- err - if conn != nil { - _ = conn.Close() - } + require.NoError(err) + atomic.AddInt32(&counter, 1) + t.Cleanup(func() { conn.Close() }) } for i := 0; i < st.AcceptQueueLength; i++ { go doDial() } - require.Eventually(func() bool { return len(errCh) == st.AcceptQueueLength }, 2*time.Second, 100*time.Millisecond) + require.Eventually(func() bool { return int(atomic.LoadInt32(&counter)) == st.AcceptQueueLength }, 2*time.Second, 50*time.Millisecond) // dial a new connection. This connection should not complete setup, since the queue is full go doDial() - time.Sleep(500 * time.Millisecond) - require.Len(errCh, st.AcceptQueueLength) + time.Sleep(100 * time.Millisecond) + require.Equal(int(atomic.LoadInt32(&counter)), st.AcceptQueueLength) // accept a single connection. Now the new connection should be set up, and fill the queue again conn, err := ln.Accept() require.NoError(err) - _ = conn.Close() + require.NoError(conn.Close()) - require.Eventually(func() bool { return len(errCh) == st.AcceptQueueLength+1 }, 2*time.Second, 100*time.Millisecond) + require.Eventually(func() bool { return int(atomic.LoadInt32(&counter)) == st.AcceptQueueLength+1 }, 2*time.Second, 50*time.Millisecond) } func TestListenerConnectionGater(t *testing.T) { From f06d0dfdce9356cee036e546ae0f7a2a98c79b7a Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 20 Dec 2021 11:38:04 +0400 Subject: [PATCH 44/55] reset the temporary error catcher delay after successful accept --- p2p/net/upgrader/listener.go | 1 + 1 file changed, 1 insertion(+) diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index 9178fe33a4..6552decec9 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -80,6 +80,7 @@ func (l *listener) handleIncoming() { l.err = err return } + catcher.Reset() // gate the connection if applicable if l.upgrader.ConnGater != nil && !l.upgrader.ConnGater.InterceptAccept(maconn) { From b16a4466da99d88505964759f74ceedd11530507 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 20 Dec 2021 11:46:26 +0400 Subject: [PATCH 45/55] make the accept timeout configurable, stop using transport.AcceptTimeout --- p2p/net/upgrader/listener.go | 2 +- p2p/net/upgrader/listener_test.go | 8 +------- p2p/net/upgrader/upgrader.go | 18 ++++++++++++++++++ 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index 9178fe33a4..b808cd9753 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -105,7 +105,7 @@ func (l *listener) handleIncoming() { go func() { defer wg.Done() - ctx, cancel := context.WithTimeout(l.ctx, transport.AcceptTimeout) + ctx, cancel := context.WithTimeout(l.ctx, l.upgrader.acceptTimeout()) defer cancel() conn, err := l.upgrader.Upgrade(ctx, l.transport, maconn, network.DirInbound, "") diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index b47fd91fad..768a9290cb 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -21,10 +21,6 @@ import ( "github.com/stretchr/testify/require" ) -func init() { - transport.AcceptTimeout = 1 * time.Hour -} - type MuxAdapter struct { tpt sec.SecureTransport } @@ -100,11 +96,9 @@ func TestConnectionsClosedIfNotAccepted(t *testing.T) { if os.Getenv("CI") != "" { timeout = 500 * time.Millisecond } - origAcceptTimeout := transport.AcceptTimeout - transport.AcceptTimeout = timeout - t.Cleanup(func() { transport.AcceptTimeout = origAcceptTimeout }) id, upgrader := createUpgrader(t) + upgrader.AcceptTimeout = timeout ln := createListener(t, upgrader) defer ln.Close() diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index ff669c3f74..b9123bf2ab 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "net" + "time" "github.com/libp2p/go-libp2p-core/connmgr" "github.com/libp2p/go-libp2p-core/mux" @@ -13,6 +14,7 @@ import ( ipnet "github.com/libp2p/go-libp2p-core/pnet" "github.com/libp2p/go-libp2p-core/sec" "github.com/libp2p/go-libp2p-core/transport" + pnet "github.com/libp2p/go-libp2p-pnet" manet "github.com/multiformats/go-multiaddr/net" ) @@ -24,6 +26,8 @@ var ErrNilPeer = errors.New("nil peer") // AcceptQueueLength is the number of connections to fully setup before not accepting any new connections var AcceptQueueLength = 16 +const defaultAcceptTimeout = 15 * time.Second + // Upgrader is a multistream upgrader that can upgrade an underlying connection // to a full transport connection (secure and multiplexed). type Upgrader struct { @@ -31,6 +35,13 @@ type Upgrader struct { Secure sec.SecureMuxer Muxer mux.Multiplexer ConnGater connmgr.ConnectionGater + + // AcceptTimeout is the maximum duration an Accept is allowed to take. + // This includes the time between accepting the raw network connection, + // protocol selection as well as the handshake, if applicable. + // + // If unset, the default value (15s) is used. + AcceptTimeout time.Duration } // UpgradeListener upgrades the passed multiaddr-net listener into a full libp2p-transport listener. @@ -147,3 +158,10 @@ func (u *Upgrader) setupMuxer(ctx context.Context, conn net.Conn, server bool) ( return nil, ctx.Err() } } + +func (u *Upgrader) acceptTimeout() time.Duration { + if u.AcceptTimeout == 0 { + return defaultAcceptTimeout + } + return u.AcceptTimeout +} From a3f424bc1d6edf160e740869b58c1b5ca99046ed Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 2 Jan 2022 15:47:58 +0400 Subject: [PATCH 46/55] use the new transport.Upgrader interface --- p2p/net/upgrader/listener.go | 6 +- p2p/net/upgrader/listener_test.go | 21 +++---- p2p/net/upgrader/upgrader.go | 91 ++++++++++++++++++++----------- p2p/net/upgrader/upgrader_test.go | 19 ++++--- 4 files changed, 79 insertions(+), 58 deletions(-) diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index 93c82570d9..c26bc5a0df 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -19,7 +19,7 @@ type listener struct { manet.Listener transport transport.Transport - upgrader *Upgrader + upgrader *upgrader incoming chan transport.CapableConn err error @@ -83,7 +83,7 @@ func (l *listener) handleIncoming() { catcher.Reset() // gate the connection if applicable - if l.upgrader.ConnGater != nil && !l.upgrader.ConnGater.InterceptAccept(maconn) { + if l.upgrader.connGater != nil && !l.upgrader.connGater.InterceptAccept(maconn) { log.Debugf("gater blocked incoming connection on local addr %s from %s", maconn.LocalMultiaddr(), maconn.RemoteMultiaddr()) @@ -106,7 +106,7 @@ func (l *listener) handleIncoming() { go func() { defer wg.Done() - ctx, cancel := context.WithTimeout(l.ctx, l.upgrader.acceptTimeout()) + ctx, cancel := context.WithTimeout(l.ctx, l.upgrader.acceptTimeout) defer cancel() conn, err := l.upgrader.Upgrade(ctx, l.transport, maconn, network.DirInbound, "") diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index 768a9290cb..76f514569b 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -37,7 +37,7 @@ func (mux *MuxAdapter) SecureOutbound(ctx context.Context, insecure net.Conn, p return sconn, false, err } -func createListener(t *testing.T, upgrader *st.Upgrader) transport.Listener { +func createListener(t *testing.T, upgrader transport.Upgrader) transport.Listener { t.Helper() addr, err := ma.NewMultiaddr("/ip4/127.0.0.1/tcp/0") require.NoError(t, err) @@ -97,8 +97,7 @@ func TestConnectionsClosedIfNotAccepted(t *testing.T) { timeout = 500 * time.Millisecond } - id, upgrader := createUpgrader(t) - upgrader.AcceptTimeout = timeout + id, upgrader := createUpgrader(t, st.WithAcceptTimeout(timeout)) ln := createListener(t, upgrader) defer ln.Close() @@ -132,10 +131,8 @@ func TestConnectionsClosedIfNotAccepted(t *testing.T) { func TestFailedUpgradeOnListen(t *testing.T) { require := require.New(t) - id, upgrader := createUpgrader(t) - upgrader.Muxer = &errorMuxer{} + id, upgrader := createUpgraderWithMuxer(t, &errorMuxer{}) ln := createListener(t, upgrader) - defer ln.Close() errCh := make(chan error) go func() { @@ -171,9 +168,8 @@ func TestListenerClose(t *testing.T) { } // unblocks Accept when it is closed. - err := ln.Close() - require.NoError(err) - err = <-errCh + require.NoError(ln.Close()) + err := <-errCh require.Error(err) require.Contains(err.Error(), "use of closed network connection") @@ -225,10 +221,8 @@ func TestListenerCloseClosesQueued(t *testing.T) { func TestConcurrentAccept(t *testing.T) { var num = 3 * st.AcceptQueueLength - id, upgrader := createUpgrader(t) blockingMuxer := newBlockingMuxer() - upgrader.Muxer = blockingMuxer - + id, upgrader := createUpgraderWithMuxer(t, blockingMuxer) ln := createListener(t, upgrader) defer ln.Close() @@ -312,8 +306,7 @@ func TestListenerConnectionGater(t *testing.T) { require := require.New(t) testGater := &testGater{} - id, upgrader := createUpgrader(t) - upgrader.ConnGater = testGater + id, upgrader := createUpgrader(t, st.WithConnectionGater(testGater)) ln := createListener(t, upgrader) defer ln.Close() diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index b9123bf2ab..139c2fe926 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -28,24 +28,63 @@ var AcceptQueueLength = 16 const defaultAcceptTimeout = 15 * time.Second +type Option func(*upgrader) error + +func WithPSK(psk ipnet.PSK) Option { + return func(u *upgrader) error { + u.psk = psk + return nil + } +} + +func WithAcceptTimeout(t time.Duration) Option { + return func(u *upgrader) error { + u.acceptTimeout = t + return nil + } +} + +func WithConnectionGater(g connmgr.ConnectionGater) Option { + return func(u *upgrader) error { + u.connGater = g + return nil + } +} + // Upgrader is a multistream upgrader that can upgrade an underlying connection // to a full transport connection (secure and multiplexed). -type Upgrader struct { - PSK ipnet.PSK - Secure sec.SecureMuxer - Muxer mux.Multiplexer - ConnGater connmgr.ConnectionGater +type upgrader struct { + secure sec.SecureMuxer + muxer mux.Multiplexer + + psk ipnet.PSK + connGater connmgr.ConnectionGater // AcceptTimeout is the maximum duration an Accept is allowed to take. // This includes the time between accepting the raw network connection, // protocol selection as well as the handshake, if applicable. // // If unset, the default value (15s) is used. - AcceptTimeout time.Duration + acceptTimeout time.Duration +} + +var _ transport.Upgrader = &upgrader{} + +func NewUpgrader(secureMuxer sec.SecureMuxer, muxer mux.Multiplexer, opts ...Option) (transport.Upgrader, error) { + u := &upgrader{ + secure: secureMuxer, + muxer: muxer, + acceptTimeout: defaultAcceptTimeout, + } + for _, opt := range opts { + if err := opt(u); err != nil { + return nil, err + } + } + return u, nil } -// UpgradeListener upgrades the passed multiaddr-net listener into a full libp2p-transport listener. -func (u *Upgrader) UpgradeListener(t transport.Transport, list manet.Listener) transport.Listener { +func (u *upgrader) UpgradeListener(t transport.Transport, list manet.Listener) transport.Listener { ctx, cancel := context.WithCancel(context.Background()) l := &listener{ Listener: list, @@ -60,22 +99,15 @@ func (u *Upgrader) UpgradeListener(t transport.Transport, list manet.Listener) t return l } -// UpgradeOutbound upgrades the given outbound multiaddr-net connection into a -// full libp2p-transport connection. -// Deprecated: use Upgrade instead. -func (u *Upgrader) UpgradeOutbound(ctx context.Context, t transport.Transport, maconn manet.Conn, p peer.ID) (transport.CapableConn, error) { +func (u *upgrader) UpgradeOutbound(ctx context.Context, t transport.Transport, maconn manet.Conn, p peer.ID) (transport.CapableConn, error) { return u.Upgrade(ctx, t, maconn, network.DirOutbound, p) } -// UpgradeInbound upgrades the given inbound multiaddr-net connection into a -// full libp2p-transport connection. -// Deprecated: use Upgrade instead. -func (u *Upgrader) UpgradeInbound(ctx context.Context, t transport.Transport, maconn manet.Conn) (transport.CapableConn, error) { +func (u *upgrader) UpgradeInbound(ctx context.Context, t transport.Transport, maconn manet.Conn) (transport.CapableConn, error) { return u.Upgrade(ctx, t, maconn, network.DirInbound, "") } -// Upgrade upgrades the multiaddr/net connection into a full libp2p-transport connection. -func (u *Upgrader) Upgrade(ctx context.Context, t transport.Transport, maconn manet.Conn, dir network.Direction, p peer.ID) (transport.CapableConn, error) { +func (u *upgrader) Upgrade(ctx context.Context, t transport.Transport, maconn manet.Conn, dir network.Direction, p peer.ID) (transport.CapableConn, error) { if dir == network.DirOutbound && p == "" { return nil, ErrNilPeer } @@ -85,8 +117,8 @@ func (u *Upgrader) Upgrade(ctx context.Context, t transport.Transport, maconn ma } var conn net.Conn = maconn - if u.PSK != nil { - pconn, err := pnet.NewProtectedConn(u.PSK, conn) + if u.psk != nil { + pconn, err := pnet.NewProtectedConn(u.psk, conn) if err != nil { conn.Close() return nil, fmt.Errorf("failed to setup private network protector: %s", err) @@ -104,7 +136,7 @@ func (u *Upgrader) Upgrade(ctx context.Context, t transport.Transport, maconn ma } // call the connection gater, if one is registered. - if u.ConnGater != nil && !u.ConnGater.InterceptSecured(dir, sconn.RemotePeer(), maconn) { + if u.connGater != nil && !u.connGater.InterceptSecured(dir, sconn.RemotePeer(), maconn) { if err := maconn.Close(); err != nil { log.Errorf("failed to close connection with peer %s and addr %s; err: %s", p.Pretty(), maconn.RemoteMultiaddr(), err) @@ -129,14 +161,14 @@ func (u *Upgrader) Upgrade(ctx context.Context, t transport.Transport, maconn ma return tc, nil } -func (u *Upgrader) setupSecurity(ctx context.Context, conn net.Conn, p peer.ID, dir network.Direction) (sec.SecureConn, bool, error) { +func (u *upgrader) setupSecurity(ctx context.Context, conn net.Conn, p peer.ID, dir network.Direction) (sec.SecureConn, bool, error) { if dir == network.DirInbound { - return u.Secure.SecureInbound(ctx, conn, p) + return u.secure.SecureInbound(ctx, conn, p) } - return u.Secure.SecureOutbound(ctx, conn, p) + return u.secure.SecureOutbound(ctx, conn, p) } -func (u *Upgrader) setupMuxer(ctx context.Context, conn net.Conn, server bool) (mux.MuxedConn, error) { +func (u *upgrader) setupMuxer(ctx context.Context, conn net.Conn, server bool) (mux.MuxedConn, error) { // TODO: The muxer should take a context. done := make(chan struct{}) @@ -144,7 +176,7 @@ func (u *Upgrader) setupMuxer(ctx context.Context, conn net.Conn, server bool) ( var err error go func() { defer close(done) - smconn, err = u.Muxer.NewConn(conn, server) + smconn, err = u.muxer.NewConn(conn, server) }() select { @@ -158,10 +190,3 @@ func (u *Upgrader) setupMuxer(ctx context.Context, conn net.Conn, server bool) ( return nil, ctx.Err() } } - -func (u *Upgrader) acceptTimeout() time.Duration { - if u.AcceptTimeout == 0 { - return defaultAcceptTimeout - } - return u.AcceptTimeout -} diff --git a/p2p/net/upgrader/upgrader_test.go b/p2p/net/upgrader/upgrader_test.go index 4bebf25e18..e16f9cc99c 100644 --- a/p2p/net/upgrader/upgrader_test.go +++ b/p2p/net/upgrader/upgrader_test.go @@ -13,6 +13,7 @@ import ( "github.com/libp2p/go-libp2p-core/sec/insecure" "github.com/libp2p/go-libp2p-core/test" "github.com/libp2p/go-libp2p-core/transport" + mplex "github.com/libp2p/go-libp2p-mplex" st "github.com/libp2p/go-libp2p-transport-upgrader" @@ -22,15 +23,18 @@ import ( "github.com/stretchr/testify/require" ) -func createUpgrader(t *testing.T) (peer.ID, *st.Upgrader) { +func createUpgrader(t *testing.T, opts ...st.Option) (peer.ID, transport.Upgrader) { + return createUpgraderWithMuxer(t, &negotiatingMuxer{}, opts...) +} + +func createUpgraderWithMuxer(t *testing.T, muxer mux.Multiplexer, opts ...st.Option) (peer.ID, transport.Upgrader) { priv, _, err := test.RandTestKeyPair(crypto.Ed25519, 256) require.NoError(t, err) id, err := peer.IDFromPrivateKey(priv) require.NoError(t, err) - return id, &st.Upgrader{ - Secure: &MuxAdapter{tpt: insecure.NewWithIdentity(id, priv)}, - Muxer: &negotiatingMuxer{}, - } + u, err := st.NewUpgrader(&MuxAdapter{tpt: insecure.NewWithIdentity(id, priv)}, muxer, opts...) + require.NoError(t, err) + return id, u } // negotiatingMuxer sets up a new mplex connection @@ -99,7 +103,7 @@ func testConn(t *testing.T, clientConn, serverConn transport.CapableConn) { require.Equal([]byte("foobar"), b) } -func dial(t *testing.T, upgrader *st.Upgrader, raddr ma.Multiaddr, p peer.ID) (transport.CapableConn, error) { +func dial(t *testing.T, upgrader transport.Upgrader, raddr ma.Multiaddr, p peer.ID) (transport.CapableConn, error) { t.Helper() macon, err := manet.Dial(raddr) @@ -117,8 +121,7 @@ func TestOutboundConnectionGating(t *testing.T) { defer ln.Close() testGater := &testGater{} - _, dialUpgrader := createUpgrader(t) - dialUpgrader.ConnGater = testGater + _, dialUpgrader := createUpgrader(t, st.WithConnectionGater(testGater)) conn, err := dial(t, dialUpgrader, ln.Multiaddr(), id) require.NoError(err) require.NotNil(conn) From 5a79888154d4ba2c91cdffd6c790991c297edecb Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 2 Jan 2022 16:10:17 +0400 Subject: [PATCH 47/55] rename the constructor from NewUpgrader to New --- p2p/net/upgrader/upgrader.go | 2 +- p2p/net/upgrader/upgrader_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 139c2fe926..39c41bc455 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -70,7 +70,7 @@ type upgrader struct { var _ transport.Upgrader = &upgrader{} -func NewUpgrader(secureMuxer sec.SecureMuxer, muxer mux.Multiplexer, opts ...Option) (transport.Upgrader, error) { +func New(secureMuxer sec.SecureMuxer, muxer mux.Multiplexer, opts ...Option) (transport.Upgrader, error) { u := &upgrader{ secure: secureMuxer, muxer: muxer, diff --git a/p2p/net/upgrader/upgrader_test.go b/p2p/net/upgrader/upgrader_test.go index e16f9cc99c..8f04fad2f7 100644 --- a/p2p/net/upgrader/upgrader_test.go +++ b/p2p/net/upgrader/upgrader_test.go @@ -32,7 +32,7 @@ func createUpgraderWithMuxer(t *testing.T, muxer mux.Multiplexer, opts ...st.Opt require.NoError(t, err) id, err := peer.IDFromPrivateKey(priv) require.NoError(t, err) - u, err := st.NewUpgrader(&MuxAdapter{tpt: insecure.NewWithIdentity(id, priv)}, muxer, opts...) + u, err := st.New(&MuxAdapter{tpt: insecure.NewWithIdentity(id, priv)}, muxer, opts...) require.NoError(t, err) return id, u } From e8056e8f400450cd4aae3004e0e4317e5c1b5dcd Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sun, 2 Jan 2022 16:15:04 +0400 Subject: [PATCH 48/55] remove UpgradeInbound and UpgradeOutbound --- p2p/net/upgrader/upgrader.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 39c41bc455..77a784ebf4 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -99,14 +99,6 @@ func (u *upgrader) UpgradeListener(t transport.Transport, list manet.Listener) t return l } -func (u *upgrader) UpgradeOutbound(ctx context.Context, t transport.Transport, maconn manet.Conn, p peer.ID) (transport.CapableConn, error) { - return u.Upgrade(ctx, t, maconn, network.DirOutbound, p) -} - -func (u *upgrader) UpgradeInbound(ctx context.Context, t transport.Transport, maconn manet.Conn) (transport.CapableConn, error) { - return u.Upgrade(ctx, t, maconn, network.DirInbound, "") -} - func (u *upgrader) Upgrade(ctx context.Context, t transport.Transport, maconn manet.Conn, dir network.Direction, p peer.ID) (transport.CapableConn, error) { if dir == network.DirOutbound && p == "" { return nil, ErrNilPeer From 8f6a3dc30411e240bd4abaa79e444f919922cd25 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Sat, 8 Jan 2022 17:36:12 +0400 Subject: [PATCH 49/55] rename the package to upgrader --- p2p/net/upgrader/conn.go | 2 +- p2p/net/upgrader/gater_test.go | 2 +- p2p/net/upgrader/listener.go | 4 +- p2p/net/upgrader/listener_test.go | 75 ++++++++++++++++--------------- p2p/net/upgrader/threshold.go | 2 +- p2p/net/upgrader/upgrader.go | 2 +- p2p/net/upgrader/upgrader_test.go | 17 +++---- 7 files changed, 53 insertions(+), 51 deletions(-) diff --git a/p2p/net/upgrader/conn.go b/p2p/net/upgrader/conn.go index 4b8c0bcecd..1825ac820a 100644 --- a/p2p/net/upgrader/conn.go +++ b/p2p/net/upgrader/conn.go @@ -1,4 +1,4 @@ -package stream +package upgrader import ( "fmt" diff --git a/p2p/net/upgrader/gater_test.go b/p2p/net/upgrader/gater_test.go index 858bd9e564..2d6b889058 100644 --- a/p2p/net/upgrader/gater_test.go +++ b/p2p/net/upgrader/gater_test.go @@ -1,4 +1,4 @@ -package stream_test +package upgrader_test import ( "sync" diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index c26bc5a0df..d0a392ba02 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -1,4 +1,4 @@ -package stream +package upgrader import ( "context" @@ -13,7 +13,7 @@ import ( manet "github.com/multiformats/go-multiaddr/net" ) -var log = logging.Logger("stream-upgrader") +var log = logging.Logger("upgrader") type listener struct { manet.Listener diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index 76f514569b..77345c72e8 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -1,4 +1,4 @@ -package stream_test +package upgrader_test import ( "context" @@ -10,10 +10,11 @@ import ( "testing" "time" + upgrader "github.com/libp2p/go-libp2p-transport-upgrader" + "github.com/libp2p/go-libp2p-core/peer" "github.com/libp2p/go-libp2p-core/sec" "github.com/libp2p/go-libp2p-core/transport" - st "github.com/libp2p/go-libp2p-transport-upgrader" ma "github.com/multiformats/go-multiaddr" manet "github.com/multiformats/go-multiaddr/net" @@ -37,23 +38,23 @@ func (mux *MuxAdapter) SecureOutbound(ctx context.Context, insecure net.Conn, p return sconn, false, err } -func createListener(t *testing.T, upgrader transport.Upgrader) transport.Listener { +func createListener(t *testing.T, u transport.Upgrader) transport.Listener { t.Helper() addr, err := ma.NewMultiaddr("/ip4/127.0.0.1/tcp/0") require.NoError(t, err) ln, err := manet.Listen(addr) require.NoError(t, err) - return upgrader.UpgradeListener(nil, ln) + return u.UpgradeListener(nil, ln) } func TestAcceptSingleConn(t *testing.T) { require := require.New(t) - id, upgrader := createUpgrader(t) - ln := createListener(t, upgrader) + id, u := createUpgrader(t) + ln := createListener(t, u) defer ln.Close() - cconn, err := dial(t, upgrader, ln.Multiaddr(), id) + cconn, err := dial(t, u, ln.Multiaddr(), id) require.NoError(err) sconn, err := ln.Accept() @@ -65,8 +66,8 @@ func TestAcceptSingleConn(t *testing.T) { func TestAcceptMultipleConns(t *testing.T) { require := require.New(t) - id, upgrader := createUpgrader(t) - ln := createListener(t, upgrader) + id, u := createUpgrader(t) + ln := createListener(t, u) defer ln.Close() var toClose []io.Closer @@ -77,7 +78,7 @@ func TestAcceptMultipleConns(t *testing.T) { }() for i := 0; i < 10; i++ { - cconn, err := dial(t, upgrader, ln.Multiaddr(), id) + cconn, err := dial(t, u, ln.Multiaddr(), id) require.NoError(err) toClose = append(toClose, cconn) @@ -97,11 +98,11 @@ func TestConnectionsClosedIfNotAccepted(t *testing.T) { timeout = 500 * time.Millisecond } - id, upgrader := createUpgrader(t, st.WithAcceptTimeout(timeout)) - ln := createListener(t, upgrader) + id, u := createUpgrader(t, upgrader.WithAcceptTimeout(timeout)) + ln := createListener(t, u) defer ln.Close() - conn, err := dial(t, upgrader, ln.Multiaddr(), id) + conn, err := dial(t, u, ln.Multiaddr(), id) require.NoError(err) errCh := make(chan error) @@ -131,8 +132,8 @@ func TestConnectionsClosedIfNotAccepted(t *testing.T) { func TestFailedUpgradeOnListen(t *testing.T) { require := require.New(t) - id, upgrader := createUpgraderWithMuxer(t, &errorMuxer{}) - ln := createListener(t, upgrader) + id, u := createUpgraderWithMuxer(t, &errorMuxer{}) + ln := createListener(t, u) errCh := make(chan error) go func() { @@ -140,7 +141,7 @@ func TestFailedUpgradeOnListen(t *testing.T) { errCh <- err }() - _, err := dial(t, upgrader, ln.Multiaddr(), id) + _, err := dial(t, u, ln.Multiaddr(), id) require.Error(err) // close the listener. @@ -151,8 +152,8 @@ func TestFailedUpgradeOnListen(t *testing.T) { func TestListenerClose(t *testing.T) { require := require.New(t) - _, upgrader := createUpgrader(t) - ln := createListener(t, upgrader) + _, u := createUpgrader(t) + ln := createListener(t, u) errCh := make(chan error) go func() { @@ -174,7 +175,7 @@ func TestListenerClose(t *testing.T) { require.Contains(err.Error(), "use of closed network connection") // doesn't accept new connections when it is closed - _, err = dial(t, upgrader, ln.Multiaddr(), peer.ID("1")) + _, err = dial(t, u, ln.Multiaddr(), peer.ID("1")) require.Error(err) } @@ -219,11 +220,11 @@ func TestListenerCloseClosesQueued(t *testing.T) { } func TestConcurrentAccept(t *testing.T) { - var num = 3 * st.AcceptQueueLength + var num = 3 * upgrader.AcceptQueueLength blockingMuxer := newBlockingMuxer() - id, upgrader := createUpgraderWithMuxer(t, blockingMuxer) - ln := createListener(t, upgrader) + id, u := createUpgraderWithMuxer(t, blockingMuxer) + ln := createListener(t, u) defer ln.Close() accepted := make(chan transport.CapableConn, num) @@ -246,7 +247,7 @@ func TestConcurrentAccept(t *testing.T) { go func() { defer wg.Done() - conn, err := dial(t, upgrader, ln.Multiaddr(), id) + conn, err := dial(t, u, ln.Multiaddr(), id) if err != nil { errCh <- err return @@ -269,50 +270,50 @@ func TestConcurrentAccept(t *testing.T) { func TestAcceptQueueBacklogged(t *testing.T) { require := require.New(t) - id, upgrader := createUpgrader(t) - ln := createListener(t, upgrader) + id, u := createUpgrader(t) + ln := createListener(t, u) defer ln.Close() // setup AcceptQueueLength connections, but don't accept any of them var counter int32 // to be used atomically doDial := func() { - conn, err := dial(t, upgrader, ln.Multiaddr(), id) + conn, err := dial(t, u, ln.Multiaddr(), id) require.NoError(err) atomic.AddInt32(&counter, 1) t.Cleanup(func() { conn.Close() }) } - for i := 0; i < st.AcceptQueueLength; i++ { + for i := 0; i < upgrader.AcceptQueueLength; i++ { go doDial() } - require.Eventually(func() bool { return int(atomic.LoadInt32(&counter)) == st.AcceptQueueLength }, 2*time.Second, 50*time.Millisecond) + require.Eventually(func() bool { return int(atomic.LoadInt32(&counter)) == upgrader.AcceptQueueLength }, 2*time.Second, 50*time.Millisecond) // dial a new connection. This connection should not complete setup, since the queue is full go doDial() time.Sleep(100 * time.Millisecond) - require.Equal(int(atomic.LoadInt32(&counter)), st.AcceptQueueLength) + require.Equal(int(atomic.LoadInt32(&counter)), upgrader.AcceptQueueLength) // accept a single connection. Now the new connection should be set up, and fill the queue again conn, err := ln.Accept() require.NoError(err) require.NoError(conn.Close()) - require.Eventually(func() bool { return int(atomic.LoadInt32(&counter)) == st.AcceptQueueLength+1 }, 2*time.Second, 50*time.Millisecond) + require.Eventually(func() bool { return int(atomic.LoadInt32(&counter)) == upgrader.AcceptQueueLength+1 }, 2*time.Second, 50*time.Millisecond) } func TestListenerConnectionGater(t *testing.T) { require := require.New(t) testGater := &testGater{} - id, upgrader := createUpgrader(t, st.WithConnectionGater(testGater)) + id, u := createUpgrader(t, upgrader.WithConnectionGater(testGater)) - ln := createListener(t, upgrader) + ln := createListener(t, u) defer ln.Close() // no gating. - conn, err := dial(t, upgrader, ln.Multiaddr(), id) + conn, err := dial(t, u, ln.Multiaddr(), id) require.NoError(err) require.False(conn.IsClosed()) _ = conn.Close() @@ -320,28 +321,28 @@ func TestListenerConnectionGater(t *testing.T) { // rejecting after handshake. testGater.BlockSecured(true) testGater.BlockAccept(false) - conn, err = dial(t, upgrader, ln.Multiaddr(), peer.ID("invalid")) + conn, err = dial(t, u, ln.Multiaddr(), peer.ID("invalid")) require.Error(err) require.Nil(conn) // rejecting on accept will trigger first. testGater.BlockSecured(true) testGater.BlockAccept(true) - conn, err = dial(t, upgrader, ln.Multiaddr(), peer.ID("invalid")) + conn, err = dial(t, u, ln.Multiaddr(), peer.ID("invalid")) require.Error(err) require.Nil(conn) // rejecting only on acceptance. testGater.BlockSecured(false) testGater.BlockAccept(true) - conn, err = dial(t, upgrader, ln.Multiaddr(), peer.ID("invalid")) + conn, err = dial(t, u, ln.Multiaddr(), peer.ID("invalid")) require.Error(err) require.Nil(conn) // back to normal testGater.BlockSecured(false) testGater.BlockAccept(false) - conn, err = dial(t, upgrader, ln.Multiaddr(), id) + conn, err = dial(t, u, ln.Multiaddr(), id) require.NoError(err) require.False(conn.IsClosed()) _ = conn.Close() diff --git a/p2p/net/upgrader/threshold.go b/p2p/net/upgrader/threshold.go index 2c278bd398..1e8b112cb8 100644 --- a/p2p/net/upgrader/threshold.go +++ b/p2p/net/upgrader/threshold.go @@ -1,4 +1,4 @@ -package stream +package upgrader import ( "sync" diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 77a784ebf4..5e8a8a08f7 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -1,4 +1,4 @@ -package stream +package upgrader import ( "context" diff --git a/p2p/net/upgrader/upgrader_test.go b/p2p/net/upgrader/upgrader_test.go index 8f04fad2f7..0db1c8c566 100644 --- a/p2p/net/upgrader/upgrader_test.go +++ b/p2p/net/upgrader/upgrader_test.go @@ -1,4 +1,4 @@ -package stream_test +package upgrader_test import ( "context" @@ -6,6 +6,8 @@ import ( "net" "testing" + upgrader "github.com/libp2p/go-libp2p-transport-upgrader" + "github.com/libp2p/go-libp2p-core/crypto" "github.com/libp2p/go-libp2p-core/mux" "github.com/libp2p/go-libp2p-core/network" @@ -15,7 +17,6 @@ import ( "github.com/libp2p/go-libp2p-core/transport" mplex "github.com/libp2p/go-libp2p-mplex" - st "github.com/libp2p/go-libp2p-transport-upgrader" ma "github.com/multiformats/go-multiaddr" manet "github.com/multiformats/go-multiaddr/net" @@ -23,16 +24,16 @@ import ( "github.com/stretchr/testify/require" ) -func createUpgrader(t *testing.T, opts ...st.Option) (peer.ID, transport.Upgrader) { +func createUpgrader(t *testing.T, opts ...upgrader.Option) (peer.ID, transport.Upgrader) { return createUpgraderWithMuxer(t, &negotiatingMuxer{}, opts...) } -func createUpgraderWithMuxer(t *testing.T, muxer mux.Multiplexer, opts ...st.Option) (peer.ID, transport.Upgrader) { +func createUpgraderWithMuxer(t *testing.T, muxer mux.Multiplexer, opts ...upgrader.Option) (peer.ID, transport.Upgrader) { priv, _, err := test.RandTestKeyPair(crypto.Ed25519, 256) require.NoError(t, err) id, err := peer.IDFromPrivateKey(priv) require.NoError(t, err) - u, err := st.New(&MuxAdapter{tpt: insecure.NewWithIdentity(id, priv)}, muxer, opts...) + u, err := upgrader.New(&MuxAdapter{tpt: insecure.NewWithIdentity(id, priv)}, muxer, opts...) require.NoError(t, err) return id, u } @@ -116,12 +117,12 @@ func dial(t *testing.T, upgrader transport.Upgrader, raddr ma.Multiaddr, p peer. func TestOutboundConnectionGating(t *testing.T) { require := require.New(t) - id, upgrader := createUpgrader(t) - ln := createListener(t, upgrader) + id, u := createUpgrader(t) + ln := createListener(t, u) defer ln.Close() testGater := &testGater{} - _, dialUpgrader := createUpgrader(t, st.WithConnectionGater(testGater)) + _, dialUpgrader := createUpgrader(t, upgrader.WithConnectionGater(testGater)) conn, err := dial(t, dialUpgrader, ln.Multiaddr(), id) require.NoError(err) require.NotNil(conn) From ccf4315ace2506e570513b34fbbd5c592e32755e Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 22 Dec 2021 15:04:20 +0400 Subject: [PATCH 50/55] use the Resource Manager --- p2p/net/upgrader/conn.go | 12 +++++ p2p/net/upgrader/listener.go | 14 +++++- p2p/net/upgrader/listener_test.go | 83 +++++++++++++++++++++++++------ p2p/net/upgrader/upgrader.go | 54 ++++++++++++++++---- p2p/net/upgrader/upgrader_test.go | 70 ++++++++++++++++++++------ 5 files changed, 192 insertions(+), 41 deletions(-) diff --git a/p2p/net/upgrader/conn.go b/p2p/net/upgrader/conn.go index 1825ac820a..9aa3b2f2e9 100644 --- a/p2p/net/upgrader/conn.go +++ b/p2p/net/upgrader/conn.go @@ -13,9 +13,12 @@ type transportConn struct { network.ConnMultiaddrs network.ConnSecurity transport transport.Transport + scope network.ConnManagementScope stat network.ConnStats } +var _ transport.CapableConn = &transportConn{} + func (t *transportConn) Transport() transport.Transport { return t.transport } @@ -38,3 +41,12 @@ func (t *transportConn) String() string { func (t *transportConn) Stat() network.ConnStats { return t.stat } + +func (t *transportConn) Scope() network.ConnScope { + return t.scope +} + +func (t *transportConn) Close() error { + defer t.scope.Done() + return t.MuxedConn.Close() +} diff --git a/p2p/net/upgrader/listener.go b/p2p/net/upgrader/listener.go index d0a392ba02..fa94e8e6fc 100644 --- a/p2p/net/upgrader/listener.go +++ b/p2p/net/upgrader/listener.go @@ -20,6 +20,7 @@ type listener struct { transport transport.Transport upgrader *upgrader + rcmgr network.ResourceManager incoming chan transport.CapableConn err error @@ -86,9 +87,17 @@ func (l *listener) handleIncoming() { if l.upgrader.connGater != nil && !l.upgrader.connGater.InterceptAccept(maconn) { log.Debugf("gater blocked incoming connection on local addr %s from %s", maconn.LocalMultiaddr(), maconn.RemoteMultiaddr()) + if err := maconn.Close(); err != nil { + log.Warnf("failed to close incoming connection rejected by gater: %s", err) + } + continue + } + connScope, err := l.rcmgr.OpenConnection(network.DirInbound, true) + if err != nil { + log.Debugw("resource manager blocked accept of new connection", "error", err) if err := maconn.Close(); err != nil { - log.Warnf("failed to incoming connection rejected by gater; err: %s", err) + log.Warnf("failed to incoming connection rejected by resource manager: %s", err) } continue } @@ -109,7 +118,7 @@ func (l *listener) handleIncoming() { ctx, cancel := context.WithTimeout(l.ctx, l.upgrader.acceptTimeout) defer cancel() - conn, err := l.upgrader.Upgrade(ctx, l.transport, maconn, network.DirInbound, "") + conn, err := l.upgrader.Upgrade(ctx, l.transport, maconn, network.DirInbound, "", connScope) if err != nil { // Don't bother bubbling this up. We just failed // to completely negotiate the connection. @@ -117,6 +126,7 @@ func (l *listener) handleIncoming() { err, maconn.LocalMultiaddr(), maconn.RemoteMultiaddr()) + connScope.Done() return } diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index 77345c72e8..c8daef187a 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -2,6 +2,7 @@ package upgrader_test import ( "context" + "errors" "io" "net" "os" @@ -12,13 +13,16 @@ import ( upgrader "github.com/libp2p/go-libp2p-transport-upgrader" + "github.com/libp2p/go-libp2p-core/network" "github.com/libp2p/go-libp2p-core/peer" "github.com/libp2p/go-libp2p-core/sec" "github.com/libp2p/go-libp2p-core/transport" + mocknetwork "github.com/libp2p/go-libp2p-testing/mocks/network" ma "github.com/multiformats/go-multiaddr" manet "github.com/multiformats/go-multiaddr/net" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" ) @@ -54,7 +58,7 @@ func TestAcceptSingleConn(t *testing.T) { ln := createListener(t, u) defer ln.Close() - cconn, err := dial(t, u, ln.Multiaddr(), id) + cconn, err := dial(t, u, ln.Multiaddr(), id, network.NullScope) require.NoError(err) sconn, err := ln.Accept() @@ -78,7 +82,7 @@ func TestAcceptMultipleConns(t *testing.T) { }() for i := 0; i < 10; i++ { - cconn, err := dial(t, u, ln.Multiaddr(), id) + cconn, err := dial(t, u, ln.Multiaddr(), id, network.NullScope) require.NoError(err) toClose = append(toClose, cconn) @@ -102,7 +106,7 @@ func TestConnectionsClosedIfNotAccepted(t *testing.T) { ln := createListener(t, u) defer ln.Close() - conn, err := dial(t, u, ln.Multiaddr(), id) + conn, err := dial(t, u, ln.Multiaddr(), id, network.NullScope) require.NoError(err) errCh := make(chan error) @@ -141,7 +145,7 @@ func TestFailedUpgradeOnListen(t *testing.T) { errCh <- err }() - _, err := dial(t, u, ln.Multiaddr(), id) + _, err := dial(t, u, ln.Multiaddr(), id, network.NullScope) require.Error(err) // close the listener. @@ -175,7 +179,7 @@ func TestListenerClose(t *testing.T) { require.Contains(err.Error(), "use of closed network connection") // doesn't accept new connections when it is closed - _, err = dial(t, u, ln.Multiaddr(), peer.ID("1")) + _, err = dial(t, u, ln.Multiaddr(), peer.ID("1"), network.NullScope) require.Error(err) } @@ -187,7 +191,7 @@ func TestListenerCloseClosesQueued(t *testing.T) { var conns []transport.CapableConn for i := 0; i < 10; i++ { - conn, err := dial(t, upgrader, ln.Multiaddr(), id) + conn, err := dial(t, upgrader, ln.Multiaddr(), id, network.NullScope) require.NoError(err) conns = append(conns, conn) } @@ -247,7 +251,7 @@ func TestConcurrentAccept(t *testing.T) { go func() { defer wg.Done() - conn, err := dial(t, u, ln.Multiaddr(), id) + conn, err := dial(t, u, ln.Multiaddr(), id, network.NullScope) if err != nil { errCh <- err return @@ -277,7 +281,7 @@ func TestAcceptQueueBacklogged(t *testing.T) { // setup AcceptQueueLength connections, but don't accept any of them var counter int32 // to be used atomically doDial := func() { - conn, err := dial(t, u, ln.Multiaddr(), id) + conn, err := dial(t, u, ln.Multiaddr(), id, network.NullScope) require.NoError(err) atomic.AddInt32(&counter, 1) t.Cleanup(func() { conn.Close() }) @@ -313,7 +317,7 @@ func TestListenerConnectionGater(t *testing.T) { defer ln.Close() // no gating. - conn, err := dial(t, u, ln.Multiaddr(), id) + conn, err := dial(t, u, ln.Multiaddr(), id, network.NullScope) require.NoError(err) require.False(conn.IsClosed()) _ = conn.Close() @@ -321,29 +325,80 @@ func TestListenerConnectionGater(t *testing.T) { // rejecting after handshake. testGater.BlockSecured(true) testGater.BlockAccept(false) - conn, err = dial(t, u, ln.Multiaddr(), peer.ID("invalid")) + conn, err = dial(t, u, ln.Multiaddr(), "invalid", network.NullScope) require.Error(err) require.Nil(conn) - // rejecting on accept will trigger first. + // rejecting on accept will trigger firupgrader. testGater.BlockSecured(true) testGater.BlockAccept(true) - conn, err = dial(t, u, ln.Multiaddr(), peer.ID("invalid")) + conn, err = dial(t, u, ln.Multiaddr(), "invalid", network.NullScope) require.Error(err) require.Nil(conn) // rejecting only on acceptance. testGater.BlockSecured(false) testGater.BlockAccept(true) - conn, err = dial(t, u, ln.Multiaddr(), peer.ID("invalid")) + conn, err = dial(t, u, ln.Multiaddr(), "invalid", network.NullScope) require.Error(err) require.Nil(conn) // back to normal testGater.BlockSecured(false) testGater.BlockAccept(false) - conn, err = dial(t, u, ln.Multiaddr(), id) + conn, err = dial(t, u, ln.Multiaddr(), id, network.NullScope) require.NoError(err) require.False(conn.IsClosed()) _ = conn.Close() } + +func TestListenerResourceManagement(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + rcmgr := mocknetwork.NewMockResourceManager(ctrl) + id, upgrader := createUpgrader(t, upgrader.WithResourceManager(rcmgr)) + ln := createListener(t, upgrader) + defer ln.Close() + + connScope := mocknetwork.NewMockConnManagementScope(ctrl) + gomock.InOrder( + rcmgr.EXPECT().OpenConnection(network.DirInbound, true).Return(connScope, nil), + connScope.EXPECT().SetPeer(id), + connScope.EXPECT().PeerScope(), + ) + + cconn, err := dial(t, upgrader, ln.Multiaddr(), id, network.NullScope) + require.NoError(t, err) + defer cconn.Close() + + sconn, err := ln.Accept() + require.NoError(t, err) + connScope.EXPECT().Done() + defer sconn.Close() +} + +func TestListenerResourceManagementDenied(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + rcmgr := mocknetwork.NewMockResourceManager(ctrl) + id, upgrader := createUpgrader(t, upgrader.WithResourceManager(rcmgr)) + ln := createListener(t, upgrader) + + rcmgr.EXPECT().OpenConnection(network.DirInbound, true).Return(nil, errors.New("nope")) + _, err := dial(t, upgrader, ln.Multiaddr(), id, network.NullScope) + require.Error(t, err) + + done := make(chan struct{}) + go func() { + defer close(done) + ln.Accept() + }() + + select { + case <-done: + t.Fatal("accept shouldn't have accepted anything") + case <-time.After(50 * time.Millisecond): + } + require.NoError(t, ln.Close()) + <-done +} diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 5e8a8a08f7..9a3ec0f8ba 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -8,7 +8,6 @@ import ( "time" "github.com/libp2p/go-libp2p-core/connmgr" - "github.com/libp2p/go-libp2p-core/mux" "github.com/libp2p/go-libp2p-core/network" "github.com/libp2p/go-libp2p-core/peer" ipnet "github.com/libp2p/go-libp2p-core/pnet" @@ -51,14 +50,22 @@ func WithConnectionGater(g connmgr.ConnectionGater) Option { } } +func WithResourceManager(m network.ResourceManager) Option { + return func(u *upgrader) error { + u.rcmgr = m + return nil + } +} + // Upgrader is a multistream upgrader that can upgrade an underlying connection // to a full transport connection (secure and multiplexed). type upgrader struct { secure sec.SecureMuxer - muxer mux.Multiplexer + muxer network.Multiplexer psk ipnet.PSK connGater connmgr.ConnectionGater + rcmgr network.ResourceManager // AcceptTimeout is the maximum duration an Accept is allowed to take. // This includes the time between accepting the raw network connection, @@ -70,7 +77,7 @@ type upgrader struct { var _ transport.Upgrader = &upgrader{} -func New(secureMuxer sec.SecureMuxer, muxer mux.Multiplexer, opts ...Option) (transport.Upgrader, error) { +func New(secureMuxer sec.SecureMuxer, muxer network.Multiplexer, opts ...Option) (transport.Upgrader, error) { u := &upgrader{ secure: secureMuxer, muxer: muxer, @@ -81,15 +88,20 @@ func New(secureMuxer sec.SecureMuxer, muxer mux.Multiplexer, opts ...Option) (tr return nil, err } } + if u.rcmgr == nil { + u.rcmgr = network.NullResourceManager + } return u, nil } +// UpgradeListener upgrades the passed multiaddr-net listener into a full libp2p-transport listener. func (u *upgrader) UpgradeListener(t transport.Transport, list manet.Listener) transport.Listener { ctx, cancel := context.WithCancel(context.Background()) l := &listener{ Listener: list, upgrader: u, transport: t, + rcmgr: u.rcmgr, threshold: newThreshold(AcceptQueueLength), incoming: make(chan transport.CapableConn), cancel: cancel, @@ -99,7 +111,17 @@ func (u *upgrader) UpgradeListener(t transport.Transport, list manet.Listener) t return l } -func (u *upgrader) Upgrade(ctx context.Context, t transport.Transport, maconn manet.Conn, dir network.Direction, p peer.ID) (transport.CapableConn, error) { +// Upgrade upgrades the multiaddr/net connection into a full libp2p-transport connection. +func (u *upgrader) Upgrade(ctx context.Context, t transport.Transport, maconn manet.Conn, dir network.Direction, p peer.ID, connScope network.ConnManagementScope) (transport.CapableConn, error) { + c, err := u.upgrade(ctx, t, maconn, dir, p, connScope) + if err != nil { + connScope.Done() + return nil, err + } + return c, nil +} + +func (u *upgrader) upgrade(ctx context.Context, t transport.Transport, maconn manet.Conn, dir network.Direction, p peer.ID, connScope network.ConnManagementScope) (transport.CapableConn, error) { if dir == network.DirOutbound && p == "" { return nil, ErrNilPeer } @@ -130,14 +152,25 @@ func (u *upgrader) Upgrade(ctx context.Context, t transport.Transport, maconn ma // call the connection gater, if one is registered. if u.connGater != nil && !u.connGater.InterceptSecured(dir, sconn.RemotePeer(), maconn) { if err := maconn.Close(); err != nil { - log.Errorf("failed to close connection with peer %s and addr %s; err: %s", - p.Pretty(), maconn.RemoteMultiaddr(), err) + log.Errorw("failed to close connection", "peer", p, "addr", maconn.RemoteMultiaddr(), "error", err) } return nil, fmt.Errorf("gater rejected connection with peer %s and addr %s with direction %d", sconn.RemotePeer().Pretty(), maconn.RemoteMultiaddr(), dir) } + // Only call SetPeer if we didn't know the peer ID in advance. + // Otherwise, the caller will already have called it before calling Upgrade. + if p == "" { + if err := connScope.SetPeer(sconn.RemotePeer()); err != nil { + log.Debugw("resource manager blocked connection for peer", "peer", sconn.RemotePeer(), "addr", conn.RemoteAddr(), "error", err) + if err := maconn.Close(); err != nil { + log.Errorw("failed to close connection", "peer", p, "addr", maconn.RemoteMultiaddr(), "error", err) + } + return nil, fmt.Errorf("resource manager connection with peer %s and addr %s with direction %d", + sconn.RemotePeer().Pretty(), maconn.RemoteMultiaddr(), dir) + } + } - smconn, err := u.setupMuxer(ctx, sconn, server) + smconn, err := u.setupMuxer(ctx, sconn, server, connScope.PeerScope()) if err != nil { sconn.Close() return nil, fmt.Errorf("failed to negotiate stream multiplexer: %s", err) @@ -149,6 +182,7 @@ func (u *upgrader) Upgrade(ctx context.Context, t transport.Transport, maconn ma ConnSecurity: sconn, transport: t, stat: stat, + scope: connScope, } return tc, nil } @@ -160,15 +194,15 @@ func (u *upgrader) setupSecurity(ctx context.Context, conn net.Conn, p peer.ID, return u.secure.SecureOutbound(ctx, conn, p) } -func (u *upgrader) setupMuxer(ctx context.Context, conn net.Conn, server bool) (mux.MuxedConn, error) { +func (u *upgrader) setupMuxer(ctx context.Context, conn net.Conn, server bool, scope network.PeerScope) (network.MuxedConn, error) { // TODO: The muxer should take a context. done := make(chan struct{}) - var smconn mux.MuxedConn + var smconn network.MuxedConn var err error go func() { defer close(done) - smconn, err = u.muxer.NewConn(conn, server) + smconn, err = u.muxer.NewConn(conn, server, scope) }() select { diff --git a/p2p/net/upgrader/upgrader_test.go b/p2p/net/upgrader/upgrader_test.go index 0db1c8c566..bf40438d09 100644 --- a/p2p/net/upgrader/upgrader_test.go +++ b/p2p/net/upgrader/upgrader_test.go @@ -9,7 +9,6 @@ import ( upgrader "github.com/libp2p/go-libp2p-transport-upgrader" "github.com/libp2p/go-libp2p-core/crypto" - "github.com/libp2p/go-libp2p-core/mux" "github.com/libp2p/go-libp2p-core/network" "github.com/libp2p/go-libp2p-core/peer" "github.com/libp2p/go-libp2p-core/sec/insecure" @@ -17,10 +16,12 @@ import ( "github.com/libp2p/go-libp2p-core/transport" mplex "github.com/libp2p/go-libp2p-mplex" + mocknetwork "github.com/libp2p/go-libp2p-testing/mocks/network" + "github.com/golang/mock/gomock" ma "github.com/multiformats/go-multiaddr" - manet "github.com/multiformats/go-multiaddr/net" + manet "github.com/multiformats/go-multiaddr/net" "github.com/stretchr/testify/require" ) @@ -28,7 +29,7 @@ func createUpgrader(t *testing.T, opts ...upgrader.Option) (peer.ID, transport.U return createUpgraderWithMuxer(t, &negotiatingMuxer{}, opts...) } -func createUpgraderWithMuxer(t *testing.T, muxer mux.Multiplexer, opts ...upgrader.Option) (peer.ID, transport.Upgrader) { +func createUpgraderWithMuxer(t *testing.T, muxer network.Multiplexer, opts ...upgrader.Option) (peer.ID, transport.Upgrader) { priv, _, err := test.RandTestKeyPair(crypto.Ed25519, 256) require.NoError(t, err) id, err := peer.IDFromPrivateKey(priv) @@ -42,7 +43,7 @@ func createUpgraderWithMuxer(t *testing.T, muxer mux.Multiplexer, opts ...upgrad // It makes sure that this happens at the same time for client and server. type negotiatingMuxer struct{} -func (m *negotiatingMuxer) NewConn(c net.Conn, isServer bool) (mux.MuxedConn, error) { +func (m *negotiatingMuxer) NewConn(c net.Conn, isServer bool, scope network.PeerScope) (network.MuxedConn, error) { var err error // run a fake muxer negotiation if isServer { @@ -53,7 +54,7 @@ func (m *negotiatingMuxer) NewConn(c net.Conn, isServer bool) (mux.MuxedConn, er if err != nil { return nil, err } - return mplex.DefaultTransport.NewConn(c, isServer) + return mplex.DefaultTransport.NewConn(c, isServer, scope) } // blockingMuxer blocks the muxer negotiation until the contain chan is closed @@ -61,15 +62,15 @@ type blockingMuxer struct { unblock chan struct{} } -var _ mux.Multiplexer = &blockingMuxer{} +var _ network.Multiplexer = &blockingMuxer{} func newBlockingMuxer() *blockingMuxer { return &blockingMuxer{unblock: make(chan struct{})} } -func (m *blockingMuxer) NewConn(c net.Conn, isServer bool) (mux.MuxedConn, error) { +func (m *blockingMuxer) NewConn(c net.Conn, isServer bool, scope network.PeerScope) (network.MuxedConn, error) { <-m.unblock - return (&negotiatingMuxer{}).NewConn(c, isServer) + return (&negotiatingMuxer{}).NewConn(c, isServer, scope) } func (m *blockingMuxer) Unblock() { @@ -79,9 +80,9 @@ func (m *blockingMuxer) Unblock() { // errorMuxer is a muxer that errors while setting up type errorMuxer struct{} -var _ mux.Multiplexer = &errorMuxer{} +var _ network.Multiplexer = &errorMuxer{} -func (m *errorMuxer) NewConn(c net.Conn, isServer bool) (mux.MuxedConn, error) { +func (m *errorMuxer) NewConn(c net.Conn, isServer bool, scope network.PeerScope) (network.MuxedConn, error) { return nil, errors.New("mux error") } @@ -104,14 +105,14 @@ func testConn(t *testing.T, clientConn, serverConn transport.CapableConn) { require.Equal([]byte("foobar"), b) } -func dial(t *testing.T, upgrader transport.Upgrader, raddr ma.Multiaddr, p peer.ID) (transport.CapableConn, error) { +func dial(t *testing.T, upgrader transport.Upgrader, raddr ma.Multiaddr, p peer.ID, scope network.ConnManagementScope) (transport.CapableConn, error) { t.Helper() macon, err := manet.Dial(raddr) if err != nil { return nil, err } - return upgrader.Upgrade(context.Background(), nil, macon, network.DirOutbound, p) + return upgrader.Upgrade(context.Background(), nil, macon, network.DirOutbound, p, scope) } func TestOutboundConnectionGating(t *testing.T) { @@ -123,23 +124,62 @@ func TestOutboundConnectionGating(t *testing.T) { testGater := &testGater{} _, dialUpgrader := createUpgrader(t, upgrader.WithConnectionGater(testGater)) - conn, err := dial(t, dialUpgrader, ln.Multiaddr(), id) + conn, err := dial(t, dialUpgrader, ln.Multiaddr(), id, network.NullScope) require.NoError(err) require.NotNil(conn) _ = conn.Close() // blocking accepts doesn't affect the dialling side, only the listener. testGater.BlockAccept(true) - conn, err = dial(t, dialUpgrader, ln.Multiaddr(), id) + conn, err = dial(t, dialUpgrader, ln.Multiaddr(), id, network.NullScope) require.NoError(err) require.NotNil(conn) _ = conn.Close() // now let's block all connections after being secured. testGater.BlockSecured(true) - conn, err = dial(t, dialUpgrader, ln.Multiaddr(), id) + conn, err = dial(t, dialUpgrader, ln.Multiaddr(), id, network.NullScope) require.Error(err) require.Contains(err.Error(), "gater rejected connection") require.Nil(conn) +} +func TestOutboundResourceManagement(t *testing.T) { + t.Run("successful handshake", func(t *testing.T) { + id, upgrader := createUpgrader(t) + ln := createListener(t, upgrader) + defer ln.Close() + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + connScope := mocknetwork.NewMockConnManagementScope(ctrl) + connScope.EXPECT().PeerScope().Return(network.NullScope) + _, dialUpgrader := createUpgrader(t) + conn, err := dial(t, dialUpgrader, ln.Multiaddr(), id, connScope) + require.NoError(t, err) + require.NotNil(t, conn) + connScope.EXPECT().Done() + require.NoError(t, conn.Close()) + }) + + t.Run("failed negotiation", func(t *testing.T) { + id, upgrader := createUpgraderWithMuxer(t, &errorMuxer{}) + ln := createListener(t, upgrader) + defer ln.Close() + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + connScope := mocknetwork.NewMockConnManagementScope(ctrl) + gomock.InOrder( + connScope.EXPECT().PeerScope().Return(network.NullScope), + connScope.EXPECT().Done(), + ) + _, dialUpgrader := createUpgrader(t) + _, err := dial(t, dialUpgrader, ln.Multiaddr(), id, connScope) + require.Error(t, err) + }) + + t.Run("blocked by the resource manager", func(t *testing.T) { + + }) } From 67fe7657b34695efc4f5d70f0f5df03504b1a29b Mon Sep 17 00:00:00 2001 From: vyzo Date: Wed, 2 Feb 2022 19:17:01 +0200 Subject: [PATCH 51/55] always set the peer if the peer scope is null Removes the very ugly contract of "if you know the peer you have set it" and makes the code robust against upstream bugz. --- p2p/net/upgrader/upgrader.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 9a3ec0f8ba..3ea71bf777 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -157,9 +157,9 @@ func (u *upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma return nil, fmt.Errorf("gater rejected connection with peer %s and addr %s with direction %d", sconn.RemotePeer().Pretty(), maconn.RemoteMultiaddr(), dir) } - // Only call SetPeer if we didn't know the peer ID in advance. - // Otherwise, the caller will already have called it before calling Upgrade. - if p == "" { + // Only call SetPeer if it hasn't already been set -- this can happen when we don't know + // the peer in advance and in some bug scenarios. + if connScope.PeerScope() == nil { if err := connScope.SetPeer(sconn.RemotePeer()); err != nil { log.Debugw("resource manager blocked connection for peer", "peer", sconn.RemotePeer(), "addr", conn.RemoteAddr(), "error", err) if err := maconn.Close(); err != nil { From 191278d558ba6baecb073e84b3906943797c9e29 Mon Sep 17 00:00:00 2001 From: vyzo Date: Wed, 2 Feb 2022 19:17:32 +0200 Subject: [PATCH 52/55] fix tests --- p2p/net/upgrader/listener_test.go | 1 + p2p/net/upgrader/upgrader_test.go | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index c8daef187a..464e4f6bec 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -363,6 +363,7 @@ func TestListenerResourceManagement(t *testing.T) { connScope := mocknetwork.NewMockConnManagementScope(ctrl) gomock.InOrder( rcmgr.EXPECT().OpenConnection(network.DirInbound, true).Return(connScope, nil), + connScope.EXPECT().PeerScope(), connScope.EXPECT().SetPeer(id), connScope.EXPECT().PeerScope(), ) diff --git a/p2p/net/upgrader/upgrader_test.go b/p2p/net/upgrader/upgrader_test.go index bf40438d09..38bd6c74a3 100644 --- a/p2p/net/upgrader/upgrader_test.go +++ b/p2p/net/upgrader/upgrader_test.go @@ -153,7 +153,11 @@ func TestOutboundResourceManagement(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() connScope := mocknetwork.NewMockConnManagementScope(ctrl) - connScope.EXPECT().PeerScope().Return(network.NullScope) + gomock.InOrder( + connScope.EXPECT().PeerScope(), + connScope.EXPECT().SetPeer(id), + connScope.EXPECT().PeerScope().Return(network.NullScope), + ) _, dialUpgrader := createUpgrader(t) conn, err := dial(t, dialUpgrader, ln.Multiaddr(), id, connScope) require.NoError(t, err) @@ -171,6 +175,8 @@ func TestOutboundResourceManagement(t *testing.T) { defer ctrl.Finish() connScope := mocknetwork.NewMockConnManagementScope(ctrl) gomock.InOrder( + connScope.EXPECT().PeerScope(), + connScope.EXPECT().SetPeer(id), connScope.EXPECT().PeerScope().Return(network.NullScope), connScope.EXPECT().Done(), ) From f75dfe12ff7b8f35baa0acc1e7ab6abd57ffc9de Mon Sep 17 00:00:00 2001 From: vyzo Date: Wed, 2 Feb 2022 19:28:36 +0200 Subject: [PATCH 53/55] more robust nil check --- p2p/net/upgrader/upgrader.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 3ea71bf777..8f15317324 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "net" + "reflect" "time" "github.com/libp2p/go-libp2p-core/connmgr" @@ -159,7 +160,7 @@ func (u *upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma } // Only call SetPeer if it hasn't already been set -- this can happen when we don't know // the peer in advance and in some bug scenarios. - if connScope.PeerScope() == nil { + if ps := connScope.PeerScope(); ps == nil || reflect.ValueOf(ps).IsNil() { if err := connScope.SetPeer(sconn.RemotePeer()); err != nil { log.Debugw("resource manager blocked connection for peer", "peer", sconn.RemotePeer(), "addr", conn.RemoteAddr(), "error", err) if err := maconn.Close(); err != nil { From 8350e75f17964c2e2d42b19853aee14de4c757e8 Mon Sep 17 00:00:00 2001 From: vyzo Date: Thu, 3 Feb 2022 08:56:03 +0200 Subject: [PATCH 54/55] avoid reflection --- p2p/net/upgrader/upgrader.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/p2p/net/upgrader/upgrader.go b/p2p/net/upgrader/upgrader.go index 8f15317324..3ea71bf777 100644 --- a/p2p/net/upgrader/upgrader.go +++ b/p2p/net/upgrader/upgrader.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "net" - "reflect" "time" "github.com/libp2p/go-libp2p-core/connmgr" @@ -160,7 +159,7 @@ func (u *upgrader) upgrade(ctx context.Context, t transport.Transport, maconn ma } // Only call SetPeer if it hasn't already been set -- this can happen when we don't know // the peer in advance and in some bug scenarios. - if ps := connScope.PeerScope(); ps == nil || reflect.ValueOf(ps).IsNil() { + if connScope.PeerScope() == nil { if err := connScope.SetPeer(sconn.RemotePeer()); err != nil { log.Debugw("resource manager blocked connection for peer", "peer", sconn.RemotePeer(), "addr", conn.RemoteAddr(), "error", err) if err := maconn.Close(); err != nil { From 9dc18edc262b0538686a283da70e2b80b45ea2c7 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Wed, 27 Apr 2022 00:35:35 +0200 Subject: [PATCH 55/55] switch from github.com/libp2p/go-libp2p-transport-upgrader to p2p/net/upgrader --- config/config.go | 3 +-- go.mod | 6 +++--- go.sum | 1 - p2p/net/swarm/dial_worker_test.go | 2 +- p2p/net/swarm/testing/testing.go | 2 +- p2p/net/upgrader/listener_test.go | 2 +- p2p/net/upgrader/upgrader_test.go | 8 ++++---- p2p/transport/tcp/tcp.go | 2 +- p2p/transport/tcp/tcp_test.go | 2 +- p2p/transport/websocket/websocket_test.go | 2 +- 10 files changed, 14 insertions(+), 16 deletions(-) diff --git a/config/config.go b/config/config.go index 0a68736f64..e1ce654562 100644 --- a/config/config.go +++ b/config/config.go @@ -24,12 +24,11 @@ import ( blankhost "github.com/libp2p/go-libp2p/p2p/host/blank" routed "github.com/libp2p/go-libp2p/p2p/host/routed" "github.com/libp2p/go-libp2p/p2p/net/swarm" + tptu "github.com/libp2p/go-libp2p/p2p/net/upgrader" circuitv2 "github.com/libp2p/go-libp2p/p2p/protocol/circuitv2/client" relayv2 "github.com/libp2p/go-libp2p/p2p/protocol/circuitv2/relay" "github.com/libp2p/go-libp2p/p2p/protocol/holepunch" - tptu "github.com/libp2p/go-libp2p-transport-upgrader" - logging "github.com/ipfs/go-log/v2" ma "github.com/multiformats/go-multiaddr" madns "github.com/multiformats/go-multiaddr-dns" diff --git a/go.mod b/go.mod index 524441497b..9ff84efedd 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/ipfs/go-datastore v0.5.1 github.com/ipfs/go-ipfs-util v0.0.2 github.com/ipfs/go-log/v2 v2.5.1 + github.com/jbenet/go-temp-err-catcher v0.1.0 github.com/klauspost/compress v1.15.1 github.com/libp2p/go-buffer-pool v0.0.2 github.com/libp2p/go-eventbus v0.2.1 @@ -22,10 +23,10 @@ require ( github.com/libp2p/go-libp2p-nat v0.1.0 github.com/libp2p/go-libp2p-noise v0.4.0 github.com/libp2p/go-libp2p-peerstore v0.6.0 + github.com/libp2p/go-libp2p-pnet v0.2.0 github.com/libp2p/go-libp2p-resource-manager v0.2.1 github.com/libp2p/go-libp2p-testing v0.9.2 github.com/libp2p/go-libp2p-tls v0.4.1 - github.com/libp2p/go-libp2p-transport-upgrader v0.7.1 github.com/libp2p/go-mplex v0.7.0 github.com/libp2p/go-msgio v0.2.0 github.com/libp2p/go-netroute v0.2.0 @@ -74,16 +75,15 @@ require ( github.com/google/uuid v1.3.0 // indirect github.com/huin/goupnp v1.0.3 // indirect github.com/jackpal/go-nat-pmp v1.0.2 // indirect - github.com/jbenet/go-temp-err-catcher v0.1.0 // indirect github.com/jbenet/goprocess v0.1.4 // indirect github.com/klauspost/cpuid/v2 v2.0.12 // indirect github.com/koron/go-ssdp v0.0.2 // indirect github.com/libp2p/go-cidranger v1.1.0 // indirect github.com/libp2p/go-flow-metrics v0.0.3 // indirect github.com/libp2p/go-libp2p-blankhost v0.3.0 // indirect - github.com/libp2p/go-libp2p-pnet v0.2.0 // indirect github.com/libp2p/go-libp2p-quic-transport v0.17.0 // indirect github.com/libp2p/go-libp2p-swarm v0.10.2 // indirect + github.com/libp2p/go-libp2p-transport-upgrader v0.7.1 // indirect github.com/libp2p/go-libp2p-yamux v0.9.1 // indirect github.com/libp2p/go-nat v0.1.0 // indirect github.com/libp2p/go-openssl v0.0.7 // indirect diff --git a/go.sum b/go.sum index 3de3c758a2..dc599a27ed 100644 --- a/go.sum +++ b/go.sum @@ -443,7 +443,6 @@ github.com/libp2p/go-libp2p-core v0.14.0/go.mod h1:tLasfcVdTXnixsLB0QYaT1syJOhsb github.com/libp2p/go-libp2p-core v0.15.1 h1:0RY+Mi/ARK9DgG1g9xVQLb8dDaaU8tCePMtGALEfBnM= github.com/libp2p/go-libp2p-core v0.15.1/go.mod h1:agSaboYM4hzB1cWekgVReqV5M4g5M+2eNNejV+1EEhs= github.com/libp2p/go-libp2p-mplex v0.4.1/go.mod h1:cmy+3GfqfM1PceHTLL7zQzAAYaryDu6iPSC+CIb094g= -github.com/libp2p/go-libp2p-mplex v0.5.0 h1:vt3k4E4HSND9XH4Z8rUpacPJFSAgLOv6HDvG8W9Ks9E= github.com/libp2p/go-libp2p-mplex v0.5.0/go.mod h1:eLImPJLkj3iG5t5lq68w3Vm5NAQ5BcKwrrb2VmOYb3M= github.com/libp2p/go-libp2p-nat v0.1.0 h1:vigUi2MEN+fwghe5ijpScxtbbDz+L/6y8XwlzYOJgSY= github.com/libp2p/go-libp2p-nat v0.1.0/go.mod h1:DQzAG+QbDYjN1/C3B6vXucLtz3u9rEonLVPtZVzQqks= diff --git a/p2p/net/swarm/dial_worker_test.go b/p2p/net/swarm/dial_worker_test.go index 0219a63a85..c404231270 100644 --- a/p2p/net/swarm/dial_worker_test.go +++ b/p2p/net/swarm/dial_worker_test.go @@ -10,6 +10,7 @@ import ( "github.com/libp2p/go-libp2p/p2p/muxer/yamux" csms "github.com/libp2p/go-libp2p/p2p/net/conn-security-multistream" + tptu "github.com/libp2p/go-libp2p/p2p/net/upgrader" quic "github.com/libp2p/go-libp2p/p2p/transport/quic" "github.com/libp2p/go-libp2p/p2p/transport/tcp" @@ -19,7 +20,6 @@ import ( "github.com/libp2p/go-libp2p-peerstore/pstoremem" tnet "github.com/libp2p/go-libp2p-testing/net" - tptu "github.com/libp2p/go-libp2p-transport-upgrader" msmux "github.com/libp2p/go-stream-muxer-multistream" ma "github.com/multiformats/go-multiaddr" diff --git a/p2p/net/swarm/testing/testing.go b/p2p/net/swarm/testing/testing.go index 6e56112f97..c1752dc871 100644 --- a/p2p/net/swarm/testing/testing.go +++ b/p2p/net/swarm/testing/testing.go @@ -7,6 +7,7 @@ import ( "github.com/libp2p/go-libp2p/p2p/muxer/yamux" csms "github.com/libp2p/go-libp2p/p2p/net/conn-security-multistream" "github.com/libp2p/go-libp2p/p2p/net/swarm" + tptu "github.com/libp2p/go-libp2p/p2p/net/upgrader" quic "github.com/libp2p/go-libp2p/p2p/transport/quic" "github.com/libp2p/go-libp2p/p2p/transport/tcp" @@ -22,7 +23,6 @@ import ( "github.com/libp2p/go-libp2p-peerstore/pstoremem" tnet "github.com/libp2p/go-libp2p-testing/net" - tptu "github.com/libp2p/go-libp2p-transport-upgrader" msmux "github.com/libp2p/go-stream-muxer-multistream" ma "github.com/multiformats/go-multiaddr" "github.com/stretchr/testify/require" diff --git a/p2p/net/upgrader/listener_test.go b/p2p/net/upgrader/listener_test.go index 464e4f6bec..1ea80d68dd 100644 --- a/p2p/net/upgrader/listener_test.go +++ b/p2p/net/upgrader/listener_test.go @@ -11,7 +11,7 @@ import ( "testing" "time" - upgrader "github.com/libp2p/go-libp2p-transport-upgrader" + "github.com/libp2p/go-libp2p/p2p/net/upgrader" "github.com/libp2p/go-libp2p-core/network" "github.com/libp2p/go-libp2p-core/peer" diff --git a/p2p/net/upgrader/upgrader_test.go b/p2p/net/upgrader/upgrader_test.go index 38bd6c74a3..be201eca60 100644 --- a/p2p/net/upgrader/upgrader_test.go +++ b/p2p/net/upgrader/upgrader_test.go @@ -6,7 +6,8 @@ import ( "net" "testing" - upgrader "github.com/libp2p/go-libp2p-transport-upgrader" + "github.com/libp2p/go-libp2p/p2p/muxer/yamux" + "github.com/libp2p/go-libp2p/p2p/net/upgrader" "github.com/libp2p/go-libp2p-core/crypto" "github.com/libp2p/go-libp2p-core/network" @@ -15,7 +16,6 @@ import ( "github.com/libp2p/go-libp2p-core/test" "github.com/libp2p/go-libp2p-core/transport" - mplex "github.com/libp2p/go-libp2p-mplex" mocknetwork "github.com/libp2p/go-libp2p-testing/mocks/network" "github.com/golang/mock/gomock" @@ -39,7 +39,7 @@ func createUpgraderWithMuxer(t *testing.T, muxer network.Multiplexer, opts ...up return id, u } -// negotiatingMuxer sets up a new mplex connection +// negotiatingMuxer sets up a new yamux connection // It makes sure that this happens at the same time for client and server. type negotiatingMuxer struct{} @@ -54,7 +54,7 @@ func (m *negotiatingMuxer) NewConn(c net.Conn, isServer bool, scope network.Peer if err != nil { return nil, err } - return mplex.DefaultTransport.NewConn(c, isServer, scope) + return yamux.DefaultTransport.NewConn(c, isServer, scope) } // blockingMuxer blocks the muxer negotiation until the contain chan is closed diff --git a/p2p/transport/tcp/tcp.go b/p2p/transport/tcp/tcp.go index 494d955e6c..7af1642261 100644 --- a/p2p/transport/tcp/tcp.go +++ b/p2p/transport/tcp/tcp.go @@ -86,7 +86,7 @@ func (ll *tcpListener) Accept() (manet.Conn, error) { tryKeepAlive(c, true) // We're not calling OpenConnection in the resource manager here, // since the manet.Conn doesn't allow us to save the scope. - // It's the caller's (usually the go-libp2p-transport-upgrader) responsibility + // It's the caller's (usually the p2p/net/upgrader) responsibility // to call the resource manager. return c, nil } diff --git a/p2p/transport/tcp/tcp_test.go b/p2p/transport/tcp/tcp_test.go index 3976d76fdc..06ea31b2e9 100644 --- a/p2p/transport/tcp/tcp_test.go +++ b/p2p/transport/tcp/tcp_test.go @@ -7,6 +7,7 @@ import ( "github.com/libp2p/go-libp2p/p2p/muxer/yamux" csms "github.com/libp2p/go-libp2p/p2p/net/conn-security-multistream" + tptu "github.com/libp2p/go-libp2p/p2p/net/upgrader" "github.com/libp2p/go-libp2p-core/crypto" "github.com/libp2p/go-libp2p-core/network" @@ -17,7 +18,6 @@ import ( mocknetwork "github.com/libp2p/go-libp2p-testing/mocks/network" ttransport "github.com/libp2p/go-libp2p-testing/suites/transport" - tptu "github.com/libp2p/go-libp2p-transport-upgrader" ma "github.com/multiformats/go-multiaddr" diff --git a/p2p/transport/websocket/websocket_test.go b/p2p/transport/websocket/websocket_test.go index db80277adb..b83f528f84 100644 --- a/p2p/transport/websocket/websocket_test.go +++ b/p2p/transport/websocket/websocket_test.go @@ -16,6 +16,7 @@ import ( "time" csms "github.com/libp2p/go-libp2p/p2p/net/conn-security-multistream" + tptu "github.com/libp2p/go-libp2p/p2p/net/upgrader" "github.com/libp2p/go-libp2p-core/crypto" "github.com/libp2p/go-libp2p-core/network" @@ -27,7 +28,6 @@ import ( "github.com/libp2p/go-libp2p/p2p/muxer/yamux" ttransport "github.com/libp2p/go-libp2p-testing/suites/transport" - tptu "github.com/libp2p/go-libp2p-transport-upgrader" ma "github.com/multiformats/go-multiaddr" "github.com/stretchr/testify/require"