Skip to content
This repository was archived by the owner on Feb 12, 2024. It is now read-only.

cli block get accidently mutates binary data #897

Closed
kumavis opened this issue Jul 4, 2017 · 9 comments
Closed

cli block get accidently mutates binary data #897

kumavis opened this issue Jul 4, 2017 · 9 comments
Assignees

Comments

@kumavis
Copy link
Contributor

kumavis commented Jul 4, 2017

seems ipfs block get is mutating the data a bit https://gist.github.com./kumavis/e454a7ea2e9b9520e66bc5ef3e739744
notice the last two bytes

@daviddias
Copy link
Member

Did you manage it is on block get or block put? You should be able to cat the block after block put directly from the repo.

@kumavis
Copy link
Contributor Author

kumavis commented Jul 4, 2017

pulling over bitswap

@kumavis
Copy link
Contributor Author

kumavis commented Jul 4, 2017

block is stored in parity's db, so block put actually never happens
hitting the bridge's api directly works
ipfs block get gives consistently incorrect data -- my guess is that its a side effect of logging to stdout

@kumavis
Copy link
Contributor Author

kumavis commented Jul 4, 2017

yup

 ✘ ⚙  user@work  ~/dev/metamask-mesh   master ±
 ⧖  node -e "console.log('\u0000')" | hexdump
0000000 0a00                                   
0000002

 ⚙  user@work  ~/dev/metamask-mesh   master ±
 ⧖  node -v
v8.1.0

@kumavis
Copy link
Contributor Author

kumavis commented Jul 4, 2017

compare to

 ⚙  user@work  ~/dev/metamask-mesh   master ±
 ⧖  node -e "process.stdout.write('\u0000')" | hexdump
0000000 0000                                   
0000001

@daviddias
Copy link
Member

We probably shouldn't be writing that \n - https://github.com./ipfs/js-ipfs/blob/master/src/cli/commands/block/get.js#L20-L21

That is why you see a difference between console.log and process.stdout.write, the first always writes a new line char

@kumavis
Copy link
Contributor Author

kumavis commented Jul 4, 2017

ah interesting, not what i was expecting

 ⚙  user@work  ~/dev/metamask   origin-header 
 ⧖  node -e "process.stdout.write('\u0000'),process.stdout.write('\n')" | hexdump
0000000 0a00                                   
0000002

@kumavis
Copy link
Contributor Author

kumavis commented Jul 4, 2017

if we like the newline, we could always check if the process isTTY or not, and only append the newline if it is

@daviddias
Copy link
Member

Let's continue to track this at #900.

@daviddias daviddias removed the status/in-progress In progress label Jul 7, 2017
MicrowaveDev pushed a commit to galtproject/js-ipfs that referenced this issue May 22, 2020
BREAKING CHANGE: the `ipfs-api` library has been renamed to `ipfs-http-client`.

Now install via `npm install ipfs-http-client`.

Note that in the browser build the object attached to `window` is now `window.IpfsHttpClient`.

License: MIT
Signed-off-by: Alan Shaw <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants