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

fix(files): ensure throw meaningful error for unsupported encoding in cat() #804

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

0x-r4bbit
Copy link

@0x-r4bbit 0x-r4bbit commented Jul 4, 2018

A test to verify this is available at ipfs-inactive/interface-js-ipfs-core#330 and needs to be merged first.

Fixes #799

License: MIT
Signed-off-by: Pascal Precht pascal.precht@gmail.com

@0x-r4bbit
Copy link
Author

@alanshaw maybe you can leave a quick comment on https://github.com/ipfs/js-ipfs-api/issues/799#issuecomment-402466066

I'll put this PR is shape right away then :)

src/files/cat.js Outdated
hash = cleanCID(hash)
if (checkValidInput(hash)) {
hash = cleanCID(hash)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you're right, we should delegate the parameter checking to the IPFS node we're calling so not to duplicate the logic and have to keep two sets of the same code in sync. We just need to serialize valid inputs that aren't already serialized. Could we simply do something like this?:

try {
  hash = typeof hash === 'string' ? hash : cleanCID(hash)
} catch (err) {
  return callback(err)
}

Copy link
Author

Choose a reason for hiding this comment

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

Sure that could work I'll update the and also look into if and how this can be tested.

@0x-r4bbit 0x-r4bbit changed the base branch from feat/modular-interface-tests to master July 18, 2018 09:36
@0x-r4bbit 0x-r4bbit changed the title fix(files): ensure CID in cat() command is only cleaned when input … fix(files0: ensure throw meaningful error for unsupported encoding in cat() Jul 18, 2018
@0x-r4bbit 0x-r4bbit changed the title fix(files0: ensure throw meaningful error for unsupported encoding in cat() fix(files): ensure throw meaningful error for unsupported encoding in cat() Jul 18, 2018
@0x-r4bbit
Copy link
Author

@alanshaw this is now updated. A test for verification can be found here ipfs-inactive/interface-js-ipfs-core#330

A test to verify this is available at ipfs-inactive/interface-js-ipfs-core#330 and needs to be merged first.

Fixes #799

License: MIT
Signed-off-by: Pascal Precht pascal.precht@gmail.com
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