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

Fix hash mismatch error on matching link pointer #480

Merged
merged 1 commit into from
Dec 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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() {
Copy link
Member

Choose a reason for hiding this comment

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

should this be a bytes.Equal check?

Copy link
Member Author

@masih masih Dec 6, 2022

Choose a reason for hiding this comment

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

How come?
.Binary() returns string.

Copy link
Member

Choose a reason for hiding this comment

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

hah, TIL bytes.Equal() is basically a string cmp: https://cs.opensource.google/go/go/+/refs/tags/go1.19.3:src/bytes/bytes.go;l=18-21
so I guess this is fine because we end up with the same thing

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)
})
}
}