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 ws version #7885

Closed
wants to merge 2 commits into from
Closed

Update ws version #7885

wants to merge 2 commits into from

Conversation

dannyvv
Copy link
Member

@dannyvv dannyvv commented May 28, 2021

Microsoft Reviewers: Open in CodeFlow

@dannyvv dannyvv added the security Pull requests that address a security vulnerability label May 28, 2021
@dannyvv dannyvv requested a review from a team as a code owner May 28, 2021 20:40
@@ -51,7 +51,7 @@
"stacktrace-parser": "^0.1.3",
"use-subscription": "^1.0.0",
"whatwg-fetch": "^3.0.0",
"ws": "^6.1.4"
"ws": "^7.4.6"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this the way RN apps consume the native WS implementation, or is this a separate WS library?

Copy link
Contributor

Choose a reason for hiding this comment

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

By looking at https://github.com/websockets/ws/tree/7.4.6, I can confirm this is a third-party JS-only library.

@dannyvv for my education, why is this library needed, if RNW already provides a native WS implementation?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Most of the public developer tooling for RN is written in Javascript. Some of those will run a websocket server. I would look for usages of this in the "react-native" package.

It could be Android/iOS only, but we need to sync our packages with upstream dependencies because they can add or remove usages in their code.

@NickGerleman
Copy link
Collaborator

NickGerleman commented May 29, 2021

The versions in "package.json" in "react-native-windows" and "@office-iss/react-native-win32" are synchronized by script to match "react-native", and will be overwritten on next integration.

So we shouldn't change those versions, but can fix in upstream react-native (if needed by other rn consumers), or add a resolution.

@NickGerleman
Copy link
Collaborator

Following up, but Facebook SREs should be getting bugged to bump the package if it has a CVE attached.

@NickGerleman
Copy link
Collaborator

Confirmed FB has 30 day SLA to address the issue in their master branch, which they detected on Friday.

@dannyvv
Copy link
Member Author

dannyvv commented Jun 2, 2021

taking in favor of Jon's update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security Pull requests that address a security vulnerability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants