{"action":"opened","issue":{"url":"https://api.github.com/repos/ipfs/go-ipfs/issues/2601","repository_url":"https://api.github.com/repos/ipfs/go-ipfs","labels_url":"https://api.github.com/repos/ipfs/go-ipfs/issues/2601/labels{/name}","comments_url":"https://api.github.com/repos/ipfs/go-ipfs/issues/2601/comments","events_url":"https://api.github.com/repos/ipfs/go-ipfs/issues/2601/events","html_url":"https://github.com/ipfs/go-ipfs/issues/2601","id":150718457,"number":2601,"title":"In datastote.NewKey calling path.Clean is dangerous","user":{"login":"kevina","id":1271492,"avatar_url":"https://avatars.githubusercontent.com/u/1271492?v=3","gravatar_id":"","url":"https://api.github.com/users/kevina","html_url":"https://github.com/kevina","followers_url":"https://api.github.com/users/kevina/followers","following_url":"https://api.github.com/users/kevina/following{/other_user}","gists_url":"https://api.github.com/users/kevina/gists{/gist_id}","starred_url":"https://api.github.com/users/kevina/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/kevina/subscriptions","organizations_url":"https://api.github.com/users/kevina/orgs","repos_url":"https://api.github.com/users/kevina/repos","events_url":"https://api.github.com/users/kevina/events{/privacy}","received_events_url":"https://api.github.com/users/kevina/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":0,"created_at":"2016-04-25T00:48:23Z","updated_at":"2016-04-25T00:48:23Z","closed_at":null,"body":"[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.]\r\n\r\nI 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.\r\n\r\nHowever, 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.NewKey is written with the assumption the keys are text and not binary blobs.\r\n\r\nHere is the relevant code in [go-datastore/key.go](https://github.com/ipfs/go-datastore/blob/545f590/key.go):\r\n```go\r\n// NewKey constructs a key from string. it will clean the value.\r\nfunc NewKey(s string) Key {\r\n\tk := Key{s}\r\n\tk.Clean()\r\n\treturn k\r\n}\r\n\r\n// Clean up a Key, using path.Clean.\r\nfunc (k *Key) Clean() {\r\n\tk.string = path.Clean(\"/\" + k.string)\r\n}\r\n```\r\n\r\nThe 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.\r\n\r\nI 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 designed.  Or as a quick fix, maybe it would be better to remove the call to path.Clean in the datastore.Key.Clean() method.\r\n"}}