Skip to content

Commit

Permalink
Fix hash mismatch error on matching link pointer
Browse files Browse the repository at this point in the history
On load, the default IPLD Link System checks the hash of loaded data
against the given hash to assure that they match. The actual hash is
calculated using the given link's `LinkPrototype.BuildLink`, and then is
directly compared with the given link via `==`.

Because the comparison is done by value, if the given link is a pointer
the comparison fails which will result in false-positive hash mismatch
error.

To address this issue, instead of directly comparing objects using `==`
check that their `Binary` value, i.e. their densest possible encoding,
is equal. Add tests that assert the issue is fixed and works with
existing link representations.
  • Loading branch information
masih committed Dec 5, 2022
1 parent 61c9ab1 commit 98ae09a
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 4 deletions.
2 changes: 1 addition & 1 deletion datamodel/link.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ type Link interface {
// the golang string type is used for immutability and for ease of use as a map key.
// As with the String method, the returned value may not elide any parts of the hash.
//
// Note that there is still no contract that the returned value be able to be parsed back into a Link value;
// Note that there is still no contract that the returned value should be parsable back into a Link value;
// not even in the case of `lnk.Prototype().BuildLink(lnk.Binary()[:])`.
// This is because the value returned by this method may contain data that the LinkPrototype would also restate.
// (For a concrete example: if using CIDs, this method will return a binary string that includes
Expand Down
6 changes: 3 additions & 3 deletions linking/functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
// This file contains all the functions on LinkSystem.
// These are the helpful, user-facing functions we expect folks to use "most of the time" when loading and storing data.

// Varations:
// Variations:
// - Load vs Store vs ComputeLink
// - Load vs LoadPlusRaw
// - With or without LinkContext?
Expand Down Expand Up @@ -141,7 +141,7 @@ func (lsys *LinkSystem) LoadRaw(lnkCtx LinkContext, lnk datamodel.Link) ([]byte,
hasher.Write(buf.Bytes())
hash := hasher.Sum(nil)
lnk2 := lnk.Prototype().BuildLink(hash)
if lnk2 != lnk {
if lnk2.Binary() != lnk.Binary() {
return nil, ErrHashMismatch{Actual: lnk2, Expected: lnk}
}
// No codec to deploy; this is the raw load function.
Expand Down Expand Up @@ -205,7 +205,7 @@ func (lsys *LinkSystem) Fill(lnkCtx LinkContext, lnk datamodel.Link, na datamode
// (Then do a bit of a jig to build a link out of it -- because that's what we do the actual hash equality check on.)
hash := hasher.Sum(nil)
lnk2 := lnk.Prototype().BuildLink(hash)
if lnk2 != lnk {
if lnk2.Binary() != lnk.Binary() {
return ErrHashMismatch{Actual: lnk2, Expected: lnk}
}
// If we got all the way through IO and through the hash check:
Expand Down
73 changes: 73 additions & 0 deletions linking/functions_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
package linking_test

import (
"bytes"
"context"
"testing"

qt "github.com/frankban/quicktest"
"github.com/ipfs/go-cid"
"github.com/ipld/go-ipld-prime"
"github.com/ipld/go-ipld-prime/codec/dagcbor"
"github.com/ipld/go-ipld-prime/datamodel"
"github.com/ipld/go-ipld-prime/fluent"
cidlink "github.com/ipld/go-ipld-prime/linking/cid"
"github.com/ipld/go-ipld-prime/node/basicnode"
"github.com/ipld/go-ipld-prime/storage/memstore"
"github.com/multiformats/go-multicodec"
)

func TestLinkSystem_LoadHashMismatch(t *testing.T) {
subject := cidlink.DefaultLinkSystem()
storage := &memstore.Store{}
subject.SetReadStorage(storage)
subject.SetWriteStorage(storage)

// Construct some test IPLD node.
wantNode := fluent.MustBuildMap(basicnode.Prototype.Map, 1, func(na fluent.MapAssembler) {
na.AssembleEntry("fish").AssignString("barreleye")
})

// Encode as raw value to be used for testing LoadRaw
var buf bytes.Buffer
qt.Check(t, dagcbor.Encode(wantNode, &buf), qt.IsNil)
wantNodeRaw := buf.Bytes()

// Store the test IPLD node and get link back.
lctx := ipld.LinkContext{Ctx: context.TODO()}
gotLink, err := subject.Store(lctx, cidlink.LinkPrototype{
Prefix: cid.Prefix{
Version: 1,
Codec: uint64(multicodec.DagCbor),
MhType: uint64(multicodec.Sha2_256),
MhLength: -1,
},
}, wantNode)
qt.Check(t, err, qt.IsNil)
gotCidlink := gotLink.(cidlink.Link)

// Assert all load variations return expected values for different link representations.
for _, test := range []struct {
name string
link datamodel.Link
}{
{"datamodel.Link", gotLink},
{"cidlink.Link", gotCidlink},
{"&cidlink.Link", &gotCidlink},
} {
t.Run(test.name, func(t *testing.T) {
gotNode, err := subject.Load(lctx, test.link, basicnode.Prototype.Any)
qt.Check(t, err, qt.IsNil)
qt.Check(t, ipld.DeepEqual(wantNode, gotNode), qt.IsTrue)

gotNodeRaw, err := subject.LoadRaw(lctx, test.link)
qt.Check(t, err, qt.IsNil)
qt.Check(t, bytes.Equal(wantNodeRaw, gotNodeRaw), qt.IsTrue)

gotNode, gotNodeRaw, err = subject.LoadPlusRaw(lctx, test.link, basicnode.Prototype.Any)
qt.Check(t, err, qt.IsNil)
qt.Check(t, ipld.DeepEqual(wantNode, gotNode), qt.IsTrue)
qt.Check(t, bytes.Equal(wantNodeRaw, gotNodeRaw), qt.IsTrue)
})
}
}

0 comments on commit 98ae09a

Please sign in to comment.