Skip to content

Commit

Permalink
Ignore error if can't write back echoed protocol in negotiate (#87)
Browse files Browse the repository at this point in the history
* Ignore error if can't write back echoed protocol

* Add test to verify we negotiate even if peer closes the stream after sending data

* Rework comment
  • Loading branch information
MarcoPolo authored Jun 21, 2022
1 parent 2a41ec3 commit b2d350b
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 3 deletions.
7 changes: 4 additions & 3 deletions multistream.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,10 @@ loop:
continue loop
}

if err := delimWriteBuffered(rwc, []byte(tok)); err != nil {
return "", nil, err
}
// Ignore the error here. We want the handshake to finish, even if the
// other side has closed this rwc for writing. They may have sent us a
// message and closed. Future writers will get an error anyways.
_ = delimWriteBuffered(rwc, []byte(tok))

// hand off processing to the sub-protocol handler
return tok, h.Handle, nil
Expand Down
75 changes: 75 additions & 0 deletions multistream_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,81 @@ func TestNegotiateFail(t *testing.T) {
}
}

type mockStream struct {
expectWrite [][]byte
toRead [][]byte
}

func (s *mockStream) Close() error {
return nil
}

func (s *mockStream) Write(p []byte) (n int, err error) {
if len(s.expectWrite) == 0 {
return 0, fmt.Errorf("no more writes expected")
}

if !bytes.Equal(s.expectWrite[0], p) {
return 0, fmt.Errorf("unexpected write")
}

s.expectWrite = s.expectWrite[1:]
return len(p), nil
}

func (s *mockStream) Read(p []byte) (n int, err error) {
if len(s.toRead) == 0 {
return 0, fmt.Errorf("no more reads expected")
}

if len(p) < len(s.toRead[0]) {
copy(p, s.toRead[0])
s.toRead[0] = s.toRead[0][len(p):]
n = len(p)
} else {
copy(p, s.toRead[0])
n = len(s.toRead[0])
s.toRead = s.toRead[1:]
}

return n, nil
}

func TestNegotiatePeerSendsAndCloses(t *testing.T) {
// Tests the case where a peer will negotiate a protocol, send data, then close the stream immediately
var buf bytes.Buffer
err := delimWrite(&buf, []byte(ProtocolID))
if err != nil {
t.Fatal(err)
}
delimtedProtocolID := make([]byte, buf.Len())
copy(delimtedProtocolID, buf.Bytes())

err = delimWrite(&buf, []byte("foo"))
if err != nil {
t.Fatal(err)
}
err = delimWrite(&buf, []byte("somedata"))
if err != nil {
t.Fatal(err)
}

s := &mockStream{
// We mock the closed stream by only expecting a single write. The
// mockstream will error on any more writes (same as writing to a closed
// stream)
expectWrite: [][]byte{delimtedProtocolID},
toRead: [][]byte{buf.Bytes()},
}

mux := NewMultistreamMuxer()
mux.AddHandler("foo", nil)
_, _, err = mux.Negotiate(s)
if err != nil {
t.Fatal("Negotiate should not fail here", err)
}
}

func TestSimopenClientServer(t *testing.T) {
a, b := newPipe(t)

Expand Down

0 comments on commit b2d350b

Please sign in to comment.