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

fix: config.set should reject buffer values #800

Merged
merged 1 commit into from
Jul 17, 2018

Conversation

travisperson
Copy link
Contributor

config.set should be rejecting buffer values[0]. Currently it accepts them because a buffer returns typeof of object, and is serialized into an object {type: 'buffer', data: [...]}.

This change brings the type checking of the value field inline with the js-ipfs-repo[1].

More info: https://github.com/ipfs/interface-ipfs-core/issues/320

[0] https://github.com/ipfs/interface-ipfs-core/blob/ef910266b36c89fe21f9821f86bdb9955d60b468/js/src/config/set.js#L82-L87
[1] https://github.com/ipfs/js-ipfs-repo/blob/a7ea45bac6f1c264f86e79357f702314758a48f1/src/config.js#L65-L67

@ghost ghost assigned travisperson Jul 2, 2018
@ghost ghost added the in progress label Jul 2, 2018
@travisperson travisperson force-pushed the bug/config-set-reject-buffer branch 2 times, most recently from ad0a0da to bc1f20b Compare July 3, 2018 19:47
@daviddias daviddias changed the title config.set should reject buffer values fix: config.set should reject buffer values Jul 4, 2018
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

@daviddias
Copy link
Contributor

@travisperson please rebase master onto this branch to get that CI green :)

License: MIT
Signed-off-by: Travis Person <travis@protocol.ai>
@travisperson
Copy link
Contributor Author

@diasdavid @alanshaw 💚

@alanshaw alanshaw merged commit f3e6bf1 into master Jul 17, 2018
@ghost ghost removed the in progress label Jul 17, 2018
@daviddias daviddias deleted the bug/config-set-reject-buffer branch July 18, 2018 15:00
danieldaf pushed a commit to danieldaf/js-ipfs-api that referenced this pull request Jul 21, 2018
License: MIT
Signed-off-by: Travis Person <travis@protocol.ai>
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.

3 participants