Skip to content

Commit 9afb07d

Browse files
vladimavladima
authored
Merge pull request microsoft#11300 from Microsoft/vladima/validate-package-names
Validate inferred typing names to be legal package names
2 parents 2866552 + d120bcd commit 9afb07d

3 files changed

Lines changed: 144 additions & 10 deletions

File tree

‎src/harness/unittests/tsserverProjectSystem.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@ namespace ts.projectSystem {
5151

5252
export class TestTypingsInstaller extends TI.TypingsInstaller implements server.ITypingsInstaller {
5353
protected projectService: server.ProjectService;
54-
constructor(readonly globalTypingsCacheLocation: string, throttleLimit: number, readonly installTypingHost: server.ServerHost) {
55-
super(globalTypingsCacheLocation, "npm", safeList.path, throttleLimit);
54+
constructor(readonly globalTypingsCacheLocation: string, throttleLimit: number, readonly installTypingHost: server.ServerHost, log?: TI.Log) {
55+
super(globalTypingsCacheLocation, "npm", safeList.path, throttleLimit, log);
5656
this.init();
5757
}
5858

‎src/harness/unittests/typingsInstaller.ts‎

Lines changed: 63 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,12 @@ namespace ts.projectSystem {
1111
}
1212

1313
class Installer extends TestTypingsInstaller {
14-
constructor(host: server.ServerHost, p?: InstallerParams) {
14+
constructor(host: server.ServerHost, p?: InstallerParams, log?: TI.Log) {
1515
super(
1616
(p && p.globalTypingsCacheLocation) || "/a/data",
1717
(p && p.throttleLimit) || 5,
18-
host);
18+
host,
19+
log);
1920
}
2021

2122
installAll(expectedView: typeof TI.NpmViewRequest[], expectedInstall: typeof TI.NpmInstallRequest[]) {
@@ -685,10 +686,10 @@ namespace ts.projectSystem {
685686
};
686687
const bowerJson = {
687688
path: "/bower.json",
688-
content: JSON.stringify({
689-
"dependencies": {
690-
"jquery": "^3.1.0"
691-
}
689+
content: JSON.stringify({
690+
"dependencies": {
691+
"jquery": "^3.1.0"
692+
}
692693
})
693694
};
694695
const jqueryDTS = {
@@ -720,4 +721,60 @@ namespace ts.projectSystem {
720721
checkProjectActualFiles(p, [app.path, jqueryDTS.path]);
721722
});
722723
});
724+
725+
describe("Validate package name:", () => {
726+
it ("name cannot be too long", () => {
727+
let packageName = "a";
728+
for (let i = 0; i < 8; i++) {
729+
packageName += packageName;
730+
}
731+
assert.equal(TI.validatePackageName(packageName), TI.PackageNameValidationResult.NameTooLong);
732+
});
733+
it ("name cannot start with dot", () => {
734+
assert.equal(TI.validatePackageName(".foo"), TI.PackageNameValidationResult.NameStartsWithDot);
735+
});
736+
it ("name cannot start with underscore", () => {
737+
assert.equal(TI.validatePackageName("_foo"), TI.PackageNameValidationResult.NameStartsWithUnderscore);
738+
});
739+
it ("scoped packages not supported", () => {
740+
assert.equal(TI.validatePackageName("@scope/bar"), TI.PackageNameValidationResult.ScopedPackagesNotSupported);
741+
});
742+
it ("non URI safe characters are not supported", () => {
743+
assert.equal(TI.validatePackageName(" scope "), TI.PackageNameValidationResult.NameContainsNonURISafeCharacters);
744+
assert.equal(TI.validatePackageName("; say ‘Hello from TypeScript!’ #"), TI.PackageNameValidationResult.NameContainsNonURISafeCharacters);
745+
assert.equal(TI.validatePackageName("a/b/c"), TI.PackageNameValidationResult.NameContainsNonURISafeCharacters);
746+
});
747+
});
748+
749+
describe("Invalid package names", () => {
750+
it ("should not be installed", () => {
751+
const f1 = {
752+
path: "/a/b/app.js",
753+
content: "let x = 1"
754+
};
755+
const packageJson = {
756+
path: "/a/b/package.json",
757+
content: JSON.stringify({
758+
"dependencies": {
759+
"; say ‘Hello from TypeScript!’ #": "0.0.x"
760+
}
761+
})
762+
};
763+
const messages: string[] = [];
764+
const host = createServerHost([f1, packageJson]);
765+
const installer = new (class extends Installer {
766+
constructor() {
767+
super(host, { globalTypingsCacheLocation: "/tmp" }, { isEnabled: () => true, writeLine: msg => messages.push(msg) });
768+
}
769+
runCommand(requestKind: TI.RequestKind, requestId: number, command: string, cwd: string, cb: server.typingsInstaller.RequestCompletedAction) {
770+
assert(false, "runCommand should not be invoked");
771+
}
772+
})();
773+
const projectService = createProjectService(host, { typingsInstaller: installer });
774+
projectService.openClientFile(f1.path);
775+
776+
installer.checkPendingCommands([]);
777+
assert.isTrue(messages.indexOf("Package name '; say ‘Hello from TypeScript!’ #' contains non URI safe characters") > 0, "should find package with invalid name");
778+
});
779+
});
723780
}

‎src/server/typingsInstaller/typingsInstaller.ts‎

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,43 @@ namespace ts.server.typingsInstaller {
2323
return result.resolvedModule && result.resolvedModule.resolvedFileName;
2424
}
2525

26+
export enum PackageNameValidationResult {
27+
Ok,
28+
ScopedPackagesNotSupported,
29+
NameTooLong,
30+
NameStartsWithDot,
31+
NameStartsWithUnderscore,
32+
NameContainsNonURISafeCharacters
33+
}
34+
35+
36+
export const MaxPackageNameLength = 214;
37+
/**
38+
* Validates package name using rules defined at https://docs.npmjs.com/files/package.json
39+
*/
40+
export function validatePackageName(packageName: string): PackageNameValidationResult {
41+
Debug.assert(!!packageName, "Package name is not specified");
42+
if (packageName.length > MaxPackageNameLength) {
43+
return PackageNameValidationResult.NameTooLong;
44+
}
45+
if (packageName.charCodeAt(0) === CharacterCodes.dot) {
46+
return PackageNameValidationResult.NameStartsWithDot;
47+
}
48+
if (packageName.charCodeAt(0) === CharacterCodes._) {
49+
return PackageNameValidationResult.NameStartsWithUnderscore;
50+
}
51+
// check if name is scope package like: starts with @ and has one '/' in the middle
52+
// scoped packages are not currently supported
53+
// TODO: when support will be added we'll need to split and check both scope and package name
54+
if (/^@[^/]+\/[^/]+$/.test(packageName)) {
55+
return PackageNameValidationResult.ScopedPackagesNotSupported;
56+
}
57+
if (encodeURIComponent(packageName) !== packageName) {
58+
return PackageNameValidationResult.NameContainsNonURISafeCharacters;
59+
}
60+
return PackageNameValidationResult.Ok;
61+
}
62+
2663
export const NpmViewRequest: "npm view" = "npm view";
2764
export const NpmInstallRequest: "npm install" = "npm install";
2865

@@ -185,14 +222,54 @@ namespace ts.server.typingsInstaller {
185222
this.knownCachesSet[cacheLocation] = true;
186223
}
187224

225+
private filterTypings(typingsToInstall: string[]) {
226+
if (typingsToInstall.length === 0) {
227+
return typingsToInstall;
228+
}
229+
const result: string[] = [];
230+
for (const typing of typingsToInstall) {
231+
if (this.missingTypingsSet[typing]) {
232+
continue;
233+
}
234+
const validationResult = validatePackageName(typing);
235+
if (validationResult === PackageNameValidationResult.Ok) {
236+
result.push(typing);
237+
}
238+
else {
239+
// add typing name to missing set so we won't process it again
240+
this.missingTypingsSet[typing] = true;
241+
if (this.log.isEnabled()) {
242+
switch (validationResult) {
243+
case PackageNameValidationResult.NameTooLong:
244+
this.log.writeLine(`Package name '${typing}' should be less than ${MaxPackageNameLength} characters`);
245+
break;
246+
case PackageNameValidationResult.NameStartsWithDot:
247+
this.log.writeLine(`Package name '${typing}' cannot start with '.'`);
248+
break;
249+
case PackageNameValidationResult.NameStartsWithUnderscore:
250+
this.log.writeLine(`Package name '${typing}' cannot start with '_'`);
251+
break;
252+
case PackageNameValidationResult.ScopedPackagesNotSupported:
253+
this.log.writeLine(`Package '${typing}' is scoped and currently is not supported`);
254+
break;
255+
case PackageNameValidationResult.NameContainsNonURISafeCharacters:
256+
this.log.writeLine(`Package name '${typing}' contains non URI safe characters`);
257+
break;
258+
}
259+
}
260+
}
261+
}
262+
return result;
263+
}
264+
188265
private installTypings(req: DiscoverTypings, cachePath: string, currentlyCachedTypings: string[], typingsToInstall: string[]) {
189266
if (this.log.isEnabled()) {
190267
this.log.writeLine(`Installing typings ${JSON.stringify(typingsToInstall)}`);
191268
}
192-
typingsToInstall = filter(typingsToInstall, x => !this.missingTypingsSet[x]);
269+
typingsToInstall = this.filterTypings(typingsToInstall);
193270
if (typingsToInstall.length === 0) {
194271
if (this.log.isEnabled()) {
195-
this.log.writeLine(`All typings are known to be missing - no need to go any further`);
272+
this.log.writeLine(`All typings are known to be missing or invalid - no need to go any further`);
196273
}
197274
return;
198275
}

0 commit comments

Comments
 (0)