Skip to content

Commit

Permalink
fix: validate pkg@version spec (#500)
Browse files Browse the repository at this point in the history 
> follow #495 after supporting
spec, adjust the parameter validation rules
1. ๐Ÿ†• Add `Spec` validation rule, validating the spec by npa
2. ๐Ÿ› ๏ธ Upgrade versionOrTag to versionSpec to support semver expressions,
such as `^2.x || > 3.x`
---------

> follow #495 ๆ”ฏๆŒ spec ๅŽ๏ผŒ่ฐƒๆ•ดๅ‚ๆ•ฐๆ ก้ชŒ่ง„ๅˆ™
1. ๐Ÿ†• ๆ–ฐๅขž `Sepc` ๆ ก้ชŒ่ง„ๅˆ™๏ผŒไฝฟ็”จ npa ๆ‹ผๆŽฅๅŒ…ๅ่ฟ›่กŒ้ชŒ่ฏ
2. ๐Ÿ› ๏ธ versionOrTag ๅ‡็บงไธบ versionSpec ๆ”ฏๆŒ semver ่กจ่พพๅผ๏ผŒไพ‹ๅฆ‚ `^2.x || > 3.x`
  • Loading branch information
@elrrrrrrr
elrrrrrrr committed Jun 7, 2023
1 parent efb6412 commit a9d2ff7
Show file tree
Hide file tree
Showing 7 changed files with 94 additions and 37 deletions.
58 changes: 31 additions & 27 deletions app/port/controller/PackageVersionFileController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { PackageManagerService } from '../../core/service/PackageManagerService'
import { PackageVersionFile } from '../../core/entity/PackageVersionFile';
import { PackageVersion } from '../../core/entity/PackageVersion';
import { DistRepository } from '../../repository/DistRepository';
import { Spec } from '../typebox';

type FileItem = {
path: string,
Expand Down Expand Up @@ -65,84 +66,87 @@ export class PackageVersionFileController extends AbstractController {
}

@HTTPMethod({
// PUT /:fullname/:versionOrTag/files
path: `/:fullname(${FULLNAME_REG_STRING})/:versionOrTag/files`,
// PUT /:fullname/:versionSpec/files
path: `/:fullname(${FULLNAME_REG_STRING})/:versionSpec/files`,
method: HTTPMethodEnum.PUT,
})
@Middleware(AdminAccess)
async sync(@HTTPParam() fullname: string, @HTTPParam() versionOrTag: string) {
async sync(@Context() ctx: EggContext, @HTTPParam() fullname: string, @HTTPParam() versionSpec: string) {
ctx.tValidate(Spec, `${fullname}@${versionSpec}`);
this.#requireUnpkgEnable();
const [ scope, name ] = getScopeAndName(fullname);
const { packageVersion } = await this.packageManagerService.showPackageVersionByVersionOrTag(
scope, name, versionOrTag);
scope, name, versionSpec);
if (!packageVersion) {
throw new NotFoundError(`${fullname}@${versionOrTag} not found`);
throw new NotFoundError(`${fullname}@${versionSpec} not found`);
}
const files = await this.packageVersionFileService.syncPackageVersionFiles(packageVersion);
return files.map(file => formatFileItem(file));
}

@HTTPMethod({
// GET /:fullname/:versionOrTag/files => /:fullname/:versionOrTag/files/${pkg.main}
// GET /:fullname/:versionOrTag/files?meta
// GET /:fullname/:versionOrTag/files/
path: `/:fullname(${FULLNAME_REG_STRING})/:versionOrTag/files`,
// GET /:fullname/:versionSpec/files => /:fullname/:versionSpec/files/${pkg.main}
// GET /:fullname/:versionSpec/files?meta
// GET /:fullname/:versionSpec/files/
path: `/:fullname(${FULLNAME_REG_STRING})/:versionSpec/files`,
method: HTTPMethodEnum.GET,
})
async listFiles(@Context() ctx: EggContext,
@HTTPParam() fullname: string,
@HTTPParam() versionOrTag: string,
@HTTPParam() versionSpec: string,
@HTTPQuery() meta: string) {
this.#requireUnpkgEnable();
ctx.tValidate(Spec, `${fullname}@${versionSpec}`);
ctx.vary(this.config.cnpmcore.cdnVaryHeader);
const [ scope, name ] = getScopeAndName(fullname);
const packageVersion = await this.#getPackageVersion(ctx, fullname, scope, name, versionOrTag);
const packageVersion = await this.#getPackageVersion(ctx, fullname, scope, name, versionSpec);
ctx.set('cache-control', META_CACHE_CONTROL);
const hasMeta = typeof meta === 'string' || ctx.path.endsWith('/files/');
// meta request
if (hasMeta) {
const files = await this.#listFilesByDirectory(packageVersion, '/');
if (!files) {
throw new NotFoundError(`${fullname}@${versionOrTag}/files not found`);
throw new NotFoundError(`${fullname}@${versionSpec}/files not found`);
}
return files;
}
const { manifest } = await this.packageManagerService.showPackageVersionManifest(scope, name, versionOrTag, false, true);
const { manifest } = await this.packageManagerService.showPackageVersionManifest(scope, name, versionSpec, false, true);
// GET /foo/1.0.0/files => /foo/1.0.0/files/{main}
// ignore empty entry exp: @types/[email protected]/
const indexFile = manifest?.main || 'index.js';
ctx.redirect(join(ctx.path, indexFile));
}

@HTTPMethod({
// GET /:fullname/:versionOrTag/files/:path
// GET /:fullname/:versionOrTag/files/:path?meta
path: `/:fullname(${FULLNAME_REG_STRING})/:versionOrTag/files/:path(.+)`,
// GET /:fullname/:versionSpec/files/:path
// GET /:fullname/:versionSpec/files/:path?meta
path: `/:fullname(${FULLNAME_REG_STRING})/:versionSpec/files/:path(.+)`,
method: HTTPMethodEnum.GET,
})
async raw(@Context() ctx: EggContext,
@HTTPParam() fullname: string,
@HTTPParam() versionOrTag: string,
@HTTPParam() versionSpec: string,
@HTTPParam() path: string,
@HTTPQuery() meta: string) {
this.#requireUnpkgEnable();
ctx.tValidate(Spec, `${fullname}@${versionSpec}`);
ctx.vary(this.config.cnpmcore.cdnVaryHeader);
const [ scope, name ] = getScopeAndName(fullname);
path = `/${path}`;
const packageVersion = await this.#getPackageVersion(ctx, fullname, scope, name, versionOrTag);
const packageVersion = await this.#getPackageVersion(ctx, fullname, scope, name, versionSpec);
if (path.endsWith('/')) {
const directory = path.substring(0, path.length - 1);
const files = await this.#listFilesByDirectory(packageVersion, directory);
if (!files) {
throw new NotFoundError(`${fullname}@${versionOrTag}/files${directory} not found`);
throw new NotFoundError(`${fullname}@${versionSpec}/files${directory} not found`);
}
ctx.set('cache-control', META_CACHE_CONTROL);
return files;
}

const file = await this.packageVersionFileService.showPackageVersionFile(packageVersion, path);
if (!file) {
throw new NotFoundError(`File ${fullname}@${versionOrTag}${path} not found`);
throw new NotFoundError(`File ${fullname}@${versionSpec}${path} not found`);
}
const hasMeta = typeof meta === 'string';
if (hasMeta) {
Expand All @@ -157,20 +161,20 @@ export class PackageVersionFileController extends AbstractController {
return await this.distRepository.getDistStream(file.dist);
}

async #getPackageVersion(ctx: EggContext, fullname: string, scope: string, name: string, versionOrTag: string) {
async #getPackageVersion(ctx: EggContext, fullname: string, scope: string, name: string, versionSpec: string) {
const { blockReason, packageVersion } = await this.packageManagerService.showPackageVersionByVersionOrTag(
scope, name, versionOrTag);
scope, name, versionSpec);
if (blockReason) {
this.setCDNHeaders(ctx);
throw this.createPackageBlockError(blockReason, fullname, versionOrTag);
throw this.createPackageBlockError(blockReason, fullname, versionSpec);
}
if (!packageVersion) {
throw new NotFoundError(`${fullname}@${versionOrTag} not found`);
throw new NotFoundError(`${fullname}@${versionSpec} not found`);
}
if (packageVersion.version !== versionOrTag) {
if (packageVersion.version !== versionSpec) {
ctx.set('cache-control', META_CACHE_CONTROL);
let location = ctx.url.replace(`/${fullname}/${versionOrTag}/files`, `/${fullname}/${packageVersion.version}/files`);
location = location.replace(`/${fullname}/${encodeURIComponent(versionOrTag)}/files`, `/${fullname}/${packageVersion.version}/files`);
let location = ctx.url.replace(`/${fullname}/${versionSpec}/files`, `/${fullname}/${packageVersion.version}/files`);
location = location.replace(`/${fullname}/${encodeURIComponent(versionSpec)}/files`, `/${fullname}/${packageVersion.version}/files`);
throw this.createControllerRedirectError(location);
}
return packageVersion;
Expand Down
14 changes: 8 additions & 6 deletions app/port/controller/package/ShowPackageVersionController.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,35 +12,37 @@ import { AbstractController } from '../AbstractController';
import { getScopeAndName, FULLNAME_REG_STRING } from '../../../common/PackageUtil';
import { isSyncWorkerRequest } from '../../../common/SyncUtil';
import { PackageManagerService } from '../../../core/service/PackageManagerService';
import { Spec } from 'app/port/typebox';

@HTTPController()
export class ShowPackageVersionController extends AbstractController {
@Inject()
private packageManagerService: PackageManagerService;

@HTTPMethod({
// GET /:fullname/:versionOrTag
path: `/:fullname(${FULLNAME_REG_STRING})/:versionOrTag`,
// GET /:fullname/:versionSpec
path: `/:fullname(${FULLNAME_REG_STRING})/:versionSpec`,
method: HTTPMethodEnum.GET,
})
async show(@Context() ctx: EggContext, @HTTPParam() fullname: string, @HTTPParam() versionOrTag: string) {
async show(@Context() ctx: EggContext, @HTTPParam() fullname: string, @HTTPParam() versionSpec: string) {
// https://github.com/npm/registry/blob/master/docs/responses/package-metadata.md#full-metadata-format
ctx.tValidate(Spec, `${fullname}@${versionSpec}`);
const [ scope, name ] = getScopeAndName(fullname);
const isSync = isSyncWorkerRequest(ctx);
const abbreviatedMetaType = 'application/vnd.npm.install-v1+json';
const isFullManifests = ctx.accepts([ 'json', abbreviatedMetaType ]) !== abbreviatedMetaType;

const { blockReason, manifest, pkg } = await this.packageManagerService.showPackageVersionManifest(scope, name, versionOrTag, isSync, isFullManifests);
const { blockReason, manifest, pkg } = await this.packageManagerService.showPackageVersionManifest(scope, name, versionSpec, isSync, isFullManifests);
if (!pkg) {
const allowSync = this.getAllowSync(ctx);
throw this.createPackageNotFoundErrorWithRedirect(fullname, undefined, allowSync);
}
if (blockReason) {
this.setCDNHeaders(ctx);
throw this.createPackageBlockError(blockReason, fullname, versionOrTag);
throw this.createPackageBlockError(blockReason, fullname, versionSpec);
}
if (!manifest) {
throw new NotFoundError(`${fullname}@${versionOrTag} not found`);
throw new NotFoundError(`${fullname}@${versionSpec} not found`);
}
this.setCDNHeaders(ctx);
return manifest;
Expand Down
16 changes: 16 additions & 0 deletions app/port/typebox.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Type, Static } from '@sinclair/typebox';
import { RegistryType } from '../common/enum/Registry';
import semver from 'semver';
import npa from 'npm-package-arg';
import { HookType } from '../common/enum/Hook';
import binaryConfig from '../../config/binaries';

Expand Down Expand Up @@ -53,6 +54,11 @@ export const Version = Type.String({
maxLength: 256,
});

export const Spec = Type.String({
format: 'semver-spec',
minLength: 1,
});

export const Description = Type.String({ maxLength: 10240, transform: [ 'trim' ] });

export const TagRule = Type.Object({
Expand Down Expand Up @@ -125,6 +131,16 @@ export function patchAjv(ajv: any) {
return !semver.validRange(tag);
},
});
ajv.addFormat('semver-spec', {
type: 'string',
validate: (spec: string) => {
try {
return !!npa(spec);
} catch (e) {
return false;
}
},
});
ajv.addFormat('binary-name', {
type: 'string',
validate: (binaryName: string) => {
Expand Down
11 changes: 10 additions & 1 deletion test/port/controller/PackageVersionFileController/listFiles.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('test/port/controller/PackageVersionFileController/listFiles.test.ts',
publisher = await TestUtil.createUser();
});

describe('[GET /:fullname/:versionOrTag/files] listFiles()', () => {
describe('[GET /:fullname/:versionSpec/files] listFiles()', () => {
it('should 404 when enableUnpkg = false', async () => {
mock(app.config.cnpmcore, 'allowPublishNonScopePackage', true);
mock(app.config.cnpmcore, 'enableUnpkg', false);
Expand Down Expand Up @@ -68,6 +68,15 @@ describe('test/port/controller/PackageVersionFileController/listFiles.test.ts',
assert.equal(res.body.error, '[NOT_FOUND] File [email protected]/index.js not found');
});

it('should 422 when invalid spec', async () => {
mock(app.config.cnpmcore, 'enableUnpkg', true);
const res = await app.httpRequest()
.get('/foo/@invalid-spec/files')
.expect(422);

assert.equal(res.body.error, '[INVALID_PARAM] must match format "semver-spec"');
});

it('should list one package version files', async () => {
mock(app.config.cnpmcore, 'allowPublishNonScopePackage', true);
const pkg = await TestUtil.getFullPackage({
Expand Down
11 changes: 10 additions & 1 deletion test/port/controller/PackageVersionFileController/raw.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe('test/port/controller/PackageVersionFileController/raw.test.ts', () =>
publisher = await TestUtil.createUser();
});

describe('[GET /:fullname/:versionOrTag/files/:path] raw()', () => {
describe('[GET /:fullname/:versionSpec/files/:path] raw()', () => {
it('should show one package version raw file', async () => {
mock(app.config.cnpmcore, 'allowPublishNonScopePackage', true);
const pkg = await TestUtil.getFullPackage({
Expand Down Expand Up @@ -137,6 +137,15 @@ describe('test/port/controller/PackageVersionFileController/raw.test.ts', () =>
assert.equal(res.body.error, `[NOT_FOUND] File ${pkg.name}@1.0.0/package2.json not found`);
});

it('should 422 when invalid spec', async () => {
mock(app.config.cnpmcore, 'enableUnpkg', true);
const res = await app.httpRequest()
.get('/foo/@invalid-spec/files/package.json?meta')
.expect(422);

assert.equal(res.body.error, '[INVALID_PARAM] must match format "semver-spec"');
});

it('should ignore not exists file on tar onentry', async () => {
const tarball = await TestUtil.readFixturesFile('unpkg.com/ide-metrics-api-grpc-0.0.1-main-gha.8962.tgz');
const { integrity } = await calculateIntegrity(tarball);
Expand Down
2 changes: 1 addition & 1 deletion test/port/controller/PackageVersionFileController/sync.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('test/port/controller/PackageVersionFileController/sync.test.ts', () =>
adminUser = await TestUtil.createAdmin();
});

describe('[PUT /:fullname/:versionOrTag/files] sync()', () => {
describe('[PUT /:fullname/:versionSpec/files] sync()', () => {
it('should 404 when enableUnpkg = false', async () => {
mock(app.config.cnpmcore, 'allowPublishNonScopePackage', true);
mock(app.config.cnpmcore, 'enableUnpkg', false);
Expand Down
19 changes: 18 additions & 1 deletion test/port/controller/package/ShowPackageVersionController.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('test/port/controller/package/ShowPackageVersionController.test.ts', ()
publisher = await TestUtil.createUser();
});

describe('[GET /:fullname/:versionOrTag] show()', () => {
describe('[GET /:fullname/:versionSpec] show()', () => {
it('should show one package version', async () => {
mock(app.config.cnpmcore, 'allowPublishNonScopePackage', true);
const pkg = await TestUtil.getFullPackage({
Expand All @@ -37,6 +37,15 @@ describe('test/port/controller/package/ShowPackageVersionController.test.ts', ()
assert.equal(res.body.dist.integrity, 'sha512-n+4CQg0Rp1Qo0p9a0R5E5io67T9iD3Lcgg6exmpmt0s8kd4XcOoHu2kiu6U7xd69cGq0efkNGWUBP229ObfRSA==');
assert.equal(res.body.dist.size, 251);
assert.equal(res.body.description, 'work with utf8mb4 ๐Ÿ’ฉ, ๐Œ† utf8_unicode_ci, foo๐Œ†bar ๐Ÿป');

// support semver spec
await app.httpRequest()
.get('/foo/%5E1.0')
.expect(200);

await app.httpRequest()
.get('/foo/^1.0')
.expect(200);
});

it('should fix bug version', async () => {
Expand Down Expand Up @@ -125,6 +134,14 @@ describe('test/port/controller/package/ShowPackageVersionController.test.ts', ()
assert(res.body.version === '2.0.0');
});

it('should 422 with invalid spec', async () => {
const res = await app.httpRequest()
.get('/foo/@invalid-spec')
.expect(422)
.expect('content-type', 'application/json; charset=utf-8');
assert(res.error, '[INVALID_PARAM] must match format "semver-spec"');
});

it('should work with scoped package', async () => {
const pkg = await TestUtil.getFullPackage({
name: '@cnpm/foo',
Expand Down

0 comments on commit a9d2ff7

Please sign in to comment.