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

upgrade ws to 8.17.1 & jsdom to 24.1.0 #13903

Merged

Conversation

rschnekenbu
Copy link
Contributor

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

yarn.lock Outdated
Comment on lines 12405 to 12408
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==
Copy link
Member

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.

Copy link
Contributor Author

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.

@rschnekenbu rschnekenbu changed the title upgrade ws to 8.18.0 upgrade ws to 8.17.1 & jsdom to 24.1.0 Jul 18, 2024
@rschnekenbu rschnekenbu requested a review from msujew July 18, 2024 07:59
Copy link
Member

@msujew msujew left a 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 👍

@msujew msujew added the dependencies pull requests that update a dependency file label Jul 18, 2024
fixes eclipse-theia#13848

contributed on behalf of STMicroelectronics

Signed-off-by: Remi Schnekenburger <rschnekenburger@eclipsesource.com>
@rschnekenbu rschnekenbu merged commit ed576b5 into eclipse-theia:master Jul 18, 2024
10 of 14 checks passed
@rschnekenbu rschnekenbu deleted the issues/13848-ws_upgrade branch July 18, 2024 13:53
@martin-fleck-at
Copy link
Contributor

@rschnekenbu @msujew All CI jobs are now failing because of the minimum Node 18 version in jsdom, i.e.,

error jsdom@24.1.0: The engine "node" is incompatible with this module. Expected version ">=18". Got "16.20.1"
error Found incompatible module.

See for instance: https://github.com/eclipse-theia/theia/actions/runs/9992821246

@rschnekenbu
Copy link
Contributor Author

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.

@sgraband sgraband added this to the 1.52.0 milestone Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies pull requests that update a dependency file
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Upgrade ws to 8.18.0
4 participants