-
Notifications
You must be signed in to change notification settings - Fork 50
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
Conversation
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.
a05ac5f
to
98ae09a
Compare
@@ -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() { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌
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 theirBinary
value, i.e. their densest possible encoding, is equal. Add tests that assert the issue is fixed and works with existing link representations.