-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
upgrade ws to 8.17.1 & jsdom to 24.1.0 #13903
upgrade ws to 8.17.1 & jsdom to 24.1.0 #13903
Conversation
589b4f6
to
8197565
Compare
yarn.lock
Outdated
ws@^8.13.0: | ||
version "8.16.0" | ||
resolved "https://registry.yarnpkg.com/ws/-/ws-8.16.0.tgz#d1cd774f36fbc07165066a60e40323eab6446fd4" | ||
integrity sha512-HS0c//TP7Ina87TfiPUz1rQzMhHrl/SG2guqRcTOIUYD2q8uhUdNHZYJUaQ8aTGPzCh+c6oawMKW35nFl1dxyQ== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Is the dependency to 8.16.0
a runtime dependency? As far as I can tell, it is pulled from jsdom
. We should probably update that as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@msujew, thanks for having a look to this PR!
Yes, the ^8.13.0
dependency comes from jsdom 21.1.1. The latest version of jsdom 24.1.0) depends on ws as ^8.17.0
.
Shall I update jdom to this latest version?
I could also change ws reexported dependency from package/core/package.json to ^8.17.1
instead of ^8.18.0
. Not sure what has been the best practice in the project in the past. 8.18.0
is the latest available, 8.17.1
is the first being know without DoS vulnerabiltity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍
fixes eclipse-theia#13848 contributed on behalf of STMicroelectronics Signed-off-by: Remi Schnekenburger <rschnekenburger@eclipsesource.com>
79ece61
to
5ab3c1a
Compare
@rschnekenbu @msujew All CI jobs are now failing because of the minimum Node 18 version in jsdom, i.e.,
See for instance: https://github.com/eclipse-theia/theia/actions/runs/9992821246 |
Thanks for noticing, @martin-fleck-at! The latest version to support minimum node 16 is the 22.1.0 release. I will create a task and push a fix for it. |
What it does
Fixes #13848
This is a simple update on dependent library, known to have a risk of DoS. I tested the tool, no changes in behavior, and I also di not notice any changes in the logs.
How to test
Since this updates websocket lib, build and open the tool. Observe the logs to make sure we don't get any new entries.
Follow-ups
Review checklist
Reminder for reviewers