Skip to content

Commit

Permalink
Merge pull request #682 from libp2p/fix/key-log-encoding
Browse files Browse the repository at this point in the history
Fix key log encoding
  • Loading branch information
petar authored Aug 14, 2020
2 parents fc3558c + b213268 commit 0e93285
Show file tree
Hide file tree
Showing 9 changed files with 201 additions and 115 deletions.
10 changes: 5 additions & 5 deletions dht.go
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,7 @@ func (dht *IpfsDHT) putValueToPeer(ctx context.Context, p peer.ID, rec *recpb.Re
pmes.Record = rec
rpmes, err := dht.sendRequest(ctx, p, pmes)
if err != nil {
logger.Debugw("failed to put value to peer", "to", p, "key", loggableKeyBytes(rec.Key), "error", err)
logger.Debugw("failed to put value to peer", "to", p, "key", loggableRecordKeyBytes(rec.Key), "error", err)
return err
}

Expand Down Expand Up @@ -526,17 +526,17 @@ func (dht *IpfsDHT) getValueSingle(ctx context.Context, p peer.ID, key string) (

// getLocal attempts to retrieve the value from the datastore
func (dht *IpfsDHT) getLocal(key string) (*recpb.Record, error) {
logger.Debugw("finding value in datastore", "key", loggableKeyString(key))
logger.Debugw("finding value in datastore", "key", loggableRecordKeyString(key))

rec, err := dht.getRecordFromDatastore(mkDsKey(key))
if err != nil {
logger.Warnw("get local failed", "key", key, "error", err)
logger.Warnw("get local failed", "key", loggableRecordKeyString(key), "error", err)
return nil, err
}

// Double check the key. Can't hurt.
if rec != nil && string(rec.GetKey()) != key {
logger.Errorw("BUG: found a DHT record that didn't match it's key", "expected", key, "got", rec.GetKey())
logger.Errorw("BUG: found a DHT record that didn't match it's key", "expected", loggableRecordKeyString(key), "got", rec.GetKey())
return nil, nil

}
Expand All @@ -547,7 +547,7 @@ func (dht *IpfsDHT) getLocal(key string) (*recpb.Record, error) {
func (dht *IpfsDHT) putLocal(key string, rec *recpb.Record) error {
data, err := proto.Marshal(rec)
if err != nil {
logger.Warnw("failed to put marshal record for local put", "error", err, "key", key)
logger.Warnw("failed to put marshal record for local put", "error", err, "key", loggableRecordKeyString(key))
return err
}

Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ require (
github.com/multiformats/go-base32 v0.0.3
github.com/multiformats/go-multiaddr v0.2.2
github.com/multiformats/go-multiaddr-net v0.1.5
github.com/multiformats/go-multibase v0.0.3
github.com/multiformats/go-multihash v0.0.14
github.com/multiformats/go-multistream v0.1.2
github.com/stretchr/testify v1.6.1
Expand Down
8 changes: 4 additions & 4 deletions handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func (dht *IpfsDHT) handlePutValue(ctx context.Context, p peer.ID, pmes *pb.Mess

// Make sure the record is valid (not expired, valid signature etc)
if err = dht.Validator.Validate(string(rec.GetKey()), rec.GetValue()); err != nil {
logger.Infow("bad dht record in PUT", "from", p, "key", rec.GetKey(), "error", err)
logger.Infow("bad dht record in PUT", "from", p, "key", loggableRecordKeyBytes(rec.GetKey()), "error", err)
return nil, err
}

Expand Down Expand Up @@ -196,11 +196,11 @@ func (dht *IpfsDHT) handlePutValue(ctx context.Context, p peer.ID, pmes *pb.Mess
recs := [][]byte{rec.GetValue(), existing.GetValue()}
i, err := dht.Validator.Select(string(rec.GetKey()), recs)
if err != nil {
logger.Warnw("dht record passed validation but failed select", "from", p, "key", rec.GetKey(), "error", err)
logger.Warnw("dht record passed validation but failed select", "from", p, "key", loggableRecordKeyBytes(rec.GetKey()), "error", err)
return nil, err
}
if i != 0 {
logger.Infow("DHT record in PUT older than existing record (ignoring)", "peer", p, "key", rec.GetKey())
logger.Infow("DHT record in PUT older than existing record (ignoring)", "peer", p, "key", loggableRecordKeyBytes(rec.GetKey()))
return nil, errors.New("old record")
}
}
Expand Down Expand Up @@ -344,7 +344,7 @@ func (dht *IpfsDHT) handleAddProvider(ctx context.Context, p peer.ID, pmes *pb.M
return nil, fmt.Errorf("handleAddProvider key is empty")
}

logger.Debugf("adding provider", "from", p, "key", key)
logger.Debugf("adding provider", "from", p, "key", loggableProviderRecordBytes(key))

// add provider should use the address given in the message
pinfos := pb.PBPeersToPeerInfos(pmes.GetProviderPeers())
Expand Down
92 changes: 92 additions & 0 deletions logging.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package dht

import (
"fmt"
"strings"

"github.com/ipfs/go-cid"
"github.com/multiformats/go-multibase"
"github.com/multiformats/go-multihash"
)

func multibaseB32Encode(k []byte) string {
res, err := multibase.Encode(multibase.Base32, k)
if err != nil {
// Should be unreachable
panic(err)
}
return res
}

func tryFormatLoggableRecordKey(k string) (string, error) {
if len(k) == 0 {
return "", fmt.Errorf("loggableRecordKey is empty")
}
var proto, cstr string
if k[0] == '/' {
// it's a path (probably)
protoEnd := strings.IndexByte(k[1:], '/')
if protoEnd < 0 {
return "", fmt.Errorf("loggableRecordKey starts with '/' but is not a path: %s", multibaseB32Encode([]byte(k)))
}
proto = k[1 : protoEnd+1]
cstr = k[protoEnd+2:]

encStr := multibaseB32Encode([]byte(cstr))
return fmt.Sprintf("/%s/%s", proto, encStr), nil
}

return "", fmt.Errorf("loggableRecordKey is not a path: %s", multibaseB32Encode([]byte(cstr)))
}

type loggableRecordKeyString string

func (lk loggableRecordKeyString) String() string {
k := string(lk)
newKey, err := tryFormatLoggableRecordKey(k)
if err == nil {
return newKey
}
return err.Error()
}

type loggableRecordKeyBytes []byte

func (lk loggableRecordKeyBytes) String() string {
k := string(lk)
newKey, err := tryFormatLoggableRecordKey(k)
if err == nil {
return newKey
}
return err.Error()
}

type loggableProviderRecordBytes []byte

func (lk loggableProviderRecordBytes) String() string {
newKey, err := tryFormatLoggableProviderKey(lk)
if err == nil {
return newKey
}
return err.Error()
}

func tryFormatLoggableProviderKey(k []byte) (string, error) {
if len(k) == 0 {
return "", fmt.Errorf("loggableProviderKey is empty")
}

encodedKey := multibaseB32Encode(k)

// The DHT used to provide CIDs, but now provides multihashes
// TODO: Drop this when enough of the network has upgraded
if _, err := cid.Cast(k); err == nil {
return encodedKey, nil
}

if _, err := multihash.Cast(k); err == nil {
return encodedKey, nil
}

return "", fmt.Errorf("loggableProviderKey is not a Multihash or CID: %s", encodedKey)
}
76 changes: 76 additions & 0 deletions logging_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package dht

import (
"testing"

cid "github.com/ipfs/go-cid"
)

func TestLoggableRecordKey(t *testing.T) {
c, err := cid.Decode("QmfUvYQhL2GinafMbPDYz7VFoZv4iiuLuR33aRsPurXGag")
if err != nil {
t.Fatal(err)
}

k, err := tryFormatLoggableRecordKey("/proto/" + string(c.Bytes()))
if err != nil {
t.Errorf("failed to format key: %s", err)
}
if k != "/proto/"+multibaseB32Encode(c.Bytes()) {
t.Error("expected path to be preserved as a loggable key")
}

for _, s := range []string{"/bla", "", "bla bla"} {
if _, err := tryFormatLoggableRecordKey(s); err == nil {
t.Errorf("expected to fail formatting: %s", s)
}
}

for _, s := range []string{"/bla/asdf", "/a/b/c"} {
if _, err := tryFormatLoggableRecordKey(s); err != nil {
t.Errorf("expected to be formatable: %s", s)
}
}
}

func TestLoggableProviderKey(t *testing.T) {
c0, err := cid.Decode("QmfUvYQhL2GinafMbPDYz7VFoZv4iiuLuR33aRsPurXGag")
if err != nil {
t.Fatal(err)
}

// Test logging CIDv0 provider
b32MH := multibaseB32Encode(c0.Hash())
k, err := tryFormatLoggableProviderKey(c0.Bytes())
if err != nil {
t.Errorf("failed to format key: %s", err)
}
if k != b32MH {
t.Error("expected cidv0 to be converted into base32 multihash")
}

// Test logging CIDv1 provider (from older DHT implementations)
c1 := cid.NewCidV1(cid.DagProtobuf, c0.Hash())
k, err = tryFormatLoggableProviderKey(c1.Hash())
if err != nil {
t.Errorf("failed to format key: %s", err)
}
if k != b32MH {
t.Error("expected cidv1 to be converted into base32 multihash")
}

// Test logging multihash provider
k, err = tryFormatLoggableProviderKey(c1.Hash())
if err != nil {
t.Errorf("failed to format key: %s", err)
}
if k != b32MH {
t.Error("expected multihash to be displayed in base32")
}

for _, s := range []string{"/bla", "", "bla bla", "/bla/asdf", "/a/b/c"} {
if _, err := tryFormatLoggableProviderKey([]byte(s)); err == nil {
t.Errorf("expected to fail formatting: %s", s)
}
}
}
54 changes: 0 additions & 54 deletions lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,69 +3,15 @@ package dht
import (
"context"
"fmt"
"strings"
"time"

"github.com/libp2p/go-libp2p-core/peer"
"github.com/libp2p/go-libp2p-core/routing"

"github.com/ipfs/go-cid"
pb "github.com/libp2p/go-libp2p-kad-dht/pb"
kb "github.com/libp2p/go-libp2p-kbucket"
"github.com/multiformats/go-base32"
)

func tryFormatLoggableKey(k string) (string, error) {
if len(k) == 0 {
return "", fmt.Errorf("loggableKey is empty")
}
var proto, cstr string
if k[0] == '/' {
// it's a path (probably)
protoEnd := strings.IndexByte(k[1:], '/')
if protoEnd < 0 {
return k, fmt.Errorf("loggableKey starts with '/' but is not a path: %x", k)
}
proto = k[1 : protoEnd+1]
cstr = k[protoEnd+2:]
} else {
proto = "provider"
cstr = k
}

var encStr string
c, err := cid.Cast([]byte(cstr))
if err == nil {
encStr = c.String()
} else {
encStr = base32.RawStdEncoding.EncodeToString([]byte(cstr))
}

return fmt.Sprintf("/%s/%s", proto, encStr), nil
}

type loggableKeyBytes []byte

func (lk loggableKeyString) String() string {
k := string(lk)
newKey, err := tryFormatLoggableKey(k)
if err == nil {
return newKey
}
return k
}

type loggableKeyString string

func (lk loggableKeyBytes) String() string {
k := string(lk)
newKey, err := tryFormatLoggableKey(k)
if err == nil {
return newKey
}
return k
}

// GetClosestPeers is a Kademlia 'node lookup' operation. Returns a channel of
// the K closest peers to the given key.
//
Expand Down
42 changes: 0 additions & 42 deletions lookup_test.go

This file was deleted.

Loading

0 comments on commit 0e93285

Please sign in to comment.