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

node-portfinder not supported anymore #4383

Closed
pietia opened this issue Apr 11, 2022 · 12 comments · Fixed by #4384
Closed

node-portfinder not supported anymore #4383

pietia opened this issue Apr 11, 2022 · 12 comments · Fixed by #4384

Comments

@pietia
Copy link

pietia commented Apr 11, 2022

Bug report

Please consider replacing node-portfinder with some other module. node-portfinder is not supported anymore, meaning webpack-dev-server users are struggling now how to update async ( CVE-2021-43138, http-party/node-portfinder#126).

Actual Behavior

Expected Behavior

How Do We Reproduce?

Please paste the results of npx webpack-cli info here, and mention other relevant information

@alexander-akait
Copy link
Member

node-portfinder is not supported anymore

Why you decided it? Let's wait when developer gives feedback, not all people available 24/7

@pietia
Copy link
Author

pietia commented Apr 11, 2022

#4383

node-portfinder is not supported anymore

Why you decided it? Let's wait when developer gives feedback, not all people available 24/7

I decided about what? http-party/node-portfinder#117 (comment)

@alexander-akait
Copy link
Member

Yes, I see, but there is security fix, so make sense to do patch release, anyway we have idea how to replace it, let's give some time

@arderyp
Copy link

arderyp commented Apr 12, 2022

I think the concern is that portfinder hasn't been touched in years, so people are worried its dead and the maintainers won't chime in the issue a patch for this, in which case replacing the dependency is the only other option.

Hopefully time will prove portfinder isn't dead yet :)

@alexander-akait
Copy link
Member

As fast workaround we can move required parts of portfinder inside webpack-dev-server, there is no many code, if someone want to do it asap feel free to send a PR

@ludofischer
Copy link
Contributor

As fast workaround we can move required parts of portfinder inside webpack-dev-server, there is no many code, if someone want to do it asap feel free to send a PR

There's also a zero-dep alternative get-port. I was looking at this port finding problem even before the security vulnerability, I've made webpack-dev-server with get-port here. It's not exactly equivalent because when the preferred port is not available get-port returns a random one while portfinder increments the port number.

My impression is that both packages have a lot of functionality that webpack-dev-server does not need, as it only requires to find a port once on startup, so adding the code to webpack-dev-server itself sounds reasonable. The basic mechanism is the same in get-port and portfinder: create a node server and check whether it successfully binds. If not, check the next port.

From open issues in both repos, I think both get-port and portfinder suffer from race conditions when you run tests with Jest in parallel. This could be one reason the webpack-dev-server tests are so flaky on CI.

@arderyp
Copy link

arderyp commented Apr 13, 2022

@ludofischer awesome summary. I am wholly in the camp of "don't take on a third party dependency to use a single function therein". If the small bit of logic we need is re-implemented here, and we drop a third party port dependency, then we reduce the chance of this happening again.

@alexander-akait
Copy link
Member

@ludofischer We fixed flaky tests recently, most of them were due async logic in web socket communication and jest is not good tool to do it, random port - we don't use it, before tests we open required ports for all tests and assign port to each other using mocks.

Anyway porting small function to get port is good for me - less deps and less size, and yes, it is simple function

@alexander-akait
Copy link
Member

So PR welcome (I'll have time this weekend, if nobody will send a PR)

@jersobh
Copy link

jersobh commented Apr 13, 2022

I've just created an override and it seems to work.
Just add to package.json:

  "overrides": {
    "webpack-dev-server": {
      "portfinder": {
        "async": "3.2.3"
      }
    }
  },

@arderyp
Copy link

arderyp commented Apr 13, 2022

That's interesting @jersobh, but it sounds like portfinder is not only dead, but advised by the creator to not be run in production (see issue linked above). So I think the proper solution here is to drop that dependency, not workaround it 🫳

@pietia
Copy link
Author

pietia commented Apr 13, 2022

@jersobh it's not supported by all npm versions
@arderyp +1 for proper fix.

mugglim added a commit to mugglim/simple-virtual-dom that referenced this issue Apr 14, 2022
What : webpack-dev-server@4.8.1 requires async@^2.6.2 via
portfinder@1.0.28

How: override async package in package.json

Ref : webpack/webpack-dev-server#4383
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 a pull request may close this issue.

5 participants