From 16458f801ea3504e06b65231570201502ce35f90 Mon Sep 17 00:00:00 2001 From: Juan Picado Date: Fri, 29 Oct 2021 09:00:02 +0200 Subject: [PATCH] refactor: pass options instead request object (#2605) --- packages/api/src/package.ts | 8 +- packages/core/tarball/package.json | 1 - .../convertDistRemoteToLocalTarballUrls.ts | 11 +- .../tarball/src/getLocalRegistryTarballUri.ts | 10 +- ...onvertDistRemoteToLocalTarballUrls.spec.ts | 18 ++- packages/core/tarball/tsconfig.json | 3 + packages/core/url/src/index.ts | 18 ++- packages/core/url/tests/getPublicUrl.spec.ts | 128 +++++++++++++++--- packages/web/src/api/package.ts | 2 +- packages/web/src/api/sidebar.ts | 2 +- pnpm-lock.yaml | 2 - 11 files changed, 163 insertions(+), 40 deletions(-) diff --git a/packages/api/src/package.ts b/packages/api/src/package.ts index 73d8d8b3a..32b0e9121 100644 --- a/packages/api/src/package.ts +++ b/packages/api/src/package.ts @@ -40,7 +40,7 @@ export default function (route: Router, auth: IAuth, storage: Storage, config: C route.get( '/:package/:version?', can('access'), - function (req: $RequestExtend, res: $ResponseExtend, next: $NextFunctionVer): void { + function (req: $RequestExtend, _res: $ResponseExtend, next: $NextFunctionVer): void { debug('init package by version'); const name = req.params.package; const getPackageMetaCallback = function (err, metadata: Package): void { @@ -49,7 +49,11 @@ export default function (route: Router, auth: IAuth, storage: Storage, config: C return next(err); } debug('convert dist remote to local with prefix %o', config?.url_prefix); - metadata = convertDistRemoteToLocalTarballUrls(metadata, req, config?.url_prefix); + metadata = convertDistRemoteToLocalTarballUrls( + metadata, + { protocol: req.protocol, headers: req.headers as any, host: req.host }, + config?.url_prefix + ); let queryVersion = req.params.version; debug('query by param version: %o', queryVersion); diff --git a/packages/core/tarball/package.json b/packages/core/tarball/package.json index 25869cb2d..3da88d836 100644 --- a/packages/core/tarball/package.json +++ b/packages/core/tarball/package.json @@ -41,7 +41,6 @@ }, "devDependencies": { "@verdaccio/types": "workspace:11.0.0-6-next.9", - "express": "4.17.1", "node-mocks-http": "1.10.1" }, "scripts": { diff --git a/packages/core/tarball/src/convertDistRemoteToLocalTarballUrls.ts b/packages/core/tarball/src/convertDistRemoteToLocalTarballUrls.ts index a7b760ab9..6c543be22 100644 --- a/packages/core/tarball/src/convertDistRemoteToLocalTarballUrls.ts +++ b/packages/core/tarball/src/convertDistRemoteToLocalTarballUrls.ts @@ -1,5 +1,5 @@ import { Package } from '@verdaccio/types'; -import { Request } from 'express'; +import { RequestOptions } from '@verdaccio/url'; import _ from 'lodash'; import { getLocalRegistryTarballUri } from './getLocalRegistryTarballUri'; @@ -13,7 +13,7 @@ import { getLocalRegistryTarballUri } from './getLocalRegistryTarballUri'; */ export function convertDistRemoteToLocalTarballUrls( pkg: Package, - req: Request, + request: RequestOptions, urlPrefix: string | void ): Package { for (const ver in pkg.versions) { @@ -21,7 +21,12 @@ export function convertDistRemoteToLocalTarballUrls( const distName = pkg.versions[ver].dist; if (_.isNull(distName) === false && _.isNull(distName.tarball) === false) { - distName.tarball = getLocalRegistryTarballUri(distName.tarball, pkg.name, req, urlPrefix); + distName.tarball = getLocalRegistryTarballUri( + distName.tarball, + pkg.name, + request, + urlPrefix + ); } } } diff --git a/packages/core/tarball/src/getLocalRegistryTarballUri.ts b/packages/core/tarball/src/getLocalRegistryTarballUri.ts index f0f4846dc..38814a608 100644 --- a/packages/core/tarball/src/getLocalRegistryTarballUri.ts +++ b/packages/core/tarball/src/getLocalRegistryTarballUri.ts @@ -1,12 +1,12 @@ import URL from 'url'; -import { Request } from 'express'; +import { RequestOptions } from '@verdaccio/url'; import buildDebug from 'debug'; import { getPublicUrl } from '@verdaccio/url'; const debug = buildDebug('verdaccio:core:url'); -function extractTarballFromUrl(url: string): string { +export function extractTarballFromUrl(url: string): string { // @ts-ignore return URL.parse(url).pathname.replace(/^.*\//, ''); } @@ -18,10 +18,10 @@ function extractTarballFromUrl(url: string): string { export function getLocalRegistryTarballUri( uri: string, pkgName: string, - req: Request, + requestOptions: RequestOptions, urlPrefix: string | void ): string { - const currentHost = req.headers.host; + const currentHost = requestOptions?.headers?.host; if (!currentHost) { return uri; @@ -29,7 +29,7 @@ export function getLocalRegistryTarballUri( const tarballName = extractTarballFromUrl(uri); debug('tarball name %o', tarballName); // header only set with proxy that setup with HTTPS - const domainRegistry = getPublicUrl(urlPrefix || '', req); + const domainRegistry = getPublicUrl(urlPrefix || '', requestOptions); return `${domainRegistry}${pkgName}/-/${tarballName}`; } diff --git a/packages/core/tarball/tests/convertDistRemoteToLocalTarballUrls.spec.ts b/packages/core/tarball/tests/convertDistRemoteToLocalTarballUrls.spec.ts index 557635335..3a7144f7d 100644 --- a/packages/core/tarball/tests/convertDistRemoteToLocalTarballUrls.spec.ts +++ b/packages/core/tarball/tests/convertDistRemoteToLocalTarballUrls.spec.ts @@ -30,7 +30,11 @@ describe('convertDistRemoteToLocalTarballUrls', () => { }, url: '/', }); - const convertDist = convertDistRemoteToLocalTarballUrls(cloneMetadata(), req); + const convertDist = convertDistRemoteToLocalTarballUrls(cloneMetadata(), { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }); expect(convertDist.versions['1.0.0'].dist.tarball).toEqual(buildURI(fakeHost, '1.0.0')); expect(convertDist.versions['1.0.1'].dist.tarball).toEqual(buildURI(fakeHost, '1.0.1')); }); @@ -43,7 +47,11 @@ describe('convertDistRemoteToLocalTarballUrls', () => { }, url: '/', }); - const convertDist = convertDistRemoteToLocalTarballUrls(cloneMetadata(), req); + const convertDist = convertDistRemoteToLocalTarballUrls(cloneMetadata(), { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }); expect(convertDist.versions['1.0.0'].dist.tarball).toEqual( convertDist.versions['1.0.0'].dist.tarball ); @@ -57,7 +65,11 @@ describe('convertDistRemoteToLocalTarballUrls', () => { }, url: '/', }); - const convertDist = convertDistRemoteToLocalTarballUrls(cloneMetadata(), req); + const convertDist = convertDistRemoteToLocalTarballUrls(cloneMetadata(), { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }); expect(convertDist.versions['1.0.0'].dist.tarball).toEqual( convertDist.versions['1.0.0'].dist.tarball ); diff --git a/packages/core/tarball/tsconfig.json b/packages/core/tarball/tsconfig.json index eeee84ecb..d44ae8543 100644 --- a/packages/core/tarball/tsconfig.json +++ b/packages/core/tarball/tsconfig.json @@ -12,6 +12,9 @@ }, { "path": "../url" + }, + { + "path": "../core" } ] } diff --git a/packages/core/url/src/index.ts b/packages/core/url/src/index.ts index 0476894d4..8f475c826 100644 --- a/packages/core/url/src/index.ts +++ b/packages/core/url/src/index.ts @@ -88,18 +88,26 @@ export function validateURL(publicUrl: string | void) { } } -export function getPublicUrl(url_prefix: string = '', req): string { +export type RequestOptions = { + host: string; + protocol: string; + headers: { [key: string]: string }; +}; + +export function getPublicUrl(url_prefix: string = '', requestOptions: RequestOptions): string { if (validateURL(process.env.VERDACCIO_PUBLIC_URL as string)) { const envURL = new URL(wrapPrefix(url_prefix), process.env.VERDACCIO_PUBLIC_URL as string).href; debug('public url by env %o', envURL); return envURL; - } else if (req.get('host')) { - const host = req.get('host'); + } else if (requestOptions.headers['host']) { + const host = requestOptions.headers['host']; if (!isHost(host)) { throw new Error('invalid host'); } - const protoHeader = process.env.VERDACCIO_FORWARDED_PROTO ?? HEADERS.FORWARDED_PROTO; - const protocol = getWebProtocol(req.get(protoHeader), req.protocol); + const protoHeader = + process.env.VERDACCIO_FORWARDED_PROTO?.toLocaleLowerCase() ?? + HEADERS.FORWARDED_PROTO.toLowerCase(); + const protocol = getWebProtocol(requestOptions.headers[protoHeader], requestOptions.protocol); const combinedUrl = combineBaseUrl(protocol, host, url_prefix); debug('public url by request %o', combinedUrl); return combinedUrl; diff --git a/packages/core/url/tests/getPublicUrl.spec.ts b/packages/core/url/tests/getPublicUrl.spec.ts index 4516db042..05213bf86 100644 --- a/packages/core/url/tests/getPublicUrl.spec.ts +++ b/packages/core/url/tests/getPublicUrl.spec.ts @@ -11,7 +11,13 @@ describe('host', () => { method: 'GET', url: '/', }); - expect(getPublicUrl(undefined, req)).toEqual('/'); + expect( + getPublicUrl(undefined, { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }) + ).toEqual('/'); }); test('get a valid host', () => { @@ -22,7 +28,13 @@ describe('host', () => { }, url: '/', }); - expect(getPublicUrl(undefined, req)).toEqual('http://some.com/'); + expect( + getPublicUrl(undefined, { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }) + ).toEqual('http://some.com/'); }); test('check a valid host header injection', () => { @@ -31,11 +43,15 @@ describe('host', () => { headers: { host: `some.com">`, }, + hostname: `some.com">`, url: '/', }); expect(function () { - // @ts-expect-error - getPublicUrl({}, req); + getPublicUrl('', { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }); }).toThrow('invalid host'); }); @@ -48,7 +64,13 @@ describe('host', () => { url: '/', }); - expect(getPublicUrl('/prefix/', req)).toEqual('http://some.com/prefix/'); + expect( + getPublicUrl('/prefix/', { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }) + ).toEqual('http://some.com/prefix/'); }); test('get a valid host with prefix no trailing', () => { @@ -60,7 +82,13 @@ describe('host', () => { url: '/', }); - expect(getPublicUrl('/prefix-no-trailing', req)).toEqual('http://some.com/prefix-no-trailing/'); + expect( + getPublicUrl('/prefix-no-trailing', { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }) + ).toEqual('http://some.com/prefix-no-trailing/'); }); test('get a valid host with null prefix', () => { @@ -72,7 +100,13 @@ describe('host', () => { url: '/', }); - expect(getPublicUrl(null, req)).toEqual('http://some.com/'); + expect( + getPublicUrl(null, { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }) + ).toEqual('http://some.com/'); }); }); @@ -87,7 +121,13 @@ describe('X-Forwarded-Proto', () => { url: '/', }); - expect(getPublicUrl(undefined, req)).toEqual('https://some.com/'); + expect( + getPublicUrl(undefined, { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }) + ).toEqual('https://some.com/'); }); test('with a invalid X-Forwarded-Proto https', () => { @@ -100,7 +140,13 @@ describe('X-Forwarded-Proto', () => { url: '/', }); - expect(getPublicUrl(undefined, req)).toEqual('http://some.com/'); + expect( + getPublicUrl(undefined, { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }) + ).toEqual('http://some.com/'); }); test('with a HAProxy X-Forwarded-Proto https', () => { @@ -113,7 +159,13 @@ describe('X-Forwarded-Proto', () => { url: '/', }); - expect(getPublicUrl(undefined, req)).toEqual('https://some.com/'); + expect( + getPublicUrl(undefined, { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }) + ).toEqual('https://some.com/'); }); test('with a HAProxy X-Forwarded-Proto different protocol', () => { @@ -126,7 +178,13 @@ describe('X-Forwarded-Proto', () => { url: '/', }); - expect(getPublicUrl(undefined, req)).toEqual('http://some.com/'); + expect( + getPublicUrl(undefined, { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }) + ).toEqual('http://some.com/'); }); }); @@ -142,7 +200,13 @@ describe('env variable', () => { url: '/', }); - expect(getPublicUrl(undefined, req)).toEqual('https://env.domain.com/'); + expect( + getPublicUrl(undefined, { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }) + ).toEqual('https://env.domain.com/'); delete process.env.VERDACCIO_PUBLIC_URL; }); @@ -157,7 +221,13 @@ describe('env variable', () => { url: '/', }); - expect(getPublicUrl(undefined, req)).toEqual('https://env.domain.com/urlPrefix/'); + expect( + getPublicUrl(undefined, { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }) + ).toEqual('https://env.domain.com/urlPrefix/'); delete process.env.VERDACCIO_PUBLIC_URL; }); @@ -172,7 +242,13 @@ describe('env variable', () => { url: '/', }); - expect(getPublicUrl(undefined, req)).toEqual('https://env.domain.com/'); + expect( + getPublicUrl(undefined, { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }) + ).toEqual('https://env.domain.com/'); delete process.env.VERDACCIO_PUBLIC_URL; }); @@ -187,7 +263,13 @@ describe('env variable', () => { url: '/', }); - expect(getPublicUrl(undefined, req)).toEqual('http://some.com/'); + expect( + getPublicUrl(undefined, { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }) + ).toEqual('http://some.com/'); delete process.env.VERDACCIO_PUBLIC_URL; }); @@ -202,7 +284,13 @@ describe('env variable', () => { url: '/', }); - expect(getPublicUrl(undefined, req)).toEqual('http://some.com/'); + expect( + getPublicUrl(undefined, { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }) + ).toEqual('http://some.com/'); delete process.env.VERDACCIO_PUBLIC_URL; }); @@ -217,7 +305,13 @@ describe('env variable', () => { url: '/', }); - expect(getPublicUrl(undefined, req)).toEqual('http://some/'); + expect( + getPublicUrl(undefined, { + host: req.hostname, + headers: req.headers as any, + protocol: req.protocol, + }) + ).toEqual('http://some/'); delete process.env.VERDACCIO_PUBLIC_URL; }); }); diff --git a/packages/web/src/api/package.ts b/packages/web/src/api/package.ts index 112705d61..b47e9fd0b 100644 --- a/packages/web/src/api/package.ts +++ b/packages/web/src/api/package.ts @@ -80,7 +80,7 @@ function addPackageWebApi(route: Router, storage: Storage, auth: IAuth, config: pkgCopy.dist.tarball = getLocalRegistryTarballUri( pkgCopy.dist.tarball, pkg.name, - req, + { protocol: req.protocol, headers: req.headers as any, host: req.hostname }, config?.url_prefix ); } diff --git a/packages/web/src/api/sidebar.ts b/packages/web/src/api/sidebar.ts index 75da9895f..783df538a 100644 --- a/packages/web/src/api/sidebar.ts +++ b/packages/web/src/api/sidebar.ts @@ -42,7 +42,7 @@ function addSidebarWebApi(route: Router, config: Config, storage: Storage, auth: let sideBarInfo = _.clone(info); sideBarInfo.versions = convertDistRemoteToLocalTarballUrls( info, - req, + { protocol: req.protocol, headers: req.headers as any, host: req.hostname }, config.url_prefix ).versions; if (typeof v === 'string' && isVersionValid(info, v)) { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b43f7c639..307c938ae 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -398,7 +398,6 @@ importers: '@verdaccio/types': workspace:11.0.0-6-next.9 '@verdaccio/url': workspace:11.0.0-6-next.7 '@verdaccio/utils': workspace:6.0.0-6-next.8 - express: 4.17.1 lodash: 4.17.21 node-mocks-http: 1.10.1 dependencies: @@ -408,7 +407,6 @@ importers: lodash: 4.17.21 devDependencies: '@verdaccio/types': link:../types - express: 4.17.1 node-mocks-http: 1.10.1 packages/core/types: