validate inferred typing names to be legal package names

src/harness/unittests/tsserverProjectSystem.ts

@@ -51,8 +51,8 @@ namespace ts.projectSystem {
 
     export class TestTypingsInstaller extends TI.TypingsInstaller implements server.ITypingsInstaller {
         protected projectService: server.ProjectService;
-        constructor(readonly globalTypingsCacheLocation: string, throttleLimit: number, readonly installTypingHost: server.ServerHost) {
-            super(globalTypingsCacheLocation, "npm", safeList.path, throttleLimit);
+        constructor(readonly globalTypingsCacheLocation: string, throttleLimit: number, readonly installTypingHost: server.ServerHost, log?: TI.Log) {
+            super(globalTypingsCacheLocation, "npm", safeList.path, throttleLimit, log);
             this.init();
         }
 

src/harness/unittests/typingsInstaller.ts

@@ -11,11 +11,12 @@ namespace ts.projectSystem {
     }
 
     class Installer extends TestTypingsInstaller {
-        constructor(host: server.ServerHost, p?: InstallerParams) {
+        constructor(host: server.ServerHost, p?: InstallerParams, log?: TI.Log) {
             super(
                 (p && p.globalTypingsCacheLocation) || "/a/data",
                 (p && p.throttleLimit) || 5,
-                host);
+                host,
+                log);
         }
 
         installAll(expectedView: typeof TI.NpmViewRequest[], expectedInstall: typeof TI.NpmInstallRequest[]) {
@@ -685,10 +686,10 @@ namespace ts.projectSystem {
             };
             const bowerJson = {
                 path: "/bower.json",
-                content: JSON.stringify({
-                        "dependencies": {
-                            "jquery": "^3.1.0"
-                        }
+                content: JSON.stringify({
+                        "dependencies": {
+                            "jquery": "^3.1.0"
+                        }
                     })
             };
             const jqueryDTS = {
@@ -720,4 +721,60 @@ namespace ts.projectSystem {
             checkProjectActualFiles(p, [app.path, jqueryDTS.path]);
         });
     });
+
+    describe("Validate package name:", () => {
+        it ("name cannot be too long", () => {
+            let packageName = "a";
+            for (let i = 0; i < 8; i++) {
+                packageName += packageName;
+            }
+            assert.equal(TI.validatePackageName(packageName), TI.PackageNameValidationResult.NameTooLong);
+        });
+        it ("name cannot start with dot", () => {
+            assert.equal(TI.validatePackageName(".foo"), TI.PackageNameValidationResult.NameStartsWithDot);
+        });
+        it ("name cannot start with underscore", () => {
+            assert.equal(TI.validatePackageName("_foo"), TI.PackageNameValidationResult.NameStartsWithUnderscore);
+        });
+        it ("scoped packages not supported", () => {
+            assert.equal(TI.validatePackageName("@scope/bar"), TI.PackageNameValidationResult.ScopedPackagesNotSupported);
+        });
+        it ("non URI safe characters are not supported", () => {
+            assert.equal(TI.validatePackageName("  scope  "), TI.PackageNameValidationResult.NameContainsNonURISafeCharacters);
+            assert.equal(TI.validatePackageName("; say ‘Hello from TypeScript!’ #"), TI.PackageNameValidationResult.NameContainsNonURISafeCharacters);
+            assert.equal(TI.validatePackageName("a/b/c"), TI.PackageNameValidationResult.NameContainsNonURISafeCharacters);
+        });
+    });
+
+    describe("Invalid package names", () => {
+        it ("should not be installed", () => {
+            const f1 = {
+                path: "/a/b/app.js",
+                content: "let x = 1"
+            };
+            const packageJson = {
+                path: "/a/b/package.json",
+                content: JSON.stringify({
+                    "dependencies": {
+                        "; say ‘Hello from TypeScript!’ #": "0.0.x"
+                    }
+                })
+            };
+            const messages: string[] = [];
+            const host = createServerHost([f1, packageJson]);
+            const installer = new (class extends Installer {
+                constructor() {
+                    super(host, { globalTypingsCacheLocation: "/tmp" }, { isEnabled: () => true, writeLine: msg => messages.push(msg) });
+                }
+                runCommand(requestKind: TI.RequestKind, requestId: number, command: string, cwd: string, cb: server.typingsInstaller.RequestCompletedAction) {
+                    assert(false, "runCommand should not be invoked");
+                }
+            })();
+            const projectService = createProjectService(host, { typingsInstaller: installer });
+            projectService.openClientFile(f1.path);
+
+            installer.checkPendingCommands([]);
+            assert.isTrue(messages.indexOf("Package name '; say ‘Hello from TypeScript!’ #' contains non URI safe characters") > 0, "should find package with invalid name");
+        });
+    });
 }

src/server/typingsInstaller/typingsInstaller.ts

@@ -23,6 +23,43 @@ namespace ts.server.typingsInstaller {
         return result.resolvedModule && result.resolvedModule.resolvedFileName;
     }
 
+    export enum PackageNameValidationResult {
+        Ok,
+        ScopedPackagesNotSupported,
+        NameTooLong,
+        NameStartsWithDot,
+        NameStartsWithUnderscore,
+        NameContainsNonURISafeCharacters
+    }
+
+
+    export const MaxPackageNameLength = 214;
+    /**
+     * Validates package name using rules defined at https://docs.npmjs.com/files/package.json 
+     */
+    export function validatePackageName(packageName: string): PackageNameValidationResult {
+        Debug.assert(!!packageName, "Package name is not specified");
+        if (packageName.length > MaxPackageNameLength) {
+            return PackageNameValidationResult.NameTooLong;
+        }
+        if (packageName.charCodeAt(0) === CharacterCodes.dot) {
+            return PackageNameValidationResult.NameStartsWithDot;
+        }
+        if (packageName.charCodeAt(0) === CharacterCodes._) {
+            return PackageNameValidationResult.NameStartsWithUnderscore;
+        }
+        // check if name is scope package like: starts with @ and has one '/' in the middle
+        // scoped packages are not currently supported
+        // TODO: when support will be added we'll need to split and check both scope and package name
+        if (/^@[^/]+\/[^/]+$/.test(packageName)) {
+            return PackageNameValidationResult.ScopedPackagesNotSupported;
+        }
+        if (encodeURIComponent(packageName) !== packageName) {
+            return PackageNameValidationResult.NameContainsNonURISafeCharacters;
+        }
+        return PackageNameValidationResult.Ok;
+    }
+
     export const NpmViewRequest: "npm view" = "npm view";
     export const NpmInstallRequest: "npm install" = "npm install";
 
@@ -185,14 +222,54 @@ namespace ts.server.typingsInstaller {
             this.knownCachesSet[cacheLocation] = true;
         }
 
+        private filterTypings(typingsToInstall: string[]) {
+            if (typingsToInstall.length === 0) {
+                return typingsToInstall;
+            }
+            const result: string[] = [];
+            for (const typing of typingsToInstall) {
+                if (this.missingTypingsSet[typing]) {
+                    continue;
+                }
+                const validationResult = validatePackageName(typing);
+                if (validationResult === PackageNameValidationResult.Ok) {
+                    result.push(typing);
+                }
+                else {
+                    // add typing name to missing set so we won't process it again
+                    this.missingTypingsSet[typing] = true;
+                    if (this.log.isEnabled()) {
+                        switch (validationResult) {
+                            case PackageNameValidationResult.NameTooLong:
+                                this.log.writeLine(`Package name '${typing}' should be less than ${MaxPackageNameLength} characters`);
+                                break;
+                            case PackageNameValidationResult.NameStartsWithDot:
+                                this.log.writeLine(`Package name '${typing}' cannot start with '.'`);
+                                break;
+                            case PackageNameValidationResult.NameStartsWithUnderscore:
+                                this.log.writeLine(`Package name '${typing}' cannot start with '_'`);
+                                break;
+                            case PackageNameValidationResult.ScopedPackagesNotSupported:
+                                this.log.writeLine(`Package '${typing}' is scoped and currently is not supported`);
+                                break;
+                            case PackageNameValidationResult.NameContainsNonURISafeCharacters:
+                                this.log.writeLine(`Package name '${typing}' contains non URI safe characters`);
+                                break;
+                        }
+                    }
+                }
+            }
+            return result;
+        }
+
         private installTypings(req: DiscoverTypings, cachePath: string, currentlyCachedTypings: string[], typingsToInstall: string[]) {
             if (this.log.isEnabled()) {
                 this.log.writeLine(`Installing typings ${JSON.stringify(typingsToInstall)}`);
             }
-            typingsToInstall = filter(typingsToInstall, x => !this.missingTypingsSet[x]);
+            typingsToInstall = this.filterTypings(typingsToInstall);
             if (typingsToInstall.length === 0) {
                 if (this.log.isEnabled()) {
-                    this.log.writeLine(`All typings are known to be missing - no need to go any further`);
+                    this.log.writeLine(`All typings are known to be missing or invalid - no need to go any further`);
                 }
                 return;
             }