Skip to content

refactor: keep same api as v0.14.0 for SplitFirst/SplitLast #271

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.

Sign up for GitHub

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

Merged
MarcoPolo merged 2 commits into from
Feb 21, 2025
+147 −42
Merged

refactor: keep same api as v0.14.0 for SplitFirst/SplitLast #271

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
235f2a0
refactor: keep same api for SplitFirst/SplitLast
MarcoPolo Feb 21, 2025
3bc911c
add defensive copy
MarcoPolo Feb 21, 2025
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
89 changes: 70 additions & 19 deletions component.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,39 @@ type Component struct {
valueStartIdx int // Index of the first byte of the Component's value in the bytes array
}

func (c Component) AsMultiaddr() Multiaddr {
func (c *Component) AsMultiaddr() Multiaddr {
if c.Empty() {
return nil
}
return []Component{c}
return []Component{*c}

@sukunrt sukunrt Feb 21, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we make these slices with a higher capacity?

@MarcoPolo MarcoPolo Feb 21, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's hard to predict how a user will use this. If they want a higher capacity they should just use the slice of Components directly

}

func (c Component) Encapsulate(o Multiaddr) Multiaddr {
func (c *Component) Encapsulate(o Multiaddr) Multiaddr {
return c.AsMultiaddr().Encapsulate(o)
}

func (c Component) Decapsulate(o Multiaddr) Multiaddr {
func (c *Component) Decapsulate(o Multiaddr) Multiaddr {
return c.AsMultiaddr().Decapsulate(o)
}

func (c Component) Empty() bool {
func (c *Component) Empty() bool {
if c == nil {
return true
}
return len(c.bytes) == 0
}

func (c Component) Bytes() []byte {
func (c *Component) Bytes() []byte {
if c == nil {
return nil
}
return []byte(c.bytes)
}

func (c Component) MarshalBinary() ([]byte, error) {
func (c *Component) MarshalBinary() ([]byte, error) {
if c == nil {
return nil, errNilPtr
}
return c.Bytes(), nil
}

Expand All @@ -58,7 +67,10 @@ func (c *Component) UnmarshalBinary(data []byte) error {
return nil
}

func (c Component) MarshalText() ([]byte, error) {
func (c *Component) MarshalText() ([]byte, error) {
if c == nil {
return nil, errNilPtr
}
return []byte(c.String()), nil
}

Expand All @@ -79,7 +91,10 @@ func (c *Component) UnmarshalText(data []byte) error {
return nil
}

func (c Component) MarshalJSON() ([]byte, error) {
func (c *Component) MarshalJSON() ([]byte, error) {
if c == nil {
return nil, errNilPtr
}
txt, err := c.MarshalText()
if err != nil {
return nil, err
Expand All @@ -101,22 +116,40 @@ func (c *Component) UnmarshalJSON(data []byte) error {
return c.UnmarshalText([]byte(v))
}

func (c Component) Equal(o Component) bool {
func (c *Component) Equal(o *Component) bool {
if c == nil || o == nil {
return c == o
}
return c.bytes == o.bytes
}

func (c Component) Compare(o Component) int {
func (c *Component) Compare(o *Component) int {
if c == nil && o == nil {
return 0
}
if c == nil {
return -1
}
if o == nil {
return 1
}
return strings.Compare(c.bytes, o.bytes)
}

func (c Component) Protocols() []Protocol {
func (c *Component) Protocols() []Protocol {
if c == nil {
return nil
}
if c.protocol == nil {
return nil
}
return []Protocol{*c.protocol}
}

func (c Component) ValueForProtocol(code int) (string, error) {
func (c *Component) ValueForProtocol(code int) (string, error) {
if c == nil {
return "", fmt.Errorf("component is nil")
}
if c.protocol == nil {
return "", fmt.Errorf("component has nil protocol")
}
Expand All @@ -126,18 +159,27 @@ func (c Component) ValueForProtocol(code int) (string, error) {
return c.Value(), nil
}

func (c Component) Protocol() Protocol {
func (c *Component) Protocol() Protocol {
if c == nil {
return Protocol{}
}
if c.protocol == nil {
return Protocol{}
}
return *c.protocol
}

func (c Component) RawValue() []byte {
func (c *Component) RawValue() []byte {
if c == nil {
return nil
}
return []byte(c.bytes[c.valueStartIdx:])
}

func (c Component) Value() string {
func (c *Component) Value() string {
if c == nil {
return ""
}
if c.Empty() {
return ""
}
Expand All @@ -146,7 +188,10 @@ func (c Component) Value() string {
return value
}

func (c Component) valueAndErr() (string, error) {
func (c *Component) valueAndErr() (string, error) {
if c == nil {
return "", errNilPtr
}
if c.protocol == nil {
return "", fmt.Errorf("component has nil protocol")
}
Expand All @@ -160,15 +205,21 @@ func (c Component) valueAndErr() (string, error) {
return value, nil
}

func (c Component) String() string {
func (c *Component) String() string {
if c == nil {
return "<nil component>"
}
var b strings.Builder
c.writeTo(&b)
return b.String()
}

// writeTo is an efficient, private function for string-formatting a multiaddr.
// Trust me, we tend to allocate a lot when doing this.
func (c Component) writeTo(b *strings.Builder) {
func (c *Component) writeTo(b *strings.Builder) {
if c == nil {
return
}
if c.protocol == nil {
return
}
Expand Down
10 changes: 5 additions & 5 deletions multiaddr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (m Multiaddr) Equal(m2 Multiaddr) bool {
return false
}
for i, c := range m {
if !c.Equal(m2[i]) {
if !c.Equal(&m2[i]) {
return false
}
}
Expand All @@ -80,7 +80,7 @@ func (m Multiaddr) Equal(m2 Multiaddr) bool {

func (m Multiaddr) Compare(o Multiaddr) int {
for i := 0; i < len(m) && i < len(o); i++ {
if cmp := m[i].Compare(o[i]); cmp != 0 {
if cmp := m[i].Compare(&o[i]); cmp != 0 {
return cmp
}
}
Expand Down Expand Up @@ -177,13 +177,13 @@ func (m Multiaddr) Encapsulate(o Multiaddr) Multiaddr {
return Join(m, o)
}

func (m Multiaddr) EncapsulateC(c Component) Multiaddr {
func (m Multiaddr) EncapsulateC(c *Component) Multiaddr {
if c.Empty() {
return m
}
out := make([]Component, 0, len(m)+1)
out = append(out, m...)
out = append(out, c)
out = append(out, *c)
return out
}

Expand All @@ -200,7 +200,7 @@ func (m Multiaddr) Decapsulate(rightParts Multiaddr) Multiaddr {
break
}

foundMatch = rightC.Equal(leftParts[i+j])
foundMatch = rightC.Equal(&leftParts[i+j])
if !foundMatch {
break
}
Expand Down
50 changes: 45 additions & 5 deletions multiaddr_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ func TestReturnsNilOnEmpty(t *testing.T) {
a, _ = SplitLast(a)
require.Nil(t, a)

a, c := SplitLast(nil)
require.Zero(t, len(a.Protocols()))
require.Nil(t, a)
require.Nil(t, c)
require.True(t, c.Empty())

// Test that empty multiaddr from various operations returns nil
a = StringCast("/ip4/1.2.3.4/tcp/1234")
_, a = SplitFirst(a)
Expand All @@ -36,6 +42,11 @@ func TestReturnsNilOnEmpty(t *testing.T) {
_, a = SplitFirst(a)
require.Nil(t, a)

c, a = SplitFirst(nil)
require.Nil(t, a)
require.Nil(t, c)
require.True(t, c.Empty())

a = StringCast("/ip4/1.2.3.4/tcp/1234")
a = a.Decapsulate(a)
require.Nil(t, a)
Expand Down Expand Up @@ -400,7 +411,7 @@ func TestBytesSplitAndJoin(t *testing.T) {

for i, a := range split {
if a.String() != res[i] {
t.Errorf("split component failed: %s != %s", a, res[i])
t.Errorf("split component failed: %s != %s", &a, res[i])
}
}

Expand All @@ -411,7 +422,7 @@ func TestBytesSplitAndJoin(t *testing.T) {

for i, a := range split {
if a.String() != res[i] {
t.Errorf("split component failed: %s != %s", a, res[i])
t.Errorf("split component failed: %s != %s", &a, res[i])
}
}
}
Expand Down Expand Up @@ -863,7 +874,7 @@ func TestComponentBinaryMarshaler(t *testing.T) {
if err = comp2.UnmarshalBinary(b); err != nil {
t.Fatal(err)
}
if !comp.Equal(comp2) {
if !comp.Equal(&comp2) {
t.Error("expected equal components in circular marshaling test")
}
}
Expand All @@ -882,7 +893,7 @@ func TestComponentTextMarshaler(t *testing.T) {
if err = comp2.UnmarshalText(b); err != nil {
t.Fatal(err)
}
if !comp.Equal(comp2) {
if !comp.Equal(&comp2) {
t.Error("expected equal components in circular marshaling test")
}
}
Expand All @@ -901,7 +912,7 @@ func TestComponentJSONMarshaler(t *testing.T) {
if err = comp2.UnmarshalJSON(b); err != nil {
t.Fatal(err)
}
if !comp.Equal(comp2) {
if !comp.Equal(&comp2) {
t.Error("expected equal components in circular marshaling test")
}
}
Expand All @@ -914,6 +925,9 @@ func TestUseNil(t *testing.T) {
_ = f()

var foo Multiaddr = nil
_, right := SplitFirst(foo)
right.Protocols()
foo.Protocols()
foo.Bytes()
foo.Compare(nil)
foo.Decapsulate(nil)
Expand All @@ -930,6 +944,32 @@ func TestUseNil(t *testing.T) {
_, _ = foo.ValueForProtocol(0)
}

func TestUseNilComponent(t *testing.T) {
var foo *Component
foo.AsMultiaddr()
foo.Encapsulate(nil)
foo.Decapsulate(nil)
foo.Empty()
foo.Bytes()
foo.MarshalBinary()
foo.MarshalJSON()
foo.MarshalText()
foo.UnmarshalBinary(nil)
foo.UnmarshalJSON(nil)
foo.UnmarshalText(nil)
foo.Equal(nil)
foo.Compare(nil)
foo.Protocols()
foo.ValueForProtocol(0)
foo.Protocol()
foo.RawValue()
foo.Value()
_ = foo.String()

var m Multiaddr = nil
m.EncapsulateC(foo)
}

func TestFilterAddrs(t *testing.T) {
bad := []Multiaddr{
newMultiaddr(t, "/ip6/fe80::1/tcp/1234"),
Expand Down
16 changes: 8 additions & 8 deletions util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,26 +64,26 @@ func StringCast(s string) Multiaddr {
}

// SplitFirst returns the first component and the rest of the multiaddr.
func SplitFirst(m Multiaddr) (Component, Multiaddr) {
func SplitFirst(m Multiaddr) (*Component, Multiaddr) {
if m.Empty() {
return Component{}, nil
return nil, nil
}
if len(m) == 1 {
return m[0], nil
return &m[0], nil
}
return m[0], m[1:]
return &m[0], m[1:]
}

// SplitLast returns the rest of the multiaddr and the last component.
func SplitLast(m Multiaddr) (Multiaddr, Component) {
func SplitLast(m Multiaddr) (Multiaddr, *Component) {
if m.Empty() {
return nil, Component{}
return nil, nil
}
if len(m) == 1 {
// We want to explicitly return a nil slice if the prefix is now empty.
return nil, m[0]
return nil, &m[0]
}
return m[:len(m)-1], m[len(m)-1]
return m[:len(m)-1], &m[len(m)-1]

@sukunrt sukunrt Feb 21, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks unsafe.

last points to the last element in the slice. So appends to the first bit(the multiaddr) will modify last, right?

@sukunrt sukunrt Feb 21, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The same argument also applies to SplitFunc

@MarcoPolo MarcoPolo Feb 21, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a defensive copy

}

// SplitFunc splits the multiaddr when the callback first returns true. The
Expand Down
Loading