Skip to content
This repository has been archived by the owner on Sep 6, 2022. It is now read-only.

fix JSON deserialization of peer.AddrInfo #177

Closed
wants to merge 3 commits into from
Closed

fix JSON deserialization of peer.AddrInfo #177

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ee67b32
fix JSON deserialization of peer.AddrInfo
marten-seemann Feb 12, 2021
9f33b0e
add a test case for JSON encoding / decoding of AddrInfo
marten-seemann Feb 12, 2021
43d94f1
reject JSON serialized AddrInfo if the ID is missing
marten-seemann Feb 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 14 additions & 10 deletions peer/addrinfo_serde.go
View file
Open in desktop
Expand Up @@ -19,19 +19,23 @@ func (pi AddrInfo) MarshalJSON() ([]byte, error) {

func (pi *AddrInfo) UnmarshalJSON(b []byte) error {
var data map[string]interface{}
err := json.Unmarshal(b, &data)
if err != nil {
if err := json.Unmarshal(b, &data); err != nil {
return err
}
pid, err := IDB58Decode(data["ID"].(string))
if err != nil {
return err
if id, ok := data["ID"]; ok {
Copy link

Choose a reason for hiding this comment

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

Shouldn't this error if there's no ID? I'd expect addrInfo.UnmarshalJSON([]byte(`{"Addrs":[]}`)) or addrInfo.UnmarshalJSON([]byte(`{"foo":"bar"}`)) to error instead of returning nil. A test validating the bad path expectation would be great. No addresses is fine, but no ID isn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, added. PTAL.

if idString, ok := id.(string); ok {
pid, err := IDB58Decode(idString)
if err != nil {
return err
}
pi.ID = pid
}
}
pi.ID = pid
addrs, ok := data["Addrs"].([]interface{})
if ok {
for _, a := range addrs {
pi.Addrs = append(pi.Addrs, ma.StringCast(a.(string)))
if addrsEntry, ok := data["Addrs"]; ok {
if addrs, ok := addrsEntry.([]interface{}); ok {
for _, a := range addrs {
pi.Addrs = append(pi.Addrs, ma.StringCast(a.(string)))
Copy link

Choose a reason for hiding this comment

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

PR looks good, the only outstanding issue I see is that ma.StringCast will panic if the multiaddr is malformed or we don't know one of the codecs in the multiaddr. Given this might contain an unknown codec perhaps we should just ignore the particular addr and continue trying to unmarshal the others?

}
}
}
return nil
Expand Down
18 changes: 18 additions & 0 deletions peer/addrinfo_test.go
View file
Open in desktop
Expand Up @@ -133,3 +133,21 @@ func TestAddrInfosFromP2pAddrs(t *testing.T) {
delete(expected, info.ID.Pretty())
}
}

func TestAddrInfoJSON(t *testing.T) {
ai := AddrInfo{ID: testID, Addrs: []ma.Multiaddr{maddrFull}}
out, err := ai.MarshalJSON()
if err != nil {
t.Fatal(err)
}
var addrInfo AddrInfo
if err := addrInfo.UnmarshalJSON(out); err != nil {
t.Fatal(err)
}
if addrInfo.ID != testID {
t.Fatalf("expected ID to equal %s, got %s", testID.Pretty(), addrInfo.ID.Pretty())
}
if len(addrInfo.Addrs) != 1 || !addrInfo.Addrs[0].Equal(maddrFull) {
t.Fatalf("expected addrs to match %v, got %v", maddrFull, addrInfo.Addrs)
}
}