Merge pull request #13157 from keymanapp/feat/developer/13109-check-t…

…hat-package-does-not-contain-itself

feat(developer): verify that packages do not contain themselves in kmc-package

developer/src/kmc-package/src/compiler/kmp-compiler.ts

@@ -105,12 +105,14 @@ export class KmpCompiler implements KeymanCompiler {
       return null;
     }
 
+    outputFilename = outputFilename ?? inputFilename.replace(/\.kps$/, '.kmp');
+
     //
     // Validate the package file
     //
 
     const validation = new PackageValidation(this.callbacks, this.options);
-    if(!validation.validate(inputFilename, kmpJsonData)) {
+    if(!validation.validate(inputFilename, outputFilename, kmpJsonData)) {
       return null;
     }
 
@@ -127,7 +129,7 @@ export class KmpCompiler implements KeymanCompiler {
       artifacts: {
         kmp: {
           data,
-          filename: outputFilename ?? inputFilename.replace(/\.kps$/, '.kmp')
+          filename: outputFilename
         }
       }
     }

developer/src/kmc-package/src/compiler/package-compiler-messages.ts

@@ -191,5 +191,24 @@ export class PackageCompilerMessages {
     `The format for version numbers should be number[.number[.number]]. Each
     number component should be an integer, without leading zeroes.`
   );
+
+  static ERROR_PackageMustNotContainItself = SevError | 0x0028;
+  static Error_PackageMustNotContainItself = (o:{outputFilename: string}) => m(
+    this.ERROR_PackageMustNotContainItself, `The package may not include a .kmp file of the same name '${def(o.outputFilename)}'.`, `
+    While it is possible for a package file to contain other .kmp package files,
+    it is an error for a package file to contain a package with the same name as
+    itself.
+
+    **Note:** it is not recommended to include other package files within a
+    package, as the user experience for installation and uninstallation is
+    complex, and not consistent across all platforms.
+
+    **Note**: Nested packages are not checked for validity or whether or not they
+    may violate this rule transitively.
+  `);
+
+  //------------------------------------------------------------------------------|
+  // max length of detail message lines (checked by verifyCompilerMessagesObject) |
+  //------------------------------------------------------------------------------|
 }
 

developer/src/kmc-package/src/compiler/package-validation.ts

