Comments (13)
Well ... the correct response is to say that zii
is a bad actor and is going to cause side-effects all over the place and you really need to reject it from your dependency tree as fast as you can as if it's a viral infection you need to treat.
But we could deal with it directly without too much pain though and won't feel like a hack because it's technically appropriate anyway:
diff --git a/src/index.js b/src/index.js
index df66bad..8bda372 100644
--- a/src/index.js
+++ b/src/index.js
@@ -114,9 +114,9 @@ function validEncode (name, buf) {
* @throws {Error} Will throw if the encoding is not supported
*/
function encoding (nameOrCode) {
- if (constants.names[/** @type {BaseName} */(nameOrCode)]) {
+ if (constants.names.hasOwnProperty(/** @type {BaseName} */(nameOrCode))) {
return constants.names[/** @type {BaseName} */(nameOrCode)]
- } else if (constants.codes[/** @type {BaseCode} */(nameOrCode)]) {
+ } else if (constants.codes.hasOwnProperty(/** @type {BaseCode} */(nameOrCode))) {
return constants.codes[/** @type {BaseCode} */(nameOrCode)]
} else {
throw new Error(`Unsupported encoding: ${nameOrCode}`)
(edit: the types are probably not going to be happy here, maybe good enough to just remove the type cast although I'm not sure what the flow-on effects will be)
from js-multibase.
Thank you for the help with this! Looking forward to trying to integrate IPFS into our application.
from js-multibase.
damn i dont even know what to say @multiformats/javascript-team thoughts ?
from js-multibase.
I should note that this is where I'm trying to use this:
zii is pulled in a couple levels of dependencies down, so it's not an easy one for us to just patch out.
from js-multibase.
Where did you get the CID "zQmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n"? It's not valid. I think there may be some bad generation of the hashes in one of the dependencies that's incorrectly appending 'z' instead of properly processing it.
from js-multibase.
That comes from this:
Which calls this:
Which prepends a "z" and then tries to decode.
However, the problem happens before it even gets to the point where it would be valid or not. It comes from autodetection of encoding of hashes which start with a "z" trigger. If they start with the letter "z", this is true if zii is active:
Line 117 in c373031
So it returns from here:
Line 118 in c373031
Instead of falling back to:
Line 119 in c373031
And returning from:
Line 120 in c373031
It triggers the zii override gets because there is nothing in constants.names with the key "z". But because zii adds a prototype for that, the if()
runs because the condition is true. If, instead, the checks are reversed and it checks for codes first (like in pull request #81), it does not conflict with zii because there is already a "z" key in codes.
All that to say...swapping those two fixes the conflict.
from js-multibase.
Well, this is the type of second-degree problems mutating Object prototype (what zii
does) gets you into 🤷
IIUC we could accept a PR that protects our libs from this malevolence and adds additional check in:
Lines 116 to 122 in c373031
Something like:
- if (constants.names[nameOrCode]) {
+ if (constants.names[nameOrCode] && typeof constants.codes[nameOrCode] === 'string') {
Thoughts?
from js-multibase.
Well, this is the type of second-degree problems mutating Object prototype (what
zii
does) gets you into
You'll hear no argument from me on that. But unfortunately, it's a dependency that's buried rather deep within other stuff. I do have an open issue filed with that project to try to eliminate zii, but they use it a lot, so it's probably going to be quite a while before that happens.
IIUC we could accept a PR that protects our libs from this malevolence and adds additional check in:
Lines 116 to 122 in c373031
Something like:
- if (constants.names[nameOrCode]) { + if (constants.names[nameOrCode] && typeof constants.codes[nameOrCode] === 'string') {Thoughts?
That should work. Or pull request #81, which I tested with zii to make sure it works.
from js-multibase.
However it gets fixed, if you want to test it, all you have to do is npm install zii
and add require("zii")
to a source file, then run the test suite.
from js-multibase.
Something like:
- if (constants.names[nameOrCode]) { + if (constants.names[nameOrCode] && typeof constants.codes[nameOrCode] === 'string') {Thoughts?
Turns out this didn't quite work. (a) It needs to check constants.names
in the second part, and (b) the elements of that are instances of Base
, so it needs to check for an object type. At any rate, I've submitted a working version (which I tested both with and without zii) as #82.
from js-multibase.
Well ... the correct response is to say that
zii
is a bad actor and is going to cause side-effects all over the place and you really need to reject it from your dependency tree as fast as you can as if it's a viral infection you need to treat.But we could deal with it directly without too much pain though and won't feel like a hack because it's technically appropriate anyway:
diff --git a/src/index.js b/src/index.js index df66bad..8bda372 100644 --- a/src/index.js +++ b/src/index.js @@ -114,9 +114,9 @@ function validEncode (name, buf) { * @throws {Error} Will throw if the encoding is not supported */ function encoding (nameOrCode) { - if (constants.names[/** @type {BaseName} */(nameOrCode)]) { + if (constants.names.hasOwnProperty(/** @type {BaseName} */(nameOrCode))) { return constants.names[/** @type {BaseName} */(nameOrCode)] - } else if (constants.codes[/** @type {BaseCode} */(nameOrCode)]) { + } else if (constants.codes.hasOwnProperty(/** @type {BaseCode} */(nameOrCode))) { return constants.codes[/** @type {BaseCode} */(nameOrCode)] } else { throw new Error(`Unsupported encoding: ${nameOrCode}`)
I would agree with this sentiment, and have expressed such in the bug report for the project that pulls it in. But, it's a core package for JavaScript Scuttlebutt clients, so it's not like we can just drop it and it would be difficult for us to reimplement. So...it is what it is.
That change looks to me like it should work. I haven't tested it, but it looks like it would solve the problem.
from js-multibase.
@rvagg thank you looks like a good compromise
@KyleMaas do you mind doing a PR with this diff ?
from js-multibase.
@rvagg @hugomrdias See #84.
from js-multibase.
Related Issues (20)
- ReferenceError: TextDecoder is not defined
- Move from Coveralls to CodeCov.io HOT 2
- base32 is broken HOT 3
- base64pad/base64urlpad not yet implemented HOT 3
- Ambigous encoding due to underlying design HOT 1
- typescript support HOT 2
- suggest to add suuport base58 HOT 1
- [Bug] Incorrect implementation for base58btc HOT 5
- Test against spec
- Standardize README HOT 1
- More description error text
- Export the names and codes as object (instead of array)
- Implement base36 HOT 1
- `web-encoding` package causes problems with jest in env=node and env=jsdom HOT 19
- QUESTION: how to get the string from the encode function HOT 12
- Upgrade to aegir@9 HOT 1
- Recent release broke our build HOT 2
- ReferenceError: TextDecoder is not defined HOT 1
- React Native support is dropped HOT 3
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from js-multibase.