Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

Fix for empty data in block. #789

Merged
merged 3 commits into from
Jun 25, 2018
Merged

Fix for empty data in block. #789

merged 3 commits into from
Jun 25, 2018

Conversation

mikeal
Copy link
Contributor

@mikeal mikeal commented Jun 20, 2018

Found this very strange bug when creating DAGNode's with empty buffers for data.

streamToValue returns an empty array when the data is empty, which down the line triggers an exception in ipfs-block because this sends an empty array instead of a buffer.

Found this very strange bug when creating DAGNode's with empty buffers for data.

streamToValue returns an empty array when the data is empty, which down the line triggers an exception in ipfs-block because this sends an empty array instead of a buffer.
@alanshaw
Copy link
Contributor

Thanks @mikeal, do you have a sec to add a test for this to interface-ipfs-core?

@mikeal
Copy link
Contributor Author

mikeal commented Jun 20, 2018

Hrm... that will take some time. There's a lot of test framework and fixtures to figure out in those tests that I have no experience with. I can do it, but I won't find the time for a while.

alanshaw added a commit to ipfs/js-ipfs that referenced this pull request Jun 21, 2018
This PR allows the test in ipfs-inactive/interface-js-ipfs-core#308 to pass.

X-Stream-Output header is added to block.get reply for parity with go-ipfs.

block.put is altered to allow an empty block to be put (again for fetaure parity with go-ipfs) but only if there is a multipart file part for it. Error responses have also been improved.

refs ipfs-inactive/js-ipfs-http-client#789
tested by ipfs-inactive/interface-js-ipfs-core#308

License: MIT
Signed-off-by: Alan Shaw <[email protected]>
js-ipfs currently does not set the X-Stream-Output header on block.get responses so we also need a check for an empty array on our buffered res.

License: MIT
Signed-off-by: Alan Shaw <[email protected]>
@ghost ghost assigned alanshaw Jun 21, 2018
@ghost ghost added the in progress label Jun 21, 2018
@alanshaw
Copy link
Contributor

I've amended this to add the same check for when the response doesn't have the X-Stream-Output header.

go-ipfs currently sets this (which is why we drop into the streamToValue bit - where you've added your fix) but js-ipfs currently doesn't...which is why we also need to do the check for when res is a value (not a stream). :P Hope that makes sense.

I added a test for this (ipfs-inactive/interface-js-ipfs-core#308) and verified that it doesn't work before your fix and it does work after. Hoorays \o/

There's also a PR against js-ipfs (ipfs/js-ipfs#1408) to add the X-Stream-Output header to block.get and fix an issue that disallowed us to block.put an empty buffer.

Phew, fun times.

License: MIT
Signed-off-by: Alan Shaw <[email protected]>
Copy link
Contributor

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

LGTM. Failing tests are unrelated and possibly due to ipfs/kubo#5146

@alanshaw alanshaw merged commit 88edd83 into ipfs-inactive:master Jun 25, 2018
@ghost ghost removed the in progress label Jun 25, 2018
alanshaw added a commit to ipfs/js-ipfs that referenced this pull request Jun 26, 2018
* fix: allow put empty block & add X-Stream-Output header on get

This PR allows the test in ipfs-inactive/interface-js-ipfs-core#308 to pass.

X-Stream-Output header is added to block.get reply for parity with go-ipfs.

block.put is altered to allow an empty block to be put (again for fetaure parity with go-ipfs) but only if there is a multipart file part for it. Error responses have also been improved.

refs ipfs-inactive/js-ipfs-http-client#789
tested by ipfs-inactive/interface-js-ipfs-core#308

License: MIT
Signed-off-by: Alan Shaw <[email protected]>

* chore: update interface-ipfs-core dependency

License: MIT
Signed-off-by: Alan Shaw <[email protected]>

* chore: update ipfs-api dependency

License: MIT
Signed-off-by: Alan Shaw <[email protected]>

* chore: fix the hack, open the repo not the datastore

License: MIT
Signed-off-by: Alan Shaw <[email protected]>
danieldaf pushed a commit to danieldaf/js-ipfs-api that referenced this pull request Jul 21, 2018
* Fix for empty data in block.

Found this very strange bug when creating DAGNode's with empty buffers for data.

streamToValue returns an empty array when the data is empty, which down the line triggers an exception in ipfs-block because this sends an empty array instead of a buffer.

* fix: get empty block for response without X-Stream-Output

js-ipfs currently does not set the X-Stream-Output header on block.get responses so we also need a check for an empty array on our buffered res.

License: MIT
Signed-off-by: Alan Shaw <[email protected]>

* chore: update to latest interface-ipfs-core

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

Successfully merging this pull request may close these issues.

2 participants