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

PeerInfo UnMarshal Error #393

Closed
Harrywang55666 opened this issue Aug 10, 2018 · 8 comments
Closed

PeerInfo UnMarshal Error #393

Harrywang55666 opened this issue Aug 10, 2018 · 8 comments
Labels
exp/novice Someone with a little familiarity can pick up good first issue Good issue for new contributors help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) kind/enhancement A net-new feature or improvement to an existing feature

Comments

@Harrywang55666
Copy link

Harrywang55666 commented Aug 10, 2018

package main

import (
	"github.com/libp2p/go-libp2p-peerstore"
	"github.com/libp2p/go-libp2p"
	"encoding/json"
	"context"
	"github.com/whyrusleeping/go-logging"
	"os"
)

func main() {
	log := logging.MustGetLogger("main")
	logging.SetBackend(logging.NewLogBackend(os.Stderr, "", 0))
	ctx := context.Background()
	host, err := libp2p.New(ctx, libp2p.ListenAddrStrings("/ip4/0.0.0.0/tcp/0"))
	if err != nil {
		log.Fatal("new host err:", err)
	}
	data, err := json.Marshal(host.Peerstore().PeerInfo(host.ID()))
	if err != nil {
		log.Fatal("marshal json err:", err)
	}

	log.Debugf("data: %s", data)

	var info peerstore.PeerInfo
	if err := json.Unmarshal(data, &info); err != nil {
		log.Fatal("unmarshal json err:", err)
	}

	log.Debug("info:", info)
	return
}

after running result:

17:07:16.353 DEBUG       main: data: {"ID":"\u0012 \ufffdr\ufffd\ufffd\ufffdpz\ufffd\ufffdS\u0016\u0007 `|\no\u000e\ufffd\ufffd\ufffd\ufffd4\ufffd\ufffd\ufffd\ufffd\u0016O\ufffd\u0011\ufffd","Addrs":null} main.go:25
17:07:16.353 CRITI       main: unmarshal json err:input isn't valid multihash main.go:29
@Stebalien Stebalien added the kind/enhancement A net-new feature or improvement to an existing feature label Aug 10, 2018
@Stebalien
Copy link
Member

We haven't defined a way to marshal/unmarshal peer IDs to/from JSON (although we probably should).

@bigs bigs added kind/bug A bug in existing code (including security flaws) status/deferred Conscious decision to pause or backlog labels Sep 18, 2018
@hsanjuan hsanjuan added the exp/novice Someone with a little familiarity can pick up label Oct 26, 2018
@hsanjuan
Copy link
Contributor

This is just adding MarshalJSON and UnmarshalJSON to the type.

@vyzo vyzo added help wanted Seeking public contribution on this issue good first issue Good issue for new contributors labels Oct 26, 2018
@zhizouxiao
Copy link
Contributor

I think it's time to close this issue, MarshalJSON and UnmarshalJSON have been added.

func (pi *PeerInfo) MarshalJSON() ([]byte, error) {
        out := make(map[string]interface{})
        out["ID"] = pi.ID.Pretty()
        var addrs []string
        for _, a := range pi.Addrs {
                addrs = append(addrs, a.String())
        }
        out["Addrs"] = addrs
        return json.Marshal(out)
}

func (pi *PeerInfo) UnmarshalJSON(b []byte) error {
        var data map[string]interface{}
        err := json.Unmarshal(b, &data)
        if err != nil {
                return err
        }
        pid, err := peer.IDB58Decode(data["ID"].(string))
        if err != nil {
                return err
        }
        pi.ID = pid
        addrs, ok := data["Addrs"].([]interface{})
        if ok {
                for _, a := range addrs {
                        pi.Addrs = append(pi.Addrs, ma.StringCast(a.(string)))
                }
        }
        return nil
}

@Stebalien
Copy link
Member

Hm. Actually, I was wrong. The code in the top post should work.

@zhizouxiao
Copy link
Contributor

@Stebalien I figure it out. The reason is (pi *PeerInfo) MarshalJSON() ([]byte, error) with a pointer receiver, so when a pointer is needed in json.Marshal. The code below has been tested.

pi := host.Peerstore().PeerInfo(host.ID())
data, err := json.Marshal(&pi)

@Stebalien
Copy link
Member

I see. We should just change it to take a value. Also, we should change it to not panic on bad values. The current code is buggy as hell.

@zhizouxiao
Copy link
Contributor

zhizouxiao commented Nov 8, 2018

@Stebalien I made a pull request. Now json.Marshal can take value or pointer, either is okay. But I can't pass the travis CI.

@zhizouxiao
Copy link
Contributor

@Harrywang55666 I think it's time to close this issue.

@ghost ghost removed the status/deferred Conscious decision to pause or backlog label Nov 9, 2018
marten-seemann pushed a commit that referenced this issue Aug 9, 2022
marten-seemann pushed a commit that referenced this issue Aug 9, 2022
marten-seemann pushed a commit that referenced this issue Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/novice Someone with a little familiarity can pick up good first issue Good issue for new contributors help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws) kind/enhancement A net-new feature or improvement to an existing feature
Projects
None yet
Development

No branches or pull requests

6 participants