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

Why is there no dependabot notification to bump https-proxy-agent #5178

Closed
jandubois opened this issue Jul 17, 2023 · 1 comment · Fixed by #5223
Closed

Why is there no dependabot notification to bump https-proxy-agent #5178

jandubois opened this issue Jul 17, 2023 · 1 comment · Fixed by #5223
Assignees
Labels
kind/quality quality improvements, refactoring, Automation via CI, E2E, Integration, CLI or REST API
Milestone

Comments

@jandubois
Copy link
Member

See also #5057

We have bumped to 6.1.0, and reverted back to 5.0.1, but there have been several releases since then, without a dependabot PR.

Please review if other modules are similarly not tracked properly!

@jandubois jandubois added the kind/quality quality improvements, refactoring, Automation via CI, E2E, Integration, CLI or REST API label Jul 17, 2023
@jandubois jandubois added this to the 1.10 milestone Jul 17, 2023
@ericpromislow
Copy link
Contributor

I manually bumped the two proxies:

diff --git a/package.json b/package.json
index b827a2c0..815c28c5 100644
--- a/package.json
+++ b/package.json
@@ -65,9 +65,9 @@
     "express": "4.18.1",
     "ffi-napi": "4.0.3",
     "fs-extra": "11.1.0",
-    "http-proxy-agent": "5.0.0",
+    "http-proxy-agent": "7.0.0",
     "http-proxy-middleware": "2.0.6",
-    "https-proxy-agent": "5.0.1",
+    "https-proxy-agent": "6.1.0",
     "intl-messageformat": "10.2.5",
     "jquery": "3.6.3",
     "jsonpath": "1.1.1",

and made some small changes to networking/proxy.ts:

diff --git a/pkg/rancher-desktop/main/networking/proxy.ts b/pkg/rancher-desktop/main/networking/proxy.ts
index baafb400..b82d7674 100644
--- a/pkg/rancher-desktop/main/networking/proxy.ts
+++ b/pkg/rancher-desktop/main/networking/proxy.ts
@@ -7,7 +7,7 @@ import { URL } from 'url';
 
 import { Agent, ClientRequest, RequestOptions, AgentCallbackReturn } from 'agent-base';
 import Electron from 'electron';
-import HttpProxyAgent from 'http-proxy-agent';
+import { HttpProxyAgent } from 'http-proxy-agent';
 import { HttpsProxyAgent } from 'https-proxy-agent';
 import { SocksProxyAgent } from 'socks-proxy-agent';
 
@@ -63,7 +63,7 @@ export default class ElectronProxyAgent extends Agent {
         if (opts.secureEndpoint) {
           return new CustomHttpsProxyAgent(proxyURL, this.options);
         } else {
-          return HttpProxyAgent(proxyURL);
+          return new HttpProxyAgent(proxyURL);
         }
       }
       default:
@@ -75,23 +75,11 @@ export default class ElectronProxyAgent extends Agent {
   }
 }
 
-class CustomHttpsProxyAgent extends HttpsProxyAgent {
+class CustomHttpsProxyAgent extends HttpsProxyAgent<string> {
   constructor(proxyURL: string, opts: HttpsAgentOptions) {
-    // Use object destructing here to ensure we only get wanted properties.
-    const { hostname, port, protocol } = new URL(proxyURL);
-    const mergedOpts = Object.assign({}, opts, {
-      hostname, port, protocol,
-    });
-
-    super(mergedOpts);
+    super(proxyURL);
     this.options = opts;
   }
-
-  callback(req: ClientRequest, opts: RequestOptions): Promise<net.Socket> {
-    const mergedOptions = Object.assign({}, this.options, opts);
-
-    return super.callback(req, mergedOptions);
-  }
 }
 
 class CustomSocksProxyAgent extends SocksProxyAgent {

but I don't know how to test this in the same kind of environment reported in issue #5009

Also don't know why dependabot has been quiet about the 7.0.0 version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/quality quality improvements, refactoring, Automation via CI, E2E, Integration, CLI or REST API
Projects
None yet
2 participants