Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

forbid empty multiaddrs #105

Merged
merged 2 commits into from
May 20, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ func stringToBytes(s string) ([]byte, error) {
// consume first empty elem
sp = sp[1:]

if len(sp) == 0 {
return nil, fmt.Errorf("failed to parse multiaddr %q: empty multiaddr", s)
}

for len(sp) > 0 {
name := sp[0]
p := ProtocolWithName(name)
Expand Down Expand Up @@ -58,6 +62,9 @@ func stringToBytes(s string) ([]byte, error) {
}

func validateBytes(b []byte) (err error) {
if len(b) == 0 {
return fmt.Errorf("empty multiaddr")
}
for len(b) > 0 {
code, n, err := ReadVarintCode(b)
if err != nil {
Expand Down Expand Up @@ -136,6 +143,9 @@ func readComponent(b []byte) (int, Component, error) {
}

func bytesToString(b []byte) (ret string, err error) {
if len(b) == 0 {
return "", fmt.Errorf("empty multiaddr")
}
var buf strings.Builder

for len(b) > 0 {
Expand Down
4 changes: 4 additions & 0 deletions multiaddr.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,10 @@ func (m *multiaddr) Decapsulate(o Multiaddr) Multiaddr {
return &multiaddr{bytes: cpy}
}

if i == 0 {
return nil
}

ma, err := NewMultiaddr(s1[:i])
if err != nil {
panic("Multiaddr.Decapsulate incorrect byte boundaries.")
Expand Down
16 changes: 8 additions & 8 deletions multiaddr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -377,8 +377,8 @@ func TestEncapsulate(t *testing.T) {

m4, _ := NewMultiaddr("/ip4/127.0.0.1")
d := c.Decapsulate(m4)
if s := d.String(); s != "" {
t.Error("decapsulate /ip4 failed.", "/", s)
if d != nil {
t.Error("decapsulate /ip4 failed: ", d)
}
}

Expand Down Expand Up @@ -582,11 +582,11 @@ func TestBinaryMarshaler(t *testing.T) {
t.Fatal(err)
}

addr2 := newMultiaddr(t, "")
var addr2 multiaddr
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the issue I was talking about. This could be done by manually calling NewMultiaddr before but now one needs to construct a valid multiaddr.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but would work by creating a non-empty multiaddress (which just will be replaced) ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I guess. Although... multiaddrs are really supposed to be immutable. I guess this is just an unavoidable edge-case.

if err = addr2.UnmarshalBinary(b); err != nil {
t.Fatal(err)
}
if !addr.Equal(addr2) {
if !addr.Equal(&addr2) {
t.Error("expected equal addresses in circular marshaling test")
}
}
Expand All @@ -598,11 +598,11 @@ func TestTextMarshaler(t *testing.T) {
t.Fatal(err)
}

addr2 := newMultiaddr(t, "")
var addr2 multiaddr
if err = addr2.UnmarshalText(b); err != nil {
t.Fatal(err)
}
if !addr.Equal(addr2) {
if !addr.Equal(&addr2) {
t.Error("expected equal addresses in circular marshaling test")
}
}
Expand All @@ -614,11 +614,11 @@ func TestJSONMarshaler(t *testing.T) {
t.Fatal(err)
}

addr2 := newMultiaddr(t, "")
var addr2 multiaddr
if err = addr2.UnmarshalJSON(b); err != nil {
t.Fatal(err)
}
if !addr.Equal(addr2) {
if !addr.Equal(&addr2) {
t.Error("expected equal addresses in circular marshaling test")
}
}
Expand Down
1 change: 0 additions & 1 deletion util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ func TestSplitFirstLast(t *testing.T) {
[]string{ipStr, tcpStr, ipfsStr},
[]string{ipStr, tcpStr},
[]string{ipStr},
[]string{},
} {
addr := StringCast(strings.Join(x, ""))
head, tail := SplitFirst(addr)
Expand Down