From cd06651f89441b275860ec8e67224c66126bc64e Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Thu, 2 Mar 2023 17:19:55 +0300 Subject: [PATCH] fix(packagist): Replace V2 URL path instead of joining it (#20709) --- lib/modules/datasource/packagist/index.ts | 24 +++-- lib/util/url.spec.ts | 118 ++++++++++++++-------- lib/util/url.ts | 21 ++++ 3 files changed, 110 insertions(+), 53 deletions(-) diff --git a/lib/modules/datasource/packagist/index.ts b/lib/modules/datasource/packagist/index.ts index 9928405698..8970551bbd 100644 --- a/lib/modules/datasource/packagist/index.ts +++ b/lib/modules/datasource/packagist/index.ts @@ -5,7 +5,7 @@ import { cache } from '../../../util/cache/package/decorator'; import * as hostRules from '../../../util/host-rules'; import type { HttpOptions } from '../../../util/http/types'; import * as p from '../../../util/promises'; -import { parseUrl, resolveBaseUrl } from '../../../util/url'; +import { replaceUrlPath, resolveBaseUrl } from '../../../util/url'; import * as composerVersioning from '../../versioning/composer'; import { Datasource } from '../datasource'; import type { GetReleasesConfig, ReleaseResult } from '../types'; @@ -70,7 +70,7 @@ export class PackagistDatasource extends Datasource { ): string { const { key, hash } = regFile; const fileName = hash ? key.replace('%hash%', hash) : key; - const url = `${regUrl}/${fileName}`; + const url = resolveBaseUrl(regUrl, fileName); return url; } @@ -124,12 +124,16 @@ export class PackagistDatasource extends Datasource { metadataUrl: string, packageName: string ): Promise { - let pkgUrl = metadataUrl.replace('%package%', packageName); - pkgUrl = parseUrl(pkgUrl) ? pkgUrl : resolveBaseUrl(registryUrl, pkgUrl); + const pkgUrl = replaceUrlPath( + registryUrl, + metadataUrl.replace('%package%', packageName) + ); const pkgPromise = this.getJson(pkgUrl, z.unknown()); - let devUrl = metadataUrl.replace('%package%', `${packageName}~dev`); - devUrl = parseUrl(devUrl) ? devUrl : resolveBaseUrl(registryUrl, devUrl); + const devUrl = replaceUrlPath( + registryUrl, + metadataUrl.replace('%package%', `${packageName}~dev`) + ); const devPromise = this.getJson(devUrl, z.unknown()).then( (x) => x, () => null @@ -144,8 +148,6 @@ export class PackagistDatasource extends Datasource { registryUrl: string, registryMeta: RegistryMeta ): string | null { - const { origin: registryHost } = new URL(registryUrl); - if ( registryMeta.providersUrl && packageName in registryMeta.providerPackages @@ -155,12 +157,12 @@ export class PackagistDatasource extends Datasource { if (hash) { url = url.replace('%hash%', hash); } - return resolveBaseUrl(registryHost, url); + return replaceUrlPath(registryUrl, url); } if (registryMeta.providersLazyUrl) { - return resolveBaseUrl( - registryHost, + return replaceUrlPath( + registryUrl, registryMeta.providersLazyUrl.replace('%package%', packageName) ); } diff --git a/lib/util/url.spec.ts b/lib/util/url.spec.ts index d1e9e0e17e..27652dc9c5 100644 --- a/lib/util/url.spec.ts +++ b/lib/util/url.spec.ts @@ -6,57 +6,91 @@ import { joinUrlParts, parseLinkHeader, parseUrl, + replaceUrlPath, resolveBaseUrl, trimTrailingSlash, validateUrl, } from './url'; describe('util/url', () => { - test.each([ - ['http://foo.io', '', 'http://foo.io'], - ['http://foo.io/', '', 'http://foo.io'], - ['http://foo.io', '/', 'http://foo.io/'], - ['http://foo.io/', '/', 'http://foo.io/'], - - ['http://foo.io', '/aaa', 'http://foo.io/aaa'], - ['http://foo.io', 'aaa', 'http://foo.io/aaa'], - ['http://foo.io/', '/aaa', 'http://foo.io/aaa'], - ['http://foo.io/', 'aaa', 'http://foo.io/aaa'], - ['http://foo.io', '/aaa/', 'http://foo.io/aaa/'], - ['http://foo.io', 'aaa/', 'http://foo.io/aaa/'], - ['http://foo.io/', '/aaa/', 'http://foo.io/aaa/'], - ['http://foo.io/', 'aaa/', 'http://foo.io/aaa/'], - - ['http://foo.io/aaa', '/bbb', 'http://foo.io/aaa/bbb'], - ['http://foo.io/aaa', 'bbb', 'http://foo.io/aaa/bbb'], - ['http://foo.io/aaa/', '/bbb', 'http://foo.io/aaa/bbb'], - ['http://foo.io/aaa/', 'bbb', 'http://foo.io/aaa/bbb'], - - ['http://foo.io/aaa', '/bbb/', 'http://foo.io/aaa/bbb/'], - ['http://foo.io/aaa', 'bbb/', 'http://foo.io/aaa/bbb/'], - ['http://foo.io/aaa/', '/bbb/', 'http://foo.io/aaa/bbb/'], - ['http://foo.io/aaa/', 'bbb/', 'http://foo.io/aaa/bbb/'], - - ['http://foo.io', 'http://bar.io/bbb', 'http://bar.io/bbb'], - ['http://foo.io/', 'http://bar.io/bbb', 'http://bar.io/bbb'], - ['http://foo.io/aaa', 'http://bar.io/bbb', 'http://bar.io/bbb'], - ['http://foo.io/aaa/', 'http://bar.io/bbb', 'http://bar.io/bbb'], - - ['http://foo.io', 'http://bar.io/bbb/', 'http://bar.io/bbb/'], - ['http://foo.io/', 'http://bar.io/bbb/', 'http://bar.io/bbb/'], - ['http://foo.io/aaa', 'http://bar.io/bbb/', 'http://bar.io/bbb/'], - ['http://foo.io/aaa/', 'http://bar.io/bbb/', 'http://bar.io/bbb/'], - - ['http://foo.io', 'aaa?bbb=z', 'http://foo.io/aaa?bbb=z'], - ['http://foo.io', '/aaa?bbb=z', 'http://foo.io/aaa?bbb=z'], - ['http://foo.io/', 'aaa?bbb=z', 'http://foo.io/aaa?bbb=z'], - ['http://foo.io/', '/aaa?bbb=z', 'http://foo.io/aaa?bbb=z'], - - ['http://foo.io', 'aaa/?bbb=z', 'http://foo.io/aaa?bbb=z'], - ])('%s + %s => %s', (baseUrl, x, result) => { + test.each` + baseUrl | x | result + ${'http://foo.io'} | ${''} | ${'http://foo.io'} + ${'http://foo.io/'} | ${''} | ${'http://foo.io'} + ${'http://foo.io'} | ${'/'} | ${'http://foo.io/'} + ${'http://foo.io/'} | ${'/'} | ${'http://foo.io/'} + ${'http://foo.io'} | ${'/aaa'} | ${'http://foo.io/aaa'} + ${'http://foo.io'} | ${'aaa'} | ${'http://foo.io/aaa'} + ${'http://foo.io/'} | ${'/aaa'} | ${'http://foo.io/aaa'} + ${'http://foo.io/'} | ${'aaa'} | ${'http://foo.io/aaa'} + ${'http://foo.io'} | ${'/aaa/'} | ${'http://foo.io/aaa/'} + ${'http://foo.io'} | ${'aaa/'} | ${'http://foo.io/aaa/'} + ${'http://foo.io/'} | ${'/aaa/'} | ${'http://foo.io/aaa/'} + ${'http://foo.io/'} | ${'aaa/'} | ${'http://foo.io/aaa/'} + ${'http://foo.io/aaa'} | ${'/bbb'} | ${'http://foo.io/aaa/bbb'} + ${'http://foo.io/aaa'} | ${'bbb'} | ${'http://foo.io/aaa/bbb'} + ${'http://foo.io/aaa/'} | ${'/bbb'} | ${'http://foo.io/aaa/bbb'} + ${'http://foo.io/aaa/'} | ${'bbb'} | ${'http://foo.io/aaa/bbb'} + ${'http://foo.io/aaa'} | ${'/bbb/'} | ${'http://foo.io/aaa/bbb/'} + ${'http://foo.io/aaa'} | ${'bbb/'} | ${'http://foo.io/aaa/bbb/'} + ${'http://foo.io/aaa/'} | ${'/bbb/'} | ${'http://foo.io/aaa/bbb/'} + ${'http://foo.io/aaa/'} | ${'bbb/'} | ${'http://foo.io/aaa/bbb/'} + ${'http://foo.io'} | ${'http://bar.io/bbb'} | ${'http://bar.io/bbb'} + ${'http://foo.io/'} | ${'http://bar.io/bbb'} | ${'http://bar.io/bbb'} + ${'http://foo.io/aaa'} | ${'http://bar.io/bbb'} | ${'http://bar.io/bbb'} + ${'http://foo.io/aaa/'} | ${'http://bar.io/bbb'} | ${'http://bar.io/bbb'} + ${'http://foo.io'} | ${'http://bar.io/bbb/'} | ${'http://bar.io/bbb/'} + ${'http://foo.io/'} | ${'http://bar.io/bbb/'} | ${'http://bar.io/bbb/'} + ${'http://foo.io/aaa'} | ${'http://bar.io/bbb/'} | ${'http://bar.io/bbb/'} + ${'http://foo.io/aaa/'} | ${'http://bar.io/bbb/'} | ${'http://bar.io/bbb/'} + ${'http://foo.io'} | ${'aaa?bbb=z'} | ${'http://foo.io/aaa?bbb=z'} + ${'http://foo.io'} | ${'/aaa?bbb=z'} | ${'http://foo.io/aaa?bbb=z'} + ${'http://foo.io/'} | ${'aaa?bbb=z'} | ${'http://foo.io/aaa?bbb=z'} + ${'http://foo.io/'} | ${'/aaa?bbb=z'} | ${'http://foo.io/aaa?bbb=z'} + ${'http://foo.io'} | ${'aaa/?bbb=z'} | ${'http://foo.io/aaa?bbb=z'} + `('$baseUrl + $x => $result', ({ baseUrl, x, result }) => { expect(resolveBaseUrl(baseUrl, x)).toBe(result); }); + test.each` + baseUrl | x | result + ${'http://foo.io'} | ${''} | ${'http://foo.io'} + ${'http://foo.io/'} | ${''} | ${'http://foo.io'} + ${'http://foo.io'} | ${'/'} | ${'http://foo.io/'} + ${'http://foo.io/'} | ${'/'} | ${'http://foo.io/'} + ${'http://foo.io'} | ${'/aaa'} | ${'http://foo.io/aaa'} + ${'http://foo.io'} | ${'aaa'} | ${'http://foo.io/aaa'} + ${'http://foo.io/'} | ${'/aaa'} | ${'http://foo.io/aaa'} + ${'http://foo.io/'} | ${'aaa'} | ${'http://foo.io/aaa'} + ${'http://foo.io'} | ${'/aaa/'} | ${'http://foo.io/aaa/'} + ${'http://foo.io'} | ${'aaa/'} | ${'http://foo.io/aaa/'} + ${'http://foo.io/'} | ${'/aaa/'} | ${'http://foo.io/aaa/'} + ${'http://foo.io/'} | ${'aaa/'} | ${'http://foo.io/aaa/'} + ${'http://foo.io/aaa'} | ${'/bbb'} | ${'http://foo.io/bbb'} + ${'http://foo.io/aaa'} | ${'bbb'} | ${'http://foo.io/bbb'} + ${'http://foo.io/aaa/'} | ${'/bbb'} | ${'http://foo.io/bbb'} + ${'http://foo.io/aaa/'} | ${'bbb'} | ${'http://foo.io/bbb'} + ${'http://foo.io/aaa'} | ${'/bbb/'} | ${'http://foo.io/bbb/'} + ${'http://foo.io/aaa'} | ${'bbb/'} | ${'http://foo.io/bbb/'} + ${'http://foo.io/aaa/'} | ${'/bbb/'} | ${'http://foo.io/bbb/'} + ${'http://foo.io/aaa/'} | ${'bbb/'} | ${'http://foo.io/bbb/'} + ${'http://foo.io'} | ${'http://bar.io/bbb'} | ${'http://bar.io/bbb'} + ${'http://foo.io/'} | ${'http://bar.io/bbb'} | ${'http://bar.io/bbb'} + ${'http://foo.io/aaa'} | ${'http://bar.io/bbb'} | ${'http://bar.io/bbb'} + ${'http://foo.io/aaa/'} | ${'http://bar.io/bbb'} | ${'http://bar.io/bbb'} + ${'http://foo.io'} | ${'http://bar.io/bbb/'} | ${'http://bar.io/bbb/'} + ${'http://foo.io/'} | ${'http://bar.io/bbb/'} | ${'http://bar.io/bbb/'} + ${'http://foo.io/aaa'} | ${'http://bar.io/bbb/'} | ${'http://bar.io/bbb/'} + ${'http://foo.io/aaa/'} | ${'http://bar.io/bbb/'} | ${'http://bar.io/bbb/'} + ${'http://foo.io'} | ${'aaa?bbb=z'} | ${'http://foo.io/aaa?bbb=z'} + ${'http://foo.io'} | ${'/aaa?bbb=z'} | ${'http://foo.io/aaa?bbb=z'} + ${'http://foo.io/'} | ${'aaa?bbb=z'} | ${'http://foo.io/aaa?bbb=z'} + ${'http://foo.io/'} | ${'/aaa?bbb=z'} | ${'http://foo.io/aaa?bbb=z'} + ${'http://foo.io'} | ${'aaa/?bbb=z'} | ${'http://foo.io/aaa?bbb=z'} + `('replaceUrlPath("$baseUrl", "$x") => $result', ({ baseUrl, x, result }) => { + expect(replaceUrlPath(baseUrl, x)).toBe(result); + }); + it('getQueryString', () => { expect(getQueryString({ a: 1, b: [1, 2] })).toBe('a=1&b=1&b=2'); }); diff --git a/lib/util/url.ts b/lib/util/url.ts index 0f48f048ff..fda64af82b 100644 --- a/lib/util/url.ts +++ b/lib/util/url.ts @@ -30,6 +30,12 @@ export function trimLeadingSlash(path: string): string { return path.replace(/^\/+/, ''); } +/** + * Resolves an input path against a base URL + * + * @param baseUrl - base URL to resolve against + * @param input - input path (if this is a full URL, it will be returned) + */ export function resolveBaseUrl(baseUrl: string, input: string | URL): string { const inputString = input.toString(); @@ -44,6 +50,21 @@ export function resolveBaseUrl(baseUrl: string, input: string | URL): string { return host ? inputString : urlJoin(baseUrl, pathname || ''); } +/** + * Replaces the path of a URL with a new path + * + * @param baseUrl - source URL + * @param path - replacement path (if this is a full URL, it will be returned) + */ +export function replaceUrlPath(baseUrl: string | URL, path: string): string { + if (parseUrl(path)) { + return path; + } + + const { origin } = is.string(baseUrl) ? new URL(baseUrl) : baseUrl; + return urlJoin(origin, path); +} + export function getQueryString(params: Record): string { const usp = new URLSearchParams(); for (const [k, v] of Object.entries(params)) {