Skip to content

Commit 08a499a

Browse files
authored
fix: invalid jsii target parameters are not validated (#2398)
Arguments to the jsii `targets` array that did not make sense in the target language were not being validated, leading to confusing compile-time errors. Add validation for a bunch of these. --- By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license]. [Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
1 parent e3d2007 commit 08a499a

File tree

3 files changed

+139
-10
lines changed

3 files changed

+139
-10
lines changed

src/assembler.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,10 @@ export class Assembler implements Emitter {
213213
bundled: this.projectInfo.bundleDependencies,
214214
types: Object.fromEntries(this._types),
215215
submodules: noEmptyDict(toSubmoduleDeclarations(this.mySubmodules())),
216-
targets: this.projectInfo.targets,
216+
217+
// Force this into shape
218+
targets: this.projectInfo.targets as spec.Assembly['targets'],
219+
217220
metadata: {
218221
...this.projectInfo.metadata,
219222

src/project-info.ts

Lines changed: 78 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,25 @@ export type TSCompilerOptions = Partial<
4141
>
4242
>;
4343

44+
/**
45+
* The assembly targets have some typing in `spec.PackageJson`, extract it.
46+
*
47+
* The types in the upstream spec library:
48+
*
49+
* - Missing the user-visible `go` target
50+
* - Missing the synthetic `js` target
51+
*/
52+
export type AssemblyTargets = spec.PackageJson['jsii']['targets'] & {
53+
js?: {
54+
npm: string;
55+
};
56+
go?: {
57+
moduleName: string;
58+
packageName: string;
59+
};
60+
[otherLanguage: string]: unknown;
61+
};
62+
4463
export interface ProjectInfo {
4564
readonly projectRoot: string;
4665
readonly packageJson: PackageJson;
@@ -65,7 +84,7 @@ export interface ProjectInfo {
6584
readonly peerDependencies: { readonly [name: string]: string };
6685
readonly dependencyClosure: readonly spec.Assembly[];
6786
readonly bundleDependencies?: { readonly [name: string]: string };
68-
readonly targets: spec.AssemblyTargets;
87+
readonly targets: AssemblyTargets;
6988
readonly metadata?: { readonly [key: string]: any };
7089
readonly jsiiVersionFormat: 'short' | 'full';
7190
readonly diagnostics?: { readonly [code: string]: ts.DiagnosticCategory };
@@ -85,6 +104,12 @@ export interface ProjectInfo {
85104
readonly validateTsconfig?: TypeScriptConfigValidationRuleSet;
86105
}
87106

107+
/**
108+
* A type representing the contents of a `package.json` file.
109+
*
110+
* Note that there is also a `PackageJson` type in `@jsii/spec`, and this one is
111+
* not the same. Do not ask me why.
112+
*/
88113
export interface PackageJson {
89114
readonly description?: string;
90115
readonly homepage?: string;
@@ -121,7 +146,7 @@ export interface PackageJson {
121146
// main jsii config
122147
readonly diagnostics?: { readonly [id: string]: 'error' | 'warning' | 'suggestion' | 'message' };
123148
readonly metadata?: { readonly [key: string]: unknown };
124-
readonly targets?: { readonly [name: string]: unknown };
149+
readonly targets?: AssemblyTargets;
125150
readonly versionFormat?: 'short' | 'full';
126151

127152
// Either user-provided config ...
@@ -241,8 +266,10 @@ export function loadProjectInfo(projectRoot: string): ProjectInfoResult {
241266
dependencyClosure: transitiveDependencies,
242267
bundleDependencies,
243268
targets: {
244-
..._required(pkg.jsii, 'The "package.json" file must specify the "jsii" attribute').targets,
245-
js: { npm: pkg.name },
269+
...validateTargets(
270+
_required(pkg.jsii, 'The "package.json" file must specify the "jsii" attribute').targets as AssemblyTargets,
271+
),
272+
js: { npm: pkg.name! },
246273
},
247274
metadata,
248275
jsiiVersionFormat: _validateVersionFormat(pkg.jsii?.versionFormat ?? 'full'),
@@ -278,6 +305,53 @@ export function loadProjectInfo(projectRoot: string): ProjectInfoResult {
278305
return { projectInfo, diagnostics };
279306
}
280307

308+
/**
309+
* Validate the values of the `.jsii.targets` field
310+
*/
311+
function validateTargets(targets: AssemblyTargets | undefined): AssemblyTargets | undefined {
312+
if (!targets) {
313+
return undefined;
314+
}
315+
316+
// Go package names must be valid identifiers
317+
if (targets.go) {
318+
if (!isIdentifier(targets.go.packageName)) {
319+
throw new JsiiError(`jsii.targets.go.packageName contains non-identifier characters: ${targets.go.packageName}`);
320+
}
321+
}
322+
323+
if (targets.dotnet) {
324+
if (!targets.dotnet.namespace.split('.').every(isIdentifier)) {
325+
throw new JsiiError(
326+
`jsii.targets.dotnet.namespace contains non-identifier characters: ${targets.dotnet.namespace}`,
327+
);
328+
}
329+
}
330+
331+
if (targets.java) {
332+
if (!targets.java.package.split('.').every(isIdentifier)) {
333+
throw new JsiiError(`jsii.targets.java.package contains non-identifier characters: ${targets.java.package}`);
334+
}
335+
}
336+
337+
if (targets.python) {
338+
if (!targets.python.module.split('.').every(isIdentifier)) {
339+
throw new JsiiError(`jsii.targets.python.module contains non-identifier characters: ${targets.python.module}`);
340+
}
341+
}
342+
343+
return targets;
344+
345+
/**
346+
* Regexp-check for matching an identifier
347+
*
348+
* Conveniently, all identifiers look more or less the same in all languages.
349+
*/
350+
function isIdentifier(x: string) {
351+
return /^[\w_][\w\d_]*$/u.test(x);
352+
}
353+
}
354+
281355
function _guessRepositoryType(url: string): string {
282356
if (url.endsWith('.git')) {
283357
return 'git';

test/project-info.test.ts

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { writeAssembly } from '@jsii/spec';
66
import * as clone from 'clone';
77
import * as ts from 'typescript';
88

9-
import { loadProjectInfo } from '../src/project-info';
9+
import { loadProjectInfo, PackageJson } from '../src/project-info';
1010
import { TypeScriptConfigValidationRuleSet } from '../src/tsconfig';
1111
import { VERSION } from '../src/version';
1212

@@ -28,7 +28,7 @@ const BASE_PROJECT = {
2828
},
2929
dependencies: { 'jsii-test-dep': '^1.2.3' } as { [name: string]: string },
3030
peerDependencies: { 'jsii-test-dep': '^1.2.3' } as { [name: string]: string },
31-
};
31+
} satisfies PackageJson;
3232

3333
describe('loadProjectInfo', () => {
3434
test('loads valid project', () =>
@@ -236,7 +236,7 @@ describe('loadProjectInfo', () => {
236236
diagCode2: 'warning',
237237
diagCode3: 'suggestion',
238238
diagCode4: 'message',
239-
};
239+
} as const;
240240
info.jsii.diagnostics = diagnostics;
241241
},
242242
);
@@ -248,13 +248,58 @@ describe('loadProjectInfo', () => {
248248
(info) => {
249249
const diagnostics = {
250250
diagCode1: 'invalid-category',
251-
};
251+
} as any;
252252
info.jsii.diagnostics = diagnostics;
253253
},
254254
);
255255
});
256256
});
257257

258+
describe('invalid target configuration', () => {
259+
test('invalid Go packageName is rejected', () => {
260+
expectProjectLoadError(/contains non-identifier characters/, (proj) => {
261+
proj.jsii.targets.go = {
262+
moduleName: 'asdf',
263+
packageName: 'as-df',
264+
};
265+
});
266+
});
267+
268+
test('invalid .NET namespace is rejected', () => {
269+
expectProjectLoadError(/contains non-identifier characters/, (proj) => {
270+
proj.jsii.targets.dotnet = {
271+
namespace: 'a-x',
272+
packageId: 'asdf',
273+
};
274+
});
275+
});
276+
277+
test('invalid Java package is rejected', () => {
278+
expectProjectLoadError(/contains non-identifier characters/, (proj) => {
279+
proj.jsii.targets.java = {
280+
package: 'as-df',
281+
maven: {
282+
artifactId: 'asdf',
283+
groupId: 'asdf',
284+
},
285+
};
286+
});
287+
});
288+
289+
test('invalid Python module is rejected', () => {
290+
expectProjectLoadError(/contains non-identifier characters/, (proj) => {
291+
proj.jsii.targets.python = {
292+
module: 'as-df',
293+
distName: 'as-df',
294+
};
295+
});
296+
});
297+
298+
function expectProjectLoadError(error: RegExp, gremlin: Parameters<typeof _withTestProject>[1]) {
299+
_withTestProject((root) => expect(() => loadProjectInfo(root)).toThrow(error), gremlin);
300+
}
301+
});
302+
258303
describe('user-provided tsconfig', () => {
259304
test('can set a user-provided config', () => {
260305
return _withTestProject(
@@ -354,6 +399,11 @@ const TEST_DEP_DEP_ASSEMBLY: spec.Assembly = {
354399
fingerprint: 'F1NG3RPR1N7',
355400
};
356401

402+
/**
403+
* A type to represent the package.json type that gets mutated by testproject gremlins
404+
*/
405+
type HalfFilledPackageJson = DeepWriteable<PackageJson> & typeof BASE_PROJECT;
406+
357407
/**
358408
* Creates a throw-away directory with a ``package.json`` file. Cleans up after itself.
359409
*
@@ -364,7 +414,7 @@ const TEST_DEP_DEP_ASSEMBLY: spec.Assembly = {
364414
*/
365415
function _withTestProject<T>(
366416
cb: (projectRoot: string) => T,
367-
gremlin?: (packageInfo: any) => void,
417+
gremlin?: (packageInfo: HalfFilledPackageJson) => void,
368418
compressAssembly = false,
369419
): T {
370420
const tmpdir = fs.mkdtempSync(path.join(os.tmpdir(), path.basename(__filename)));
@@ -430,3 +480,5 @@ function _stripUndefined(obj: { [key: string]: any } | undefined): { [key: strin
430480
}
431481
return obj;
432482
}
483+
484+
type DeepWriteable<T> = T extends object ? { -readonly [P in keyof T]: DeepWriteable<T[P]> } : T;

0 commit comments

Comments
 (0)