fix(proxy): proxy protocol check (#5474)

* fix(proxy): proxy protocol check

* Clarify docs

.changeset/famous-hornets-know.md

@@ -0,0 +1,6 @@
+---
+'@verdaccio/proxy': patch
+'@verdaccio/website': patch
+---
+
+fix(proxy): proxy protocol check

packages/proxy/src/agent.ts

@@ -15,7 +15,9 @@ class CustomAgents {
     this.proxy = proxy;
     this.url = url;
     this.agentOptions = agentOptions;
-    const { protocol } = this.getParsedUrl();
+    // Type of agent depends on the protocol of the server URL (no on the proxy)
+    // See https://www.npmjs.com/package/hpagent
+    const { protocol } = new URL(this.url);
     this.agent = this.getAgent(protocol);
   }
 
@@ -41,10 +43,6 @@ class CustomAgents {
         : { http: new HttpAgent(this.agentOptions) };
     }
   }
-
-  private getParsedUrl() {
-    return this.proxy ? new URL(this.proxy) : new URL(this.url);
-  }
 }
 
 export default CustomAgents;

packages/proxy/src/proxy.ts

@@ -632,20 +632,6 @@ class ProxyStorage implements IProxy {
       }
     }
 
-    // validate proxy protocol matches the proxy_key type
-    if (this.proxy) {
-      const proxyUrl = new URL(this.proxy);
-      const expectedProtocol = isHTTPS ? 'https:' : 'http:';
-      if (proxyUrl.protocol !== expectedProtocol) {
-        this.logger.error(
-          { proxy: this.proxy, expectedProtocol },
-          `invalid protocol for ${proxy_key} - must be ${expectedProtocol}`
-        );
-        this.proxy = undefined;
-        return;
-      }
-    }
-
     if (typeof this.proxy === 'string') {
       debug('using proxy @{proxy} for @{url}', this.proxy, this.url.href);
       this.logger.debug(

packages/proxy/test/proxy.protocol.spec.ts

@@ -28,60 +28,47 @@ describe('Check protocol of proxy', () => {
   beforeEach(() => {
     vi.clearAllMocks();
   });
-  test('validate main config protocol - http', () => {
+  test('main config - http registry, http proxy', () => {
     expect(
-      getProxyInstance(
-        'http://registry.domain.org',
-        { http_proxy: 'http://registry.local.org' },
-        {}
-      ).proxy
-    ).toEqual('http://registry.local.org');
+      getProxyInstance('http://registry.domain.org', { http_proxy: 'http://proxy.local' }, {}).proxy
+    ).toEqual('http://proxy.local');
   });
-  test('main config invalid protocol - http', () => {
+  test('main config - http registry, https proxy', () => {
     expect(
-      getProxyInstance(
-        'http://registry.domain.org',
-        { http_proxy: 'https://registry.local.org' },
-        {}
-      ).proxy
-    ).toEqual(undefined);
-    expect(mockError).toHaveBeenCalledOnce();
+      getProxyInstance('http://registry.domain.org', { http_proxy: 'https://proxy.local' }, {})
+        .proxy
+    ).toEqual('https://proxy.local');
   });
-  test('main config invalid protocol - https', () => {
+  test('main config invalid config key - http registry', () => {
     expect(
-      getProxyInstance(
-        'https://registry.domain.org',
-        { https_proxy: 'http://registry.local.org' },
-        {}
-      ).proxy
+      getProxyInstance('http://registry.domain.org', { https_proxy: 'anything' }, {}).proxy
+    ).toEqual(undefined);
+  });
+  test('main config invalid config key - https registry', () => {
+    expect(
+      getProxyInstance('https://registry.domain.org', { http_proxy: 'anything' }, {}).proxy
     ).toEqual(undefined);
-    expect(mockError).toHaveBeenCalledOnce();
   });
 
-  test('validate uplink config protocol - http', () => {
+  test('uplink config - http registry, http proxy', () => {
     expect(
-      getProxyInstance(
-        'https://registry.domain.org',
-        {},
-        { https_proxy: 'https://proxy.domain.org' }
-      ).proxy
-    ).toEqual('https://proxy.domain.org');
+      getProxyInstance('http://registry.domain.org', {}, { http_proxy: 'http://proxy.local' }).proxy
+    ).toEqual('http://proxy.local');
   });
-  test('uplink config invalid protocol - http', () => {
+  test('uplink config - http registry, https proxy', () => {
     expect(
-      getProxyInstance('http://registry.domain.org', {}, { http_proxy: 'https://proxy.domain.org' })
+      getProxyInstance('http://registry.domain.org', {}, { http_proxy: 'https://proxy.local' })
         .proxy
-    ).toEqual(undefined);
-    expect(mockError).toHaveBeenCalledOnce();
+    ).toEqual('https://proxy.local');
   });
-  test('uplink config invalid protocol - https', () => {
+  test('uplink config invalid config key - http registry', () => {
     expect(
-      getProxyInstance(
-        'https://registry.domain.org',
-        {},
-        { https_proxy: 'http://proxy.domain.org' }
-      ).proxy
+      getProxyInstance('http://registry.domain.org', {}, { https_proxy: 'anything' }).proxy
+    ).toEqual(undefined);
+  });
+  test('uplink config invalid config key - https registry', () => {
+    expect(
+      getProxyInstance('https://registry.domain.org', {}, { http_proxy: 'anything' }).proxy
     ).toEqual(undefined);
-    expect(mockError).toHaveBeenCalledOnce();
   });
 });

website/docs/config.md

@@ -322,7 +322,22 @@ https:
 
 ### Proxy {#proxy}
 
-Proxies are special-purpose HTTP servers designed to transfer data from remote servers to local clients. You can define a HTTP or HTTPS proxy in the main configuration or separately for each uplink. The definition for uplinks have higher priority. The proxy protocol (http or https) has to match the protocol of the registry or uplink URLs.
+Proxies are special-purpose HTTP servers designed to transfer data from remote servers to local clients. You can define a HTTP or HTTPS proxy in the main configuration or separately for each uplink. The definition for uplinks have higher priority.
+
+:::note
+
+The proxy configuration key (`http_proxy` or `https_proxy`) has to match the protocol of the uplink URL!
+
+For example, to use a proxy for npm i.e. `https://registry.npmjs.com`, then you have to use `https_proxy` in your configuration to specify you proxy URL (no matter if the proxy uses `http` or `https`).
+
+```yaml
+uplinks:
+  npmjs:
+    url: https://registry.npmjs.org/
+    https_proxy: http://my.proxy.local/
+```
+
+:::
 
 #### http_proxy and https_proxy {#http_proxy-and-https_proxy}
 

website/versioned_docs/version-6.x/config.md

@@ -330,7 +330,22 @@ https:
 
 ### Proxy {#proxy}
 
-Proxies are special-purpose HTTP servers designed to transfer data from remote servers to local clients. You can define a HTTP or HTTPS proxy in the main configuration or separately for each uplink. The definition for uplinks have higher priority. The proxy protocol (http or https) has to match the protocol of the registry URL.
+Proxies are special-purpose HTTP servers designed to transfer data from remote servers to local clients. You can define a HTTP or HTTPS proxy in the main configuration or separately for each uplink. The definition for uplinks have higher priority.
+
+:::note
+
+The proxy configuration key (`http_proxy` or `https_proxy`) has to match the protocol of the uplink URL!
+
+For example, to use a proxy for npm i.e. `https://registry.npmjs.com`, then you have to use `https_proxy` in your configuration to specify you proxy URL (no matter if the proxy uses `http` or `https`).
+
+```yaml
+uplinks:
+  npmjs:
+    url: https://registry.npmjs.org/
+    https_proxy: http://my.proxy.local/
+```
+
+:::
 
 #### http_proxy and https_proxy {#http_proxy-and-https_proxy}