[I am not sure the best place to post this as this issue is related to both go-ipfs and go-datastore, I can move this to this to go-datastore if necessary.]
I may be completely missing something here but it seams datastore keys are represented via binary []byte strings, possible prefixed with a "/" and are converted to a datastore.Key by calling datastore.NewKey().
However, datastore.NewKey() also calls path.Clean(). This is a dangerous thing to do with binary strings. For example the binary []byte representation of the hash might just happen to contain a "//" which path.Clean will convert to a "/". In fact it seams that all of datastore.Key is written with the assumption the keys are text and not binary blobs.
Here is the relevant code in go-datastore/key.go:
// NewKey constructs a key from string. it will clean the value.
func NewKey(s string) Key {
k := Key{s}
k.Clean()
return k
}
// Clean up a Key, using path.Clean.
func (k *Key) Clean() {
k.string = path.Clean("/" + k.string)
}
The probability of a []byte string for a multihash containing something that path.Clean() will remove or change is low, which is why I image this issue has not come up yet.
I am not sure the best way to fix this. Maybe it might be better to keep the mutihash in its b58 encoding to mesh better with how datastore.Key was (supposedly) designed. Or as a quick fix, maybe it would be better to remove the call to path.Clean in the datastore.Key.Clean() method.
[I am not sure the best place to post this as this issue is related to both go-ipfs and go-datastore, I can move this to this to go-datastore if necessary.]
I may be completely missing something here but it seams datastore keys are represented via binary
[]bytestrings, possible prefixed with a "/" and are converted to adatastore.Keyby callingdatastore.NewKey().However,
datastore.NewKey()also callspath.Clean(). This is a dangerous thing to do with binary strings. For example the binary[]byterepresentation of the hash might just happen to contain a "//" whichpath.Cleanwill convert to a "/". In fact it seams that all ofdatastore.Keyis written with the assumption the keys are text and not binary blobs.Here is the relevant code in go-datastore/key.go:
The probability of a
[]bytestring for a multihash containing something thatpath.Clean()will remove or change is low, which is why I image this issue has not come up yet.I am not sure the best way to fix this. Maybe it might be better to keep the mutihash in its b58 encoding to mesh better with how
datastore.Keywas (supposedly) designed. Or as a quick fix, maybe it would be better to remove the call topath.Cleanin thedatastore.Key.Clean()method.