@@ -12,7 +12,7 @@ export class PackageValidation {
   constructor(private callbacks: CompilerCallbacks, private options: CompilerOptions) {
   }
 
-  public validate(filename: string, kmpJson: KmpJsonFile.KmpJsonFile) {
+  public validate(filename: string, outputFilename: string, kmpJson: KmpJsonFile.KmpJsonFile) {
     if(!this.checkForModelsAndKeyboardsInSamePackage(kmpJson)) {
       return false;
     }
@@ -22,7 +22,7 @@ export class PackageValidation {
     if(!this.checkLexicalModels(filename, kmpJson)) {
       return false;
     }
-    if(!this.checkContentFiles(kmpJson)) {
+    if(!this.checkContentFiles(kmpJson, outputFilename)) {
       return false;
     }
     if(!this.checkPackageInfo(kmpJson)) {
@@ -130,17 +130,17 @@ export class PackageValidation {
     return true;
   }
 
-  private checkContentFiles(kmpJson: KmpJsonFile.KmpJsonFile): boolean {
+  private checkContentFiles(kmpJson: KmpJsonFile.KmpJsonFile, outputFilename: string): boolean {
     for(let file of kmpJson.files) {
-      if(!this.checkContentFile(file)) {
+      if(!this.checkContentFile(file, outputFilename)) {
         return false;
       }
     }
 
     return true;
   }
 
-  private checkContentFile(file: KmpJsonFile.KmpJsonFileContentFile): boolean {
+  private checkContentFile(file: KmpJsonFile.KmpJsonFileContentFile, outputFilename: string): boolean {
     const filename = this.callbacks.path.basename(file.name);
     const ext = this.callbacks.path.extname(filename);
     const base = filename.substring(0, filename.length-ext.length);
@@ -155,9 +155,20 @@ export class PackageValidation {
       return false;
     }
 
+    if(this.doesContentFileEqualOutputFilename(file, outputFilename)) {
+      this.callbacks.reportMessage(PackageCompilerMessages.Error_PackageMustNotContainItself({outputFilename}));
+      return false;
+    }
+
     return true;
   }
 
+  private doesContentFileEqualOutputFilename(file: KmpJsonFile.KmpJsonFileContentFile, outputFilename: string): boolean {
+    // case-insensitive comparison because Windows has case-insensitive filenames
+    return this.callbacks.path.basename(file.name).toLowerCase() ==
+      this.callbacks.path.basename(outputFilename).toLowerCase();
+  }
+
   private checkIfContentFileIsDangerous(file: KmpJsonFile.KmpJsonFileContentFile): boolean {
     const filename = this.callbacks.path.basename(file.name).toLowerCase();
 

developer/src/kmc-package/test/fixtures/invalid/error_package_must_not_contain_itself.kps

@@ -0,0 +1,37 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Package>
+  <System>
+    <KeymanDeveloperVersion>15.0.266.0</KeymanDeveloperVersion>
+    <FileVersion>7.0</FileVersion>
+  </System>
+  <Info>
+    <Name URL="">error_package_must_not_contain_itself</Name>
+    <Copyright URL="">© 2019 National Research Council Canada</Copyright>
+    <Author URL="mailto:[email protected]">Eddie Antonio Santos</Author>
+    <Version>1.0</Version>
+  </Info>
+  <Files>
+    <File>
+      <Name>error_package_must_not_contain_itself.kmp</Name>
+      <Description>error_package_must_not_contain_itself</Description>
+      <CopyLocation>0</CopyLocation>
+      <FileType>.kmp</FileType>
+    </File>
+    <File>
+      <Name>basic.kmx</Name>
+      <Description>Keyboard Basic</Description>
+      <CopyLocation>0</CopyLocation>
+      <FileType>.kmx</FileType>
+    </File>
+  </Files>
+  <Keyboards>
+    <Keyboard>
+      <Name>Basic</Name>
+      <ID>basic</ID>
+      <Version>1.0</Version>
+      <Languages>
+        <Language ID="km">Central Khmer (Khmer, Cambodia)</Language>
+      </Languages>
+    </Keyboard>
+  </Keyboards>
+</Package>

developer/src/kmc-package/test/messages.tests.ts

@@ -246,4 +246,6 @@ describe('PackageCompilerMessages', function () {
     await testForMessage(this, ['invalid', 'warn_package_version_is_unrecognized_format.kps'],
       PackageCompilerMessages.WARN_PackageVersionIsUnrecognizedFormat);
   });
+
+  // ERROR_PackageMustNotContainItself test in package-compiler.tests.ts
 });

developer/src/kmc-package/test/package-compiler.tests.ts

@@ -294,18 +294,30 @@ describe('KmpCompiler', function () {
 
   it(`should handle a range of valid BCP47 tags`, function () {
     const inputFilename = makePathToFixture('bcp47', 'valid_bcp47.kps');
+    const outputFilename = makePathToFixture('bcp47', 'valid_bcp47.kmp');
     const kmpJson = kmpCompiler.transformKpsToKmpObject(inputFilename);
     assert.isNotNull(kmpJson);
     const validation = new PackageValidation(callbacks, {});
-    assert.isTrue(validation.validate(inputFilename, kmpJson));
+    assert.isTrue(validation.validate(inputFilename, outputFilename, kmpJson));
   });
 
   it(`should reject an invalid BCP47 tag`, function () {
     const inputFilename = makePathToFixture('bcp47', 'invalid_bcp47_1.kps');
+    const outputFilename = makePathToFixture('bcp47', 'invalid_bcp47_1.kmp');
     const kmpJson = kmpCompiler.transformKpsToKmpObject(inputFilename);
     assert.isNotNull(kmpJson);
     const validation = new PackageValidation(callbacks, {});
-    assert.isFalse(validation.validate(inputFilename, kmpJson));
+    assert.isFalse(validation.validate(inputFilename, outputFilename, kmpJson));
+  });
+
+  it(`should reject an package that contains itself`, function () {
+    const inputFilename = makePathToFixture('invalid', 'error_package_must_not_contain_itself.kps');
+    const outputFilename = makePathToFixture('invalid', 'error_package_must_not_contain_itself.kmp');
+    const kmpJson = kmpCompiler.transformKpsToKmpObject(inputFilename);
+    assert.isNotNull(kmpJson);
+    const validation = new PackageValidation(callbacks, {});
+    assert.isFalse(validation.validate(inputFilename, outputFilename, kmpJson));
+    assert.equal(callbacks.messages[0].code, PackageCompilerMessages.ERROR_PackageMustNotContainItself);
   });
 
 });