Merge pull request #5698 from keymanapp/fix/ios/block-desktop-only-install

fix(ios): prevents installation of packages without JS

ios/engine/KMEI/KeymanEngine/Classes/KeyboardKeymanPackage.swift

@@ -14,7 +14,7 @@ public class KeyboardKeymanPackage : TypedKeymanPackage<InstallableKeyboard> {
   override internal init(metadata: KMPMetadata, folder: URL) {
     super.init(metadata: metadata, folder: folder)
     self.keyboards = []
-    
+
     if let packagedKeyboards = metadata.keyboards {
       for keyboard in packagedKeyboards {
         keyboard.packageId = self.id
@@ -22,7 +22,7 @@ public class KeyboardKeymanPackage : TypedKeymanPackage<InstallableKeyboard> {
         if(keyboard.isValid && FileManager.default.fileExists(atPath: self.sourceFolder.appendingPathComponent("\(keyboard.keyboardId).js").path)) {
           keyboards.append(keyboard)
         } else {
-          log.debug("\(keyboard.name) not valid / corresponding file not found")
+          SentryManager.breadcrumbAndLog("\(keyboard.name) not valid / corresponding file not found")
         }
       }
     }

ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift

@@ -16,6 +16,8 @@ public enum KMPError : String, Error {
   case invalidPackage = "kmp-error-invalid"
   case fileSystem = "kmp-error-file-system"
   case copyFiles = "kmp-error-file-copying"
+  case unsupportedPackage = "kmp-error-unsupported"
+  case doesNotExist = "kmp-error-missing"
   // TODO:  Consider changing the parent type so that these may take arguments that may be used
   //        to provide better clarity.
   case wrongPackageType = "kmp-error-wrong-type"
@@ -380,35 +382,37 @@ public class KeymanPackage {
    * Typecast the return value to either KeyboardKeymanPackage or LexicalModelKeymanPackage for richly-typed
    * information about the package's contents.
    */
-  static public func parse(_ folder: URL) -> KeymanPackage? {
-    do {
-      var path = folder
-      path.appendPathComponent(kmpFile)
-      if FileManager.default.fileExists(atPath: path.path) {
-        let data = try Data(contentsOf: path, options: .mappedIfSafe)
-        let decoder = JSONDecoder()
+  static public func parse(_ folder: URL) throws -> KeymanPackage? {
+    var path = folder
+    path.appendPathComponent(kmpFile)
+    if FileManager.default.fileExists(atPath: path.path) {
+      let data = try Data(contentsOf: path, options: .mappedIfSafe)
+      let decoder = JSONDecoder()
 
-        var metadata: KMPMetadata
+      var metadata: KMPMetadata
 
-        do {
-          metadata = try decoder.decode(KMPMetadata.self, from: data)
-        } catch {
-          throw KMPError.noMetadata
-        }
-
-        switch metadata.packageType {
-          case .Keyboard:
-            return KeyboardKeymanPackage(metadata: metadata, folder: folder)
-          case .LexicalModel:
-            return LexicalModelKeymanPackage(metadata: metadata, folder: folder)
-          default:
-            throw KMPError.invalidPackage
-        }
+      do {
+        metadata = try decoder.decode(KMPMetadata.self, from: data)
+      } catch {
+        throw KMPError.noMetadata
+      }
+
+      var package: KeymanPackage
+
+      switch metadata.packageType {
+        case .Keyboard:
+          package = KeyboardKeymanPackage(metadata: metadata, folder: folder)
+        case .LexicalModel:
+          package = LexicalModelKeymanPackage(metadata: metadata, folder: folder)
+        default:
+          throw KMPError.invalidPackage
+      }
+
+      if package.resources.count == 0 {
+        throw KMPError.unsupportedPackage
+      } else {
+        return package
       }
-    } catch {
-      // It's not an app or engine error when the package itself is invalid.
-      // Definitely worth noting, though.
-      SentryManager.breadcrumbAndLog("error parsing keyman package: \(error)", sentryLevel: .error)
     }
     
     return nil
@@ -417,13 +421,19 @@ public class KeymanPackage {
   @available(*, deprecated, message: "Use of the completion block is unnecessary; this method now returns synchronously.")
   static public func extract(fileUrl: URL, destination: URL, complete: @escaping (KeymanPackage?) -> Void) throws {
     try unzipFile(fileUrl: fileUrl, destination: destination) {
-      complete(KeymanPackage.parse(destination))
+      do {
+        let package = try KeymanPackage.parse(destination)
+        complete(package)
+      } catch {
+        SentryManager.captureAndLog(error, sentryLevel: .info)
+        complete(nil)
+      }
     }
   }
 
   static public func extract(fileUrl: URL, destination: URL) throws -> KeymanPackage? {
     try unzipFile(fileUrl: fileUrl, destination: destination)
-    return KeymanPackage.parse(destination)
+    return try KeymanPackage.parse(destination)
   }
 
   static public func unzipFile(fileUrl: URL, destination: URL, complete: @escaping () -> Void = {}) throws {

ios/engine/KMEI/KeymanEngine/Classes/LexicalModelKeymanPackage.swift

@@ -24,7 +24,7 @@ public class LexicalModelKeymanPackage : TypedKeymanPackage<InstallableLexicalMo
         if(model.isValid && FileManager.default.fileExists(atPath: self.sourceFolder.appendingPathComponent("\(model.lexicalModelId).model.js").path)) {
           models.append(model)
         } else {
-          log.debug("\(model.name) not valid / corresponding file not found")
+          SentryManager.breadcrumbAndLog("\(model.name) not valid / corresponding file not found")
         }
       }
     }

ios/engine/KMEI/KeymanEngine/Classes/Manager.swift

@@ -625,7 +625,7 @@ public class Manager: NSObject, UIGestureRecognizerDelegate {
     
   // MARK: - Adhoc keyboards
   public func parseKbdKMP(_ folder: URL, isCustom: Bool) throws -> Void {
-    guard let kmp = KeymanPackage.parse(folder) as? KeyboardKeymanPackage else {
+    guard let kmp = try? KeymanPackage.parse(folder) as? KeyboardKeymanPackage else {
       throw KMPError.wrongPackageType
     }
 
@@ -640,7 +640,7 @@ public class Manager: NSObject, UIGestureRecognizerDelegate {
     
   // MARK: - Adhoc lexical models
   static public func parseLMKMP(_ folder: URL, isCustom: Bool) throws -> Void {
-    guard let kmp = KeymanPackage.parse(folder) as? LexicalModelKeymanPackage else {
+    guard let kmp = try? KeymanPackage.parse(folder) as? LexicalModelKeymanPackage else {
       throw KMPError.wrongPackageType
     }
 

ios/engine/KMEI/KeymanEngine/Classes/Resource Management/ResourceFileManager.swift

@@ -54,7 +54,7 @@ public class ResourceFileManager {
 
     var backingPackages: [KeymanPackage] = []
     for resource in userResources {
-      if let package = KeymanPackage.parse(Storage.active.resourceDir(for: resource)!) {
+      if let package = try? KeymanPackage.parse(Storage.active.resourceDir(for: resource)!) {
         // On successful parse, just ensure that we haven't already listed the package.
         if !backingPackages.contains(where: { $0.id == package.id }) {
           backingPackages.append(package)
@@ -86,14 +86,14 @@ public class ResourceFileManager {
 
   public func getInstalledPackage<Resource: LanguageResource>(for resource: Resource) -> Resource.Package? {
     if let packageDir = Storage.active.resourceDir(for: resource) {
-      return KeymanPackage.parse(packageDir) as? Resource.Package
+      return try? KeymanPackage.parse(packageDir) as? Resource.Package
     } else {
       return nil
     }
   }
 
   public func getInstalledPackage(withKey key: KeymanPackage.Key) -> KeymanPackage? {
-    return KeymanPackage.parse(Storage.active.packageDir(forKey: key))
+    return try? KeymanPackage.parse(Storage.active.packageDir(forKey: key))
   }
 
   internal func packageDownloadTempPath(forKey key: KeymanPackage.Key) -> URL {
@@ -183,10 +183,14 @@ public class ResourceFileManager {
     var extractionFolder = cacheDirectory
     extractionFolder.appendPathComponent("temp/\(archiveUrl.lastPathComponent)")
 
-    if let kmp = try KeymanPackage.extract(fileUrl: archiveUrl, destination: extractionFolder) {
-      return kmp
-    } else {
-      throw KMPError.invalidPackage
+    do {
+      if let package = try KeymanPackage.extract(fileUrl: archiveUrl, destination: extractionFolder) {
+        return package
+      } else {
+        throw KMPError.doesNotExist
+      }
+    } catch {
+      throw error
     }
   }
 

ios/engine/KMEI/KeymanEngine/en.lproj/Localizable.strings

@@ -118,12 +118,18 @@
 /* Error opening a Keyman package - package is not valid */
 "kmp-error-invalid" = "The package's file is corrupted.";
 
+/* Error opening a Keyman package - it does not exist / the specified location is wrong */
+"kmp-error-missing" = "The specified package does not exist.";
+
 /* Error installing a Keyman package - expected resource (keyboard or dictionary) is missing */
 "kmp-error-missing-resource" = "This package does not contain the requested keyboard or dictionary.";
 
 /* Error opening a Keyman package - cannot parse contents */
 "kmp-error-no-metadata" = "This package was not properly built - contents unknown.";
 
+/* Error opening a Keyman package - package's contents are for desktop platforms only */
+"kmp-error-unsupported" = "This package does not include support for your device.";
+
 /* Error opening a Keyman package - package contains unexpected resource (keyboard or dictionary) type */
 "kmp-error-wrong-type" = "This package does not contain the expected resource type.";
 

ios/keyman/Keyman/Keyman/PackageBrowserViewController.swift

@@ -65,7 +65,15 @@ class PackageBrowserViewController: UIDocumentPickerViewController, UIDocumentPi
       return
     }
 
-    if let package = rfm.prepareKMPInstall(from: destinationUrl, alertHost: self) {
+    // The package browser view has usually self-dismissed at this point and cannot
+    // present error message alerts.  We need to find something in the view
+    // hierarchy that can present the error.
+    var alertVC: UIViewController = self
+    if self.view.superview == nil && self.navVC != nil {
+      alertVC = self.navVC!
+    }
+
+    if let package = rfm.prepareKMPInstall(from: destinationUrl, alertHost: alertVC) {
       // These type checks are necessary due to generic constraints.
       if let kbdPackage = package as? KeyboardKeymanPackage {
         doPrompt(for: kbdPackage, withAssociators: [.lexicalModels])