Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update socketcluster to resolve critical security issues #65

Closed

Conversation

DonnyVerduijn
Copy link

@DonnyVerduijn DonnyVerduijn commented Oct 29, 2018

Please note, that the currently used version of SocketCluster 8.0.1 has vulnerable dependencies. Therefore, i have migrated the codebase to be compatible with the latest version of SocketCluster. I didn't run any tests, so please provide feedback. The forked repo can be installed locally using npm -i DonnyVerduijn/remotedev-server#fix-vulnerabilities.

@DonnyVerduijn
Copy link
Author

I can confirm it works together with remote-redux-devtools.

@AldoMX
Copy link

AldoMX commented Nov 2, 2018

Hey guys @jhen0409 @zalmoxisus, sorry to annoy you with an unwanted notification, but this PR fixes the following issue:
facebook/react-native#17410

Is there something I can do to help you merge this?

@zalmoxisus
Copy link
Owner

zalmoxisus commented Nov 16, 2018

@DonnyVerduijn thanks for fixing it. The problem here is thatremotedev-server was CommonJS, now we're using ES6 here and it won't be backward compatible. So it is a breaking change and we'd need to publish it as a major version, but there's already 0.3, just not shipped. So there are 2 possibilities here: rewrite as es5 (not sure if that works with current socketcluster version) or move it to v0.3 branch. That branch was enforcing node >= 4.0.0. I'd ship that version. Sorry for the confusion with the branches, that was completely rewritten and would be difficult to solve conflicts if we merge this in master. If you don't have time with that, @AldoMX offered his help.

@DonnyVerduijn
Copy link
Author

DonnyVerduijn commented Nov 16, 2018 via email

@zalmoxisus
Copy link
Owner

I added it in 0.3. Didn't use arrow functions as we'd need them in other parts as well to keep the same style.

Thanks again for you work!

@zalmoxisus zalmoxisus closed this Dec 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants