diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 988e87284c3..5b934cc4b0b 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/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(); } diff --git a/src/harness/unittests/typingsInstaller.ts b/src/harness/unittests/typingsInstaller.ts index 117ca65cb64..d79a1a7723c 100644 --- a/src/harness/unittests/typingsInstaller.ts +++ b/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"); + }); + }); } \ No newline at end of file diff --git a/src/server/typingsInstaller/typingsInstaller.ts b/src/server/typingsInstaller/typingsInstaller.ts index 5437328b581..0ed2ccce99d 100644 --- a/src/server/typingsInstaller/typingsInstaller.ts +++ b/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; }