search

SocketCluster / socketcluster-client

JavaScript client for SocketCluster
MIT License
291 stars 92 forks source link

Avoid usage of `eval` #79

Closed zalmoxisus closed 6 years ago

zalmoxisus commented 7 years ago

Hey @jondubois.

Our firefox extension was rejected from AMO because we're having unsecure eval in the code coming from cycle, which is a dependency of sc-errors. Is it possible to remove it? I'd suggest to use jsan which is inspired by cycle, but doesn't have those issues, is way more performant and takes care of lots of edge cases.

jondubois commented 7 years ago

@zalmoxisus I'm happy to replace cycle with something else. Does jsan expose a decycle function? The sc-errors module doesn't actually stringify the error object into a string, it merely removes cyclic dependencies from it so that needs to be taken into account. See https://github.com/SocketCluster/sc-errors/blob/master/index.js#L283

zalmoxisus commented 7 years ago

@jondubois it's not possible directly, you'll have to serialize and parse back: jsan.parse(jsan.stringify(obj, null, null, {circular: '[CIRCULAR]'})). Probably there'd be a lighter lib for this only case.

zalmoxisus commented 7 years ago

@jondubois I guess we could just move here those few lines of code with the comment indicating the source (though though that lib is also a fork). As I understand there's no need for that other function using eval.

jondubois commented 7 years ago

@zalmoxisus That sounds like a good idea especially since sc-errors appears to be the only place where the cycle module is used and yes we don't use the retrocycle function. So I guess we can copy that function into a file directly in the sc-errors repo and remove the cycle dependency in from package.json.

jondubois commented 7 years ago

I can do it this week sometime, but feel free to make a PR if you have the time.

zalmoxisus commented 7 years ago

No probs (https://github.com/SocketCluster/sc-errors/pull/2). Just not sure if the link to the source would be sufficient for Public Domain license.

jondubois commented 7 years ago

@zalmoxisus Are they referring to this license: https://en.wikipedia.org/wiki/Public-domain_software ? If so, it appears to be very permissive "Software in the public domain can be modified, distributed, or sold even without any attribution by anyone".

jondubois commented 7 years ago

Your PR looks good at first glance. I'll test and merge it this weekend if all is well. Thanks!