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

ipfs.files.add with {cidVersion: 1} returns valid hash with undefined size #1585

Closed
lidel opened this issue Sep 22, 2018 · 3 comments
Closed
Assignees
Labels
exp/novice Someone with a little familiarity can pick up kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up

Comments

@lidel
Copy link
Member

lidel commented Sep 22, 2018

This was originally found as a part of #1440 and ipfs/js-ipfs-http-response#9

  • Version: js-ipfs 0.32.2
  • Platform: Node
  • Subsystem:

Type: Bug

Severity: High

Description:

For some inputs ipfs.files.add with {cidVersion: 1} returns valid CID however size field is undefined. The problem does not occur if {cidVersion: 0} is used.

Result of ipfs.files.add (CIDv0)
{ path: 'Qma4hjFTnCasJ8PVp3mZbZK5g2vGDT4LByLJ7m8ciyRFZP',
    hash: 'Qma4hjFTnCasJ8PVp3mZbZK5g2vGDT4LByLJ7m8ciyRFZP',
    size: 20 }
Result of ipfs.files.add (CIDv1)
{ path: 'zb2rhdTDKmCQD2a9x2TfLR61M3s7RmESzwV5mqgnakXQbm5gp',
    hash: 'zb2rhdTDKmCQD2a9x2TfLR61M3s7RmESzwV5mqgnakXQbm5gp',
    size: undefined }

Steps to reproduce the error:

Did not look into details, but I attach test case that illustrates the issue: js-ipfs-add-cidv1-missing-size-bug-repro.zip.

To run it: yarn && yarn test (or npm install && npm test).
Hope this helps.

@lidel lidel added the kind/bug A bug in existing code (including security flaws) label Sep 22, 2018
@alanshaw alanshaw added exp/novice Someone with a little familiarity can pick up status/ready Ready to be worked P2 Medium: Good to have, but can wait until someone steps up labels Sep 25, 2018
@alanshaw
Copy link
Member

@achingbrain do you have some time to look into this one? Looks like it might be a bug in unixfs-engine.

@achingbrain
Copy link
Member

Sure, I'll take a look

@achingbrain achingbrain self-assigned this Sep 25, 2018
achingbrain added a commit that referenced this issue Sep 25, 2018
If you specify `cidVersion: 0`, for go compatibility you will have raw leaf nodes.
If your data fits in one dag node, the root node will be a raw node for the same
reason, and `ipfs.object.get` will return a buffer.

This PR inspects the returned node to see if it's a buffer, if it is, use the
`length` property, otherwise use the DAGNode `size` property.

Fixes #1585
@ghost ghost added status/in-progress In progress and removed status/ready Ready to be worked labels Sep 25, 2018
@achingbrain
Copy link
Member

Found the issue, see #1591

@lidel thanks so much for providing the failing test as an example. However even after the fix is merged it'll still fail because it asserts that the reported size of the file added with the v1 CID is greater than the input buffer - the default behaviour for v0 CIDs was to wrap all data in protobufs but for v1 it's to have it as a raw node (e.g. just a buffer), so the reported size will be the same as the input buffer. Just so you know...

@ghost ghost removed the status/in-progress In progress label Sep 27, 2018
alanshaw pushed a commit that referenced this issue Sep 27, 2018
If you specify `cidVersion: 0`, for go compatibility you will have raw leaf nodes.
If your data fits in one dag node, the root node will be a raw node for the same
reason, and `ipfs.object.get` will return a buffer.

This PR inspects the returned node to see if it's a buffer, if it is, use the
`length` property, otherwise use the DAGNode `size` property.

Fixes #1585
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/novice Someone with a little familiarity can pick up kind/bug A bug in existing code (including security flaws) P2 Medium: Good to have, but can wait until someone steps up
Projects
None yet
Development

No branches or pull requests

3 participants