Skip to content

Commit

Permalink
Merge pull request #9958 from keymanapp/fix/fv/9819-replace-zip-library
Browse files Browse the repository at this point in the history
fix(ios): fv: replace Zip framework to prevent crash on startup 🏠
  • Loading branch information
sgschantz authored Nov 22, 2023
2 parents 6aba06c + e5166c7 commit 0b02b39
Show file tree
Hide file tree
Showing 15 changed files with 189 additions and 57 deletions.
2 changes: 1 addition & 1 deletion ios/Cartfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
github "marmelroy/Zip"
github "weichsel/ZIPFoundation" ~> 0.9
github "DaveWoodCom/XCGLogger" ~> 6.1.0
github "devicekit/DeviceKit" ~> 5.0
github "ashleymills/Reachability.swift"
Expand Down
4 changes: 2 additions & 2 deletions ios/Cartfile.resolved
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
github "DaveWoodCom/XCGLogger" "6.1.0"
github "ashleymills/Reachability.swift" "v5.1.0"
github "devicekit/DeviceKit" "5.0.0"
github "devicekit/DeviceKit" "5.1.0"
github "getsentry/sentry-cocoa" "6.2.1"
github "marmelroy/Zip" "2.1.2"
github "weichsel/ZIPFoundation" "0.9.17"
16 changes: 7 additions & 9 deletions ios/engine/KMEI/KeymanEngine.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
1645D5952036C6FF0076C51B /* KeymanPackage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1645D5942036C6FF0076C51B /* KeymanPackage.swift */; };
1645D5972036C9F80076C51B /* KMPKeyboard.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1645D5962036C9F80076C51B /* KMPKeyboard.swift */; };
165EB3A12098993900040A69 /* KeyboardError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 165EB3A02098993900040A69 /* KeyboardError.swift */; };
296EF2C72AFA26C700E3E384 /* ZIPFoundation.xcframework in Frameworks */ = {isa = PBXBuildFile; fileRef = 296EF2C62AFA26C700E3E384 /* ZIPFoundation.xcframework */; };
377D10DE26846B8900467431 /* SpacebarTextViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 377D10DD26846B8900467431 /* SpacebarTextViewController.swift */; };
6CD5DFAA150F6DC8007A5DDE /* icon.png in Resources */ = {isa = PBXBuildFile; fileRef = 6CD5DFA8150F6DC8007A5DDE /* icon.png */; };
6CD5DFAB150F6DC8007A5DDE /* [email protected] in Resources */ = {isa = PBXBuildFile; fileRef = 6CD5DFA9150F6DC8007A5DDE /* [email protected] */; };
Expand Down Expand Up @@ -121,7 +122,6 @@
CE5C8BE324B5B3BA00FAFB7F /* Queries+LexicalModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = CE5C8BE224B5B3BA00FAFB7F /* Queries+LexicalModel.swift */; };
CE5C8BE524B5BD1C00FAFB7F /* QueryModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CE5C8BE424B5BD1C00FAFB7F /* QueryModelTests.swift */; };
CE5EDDC526522EAA001733AC /* XCGLogger.xcframework in Frameworks */ = {isa = PBXBuildFile; fileRef = CE5EDDC026522EA9001733AC /* XCGLogger.xcframework */; };
CE5EDDC626522EAA001733AC /* Zip.xcframework in Frameworks */ = {isa = PBXBuildFile; fileRef = CE5EDDC126522EA9001733AC /* Zip.xcframework */; };
CE5EDDC726522EAA001733AC /* Sentry.xcframework in Frameworks */ = {isa = PBXBuildFile; fileRef = CE5EDDC226522EAA001733AC /* Sentry.xcframework */; };
CE5EDDC826522EAA001733AC /* Reachability.xcframework in Frameworks */ = {isa = PBXBuildFile; fileRef = CE5EDDC326522EAA001733AC /* Reachability.xcframework */; };
CE5EDDC926522EAA001733AC /* ObjcExceptionBridging.xcframework in Frameworks */ = {isa = PBXBuildFile; fileRef = CE5EDDC426522EAA001733AC /* ObjcExceptionBridging.xcframework */; };
Expand Down Expand Up @@ -308,6 +308,10 @@
2949146F2738DA6700400732 /* ff-NG */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "ff-NG"; path = "ff-NG.lproj/ResourceInfoView.strings"; sourceTree = "<group>"; };
294914702738DD7400400732 /* ff-NG */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "ff-NG"; path = "ff-NG.lproj/Localizable.strings"; sourceTree = "<group>"; };
294914712738DD9700400732 /* ff-NG */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = "ff-NG"; path = "ff-NG.lproj/Localizable.stringsdict"; sourceTree = "<group>"; };
296EF2C62AFA26C700E3E384 /* ZIPFoundation.xcframework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcframework; name = ZIPFoundation.xcframework; path = ../../Carthage/Build/ZIPFoundation.xcframework; sourceTree = "<group>"; };
297810FE297FAEDF007C886D /* kn */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = kn; path = kn.lproj/ResourceInfoView.strings; sourceTree = "<group>"; };
297810FF297FAEF8007C886D /* kn */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = kn; path = kn.lproj/Localizable.strings; sourceTree = "<group>"; };
29781100297FAF06007C886D /* kn */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = kn; path = kn.lproj/Localizable.stringsdict; sourceTree = "<group>"; };
298566B929802892004ACA95 /* cs */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = cs; path = cs.lproj/ResourceInfoView.strings; sourceTree = "<group>"; };
298566BA298028A8004ACA95 /* cs */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = cs; path = cs.lproj/Localizable.strings; sourceTree = "<group>"; };
298566BB298028AC004ACA95 /* cs */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = cs; path = cs.lproj/Localizable.stringsdict; sourceTree = "<group>"; };
Expand All @@ -320,9 +324,6 @@
298566DD2980E477004ACA95 /* ru */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ru; path = ru.lproj/ResourceInfoView.strings; sourceTree = "<group>"; };
298566DE2980E486004ACA95 /* ru */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ru; path = ru.lproj/Localizable.strings; sourceTree = "<group>"; };
298566DF2980E48C004ACA95 /* ru */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = ru; path = ru.lproj/Localizable.stringsdict; sourceTree = "<group>"; };
297810FE297FAEDF007C886D /* kn */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = kn; path = kn.lproj/ResourceInfoView.strings; sourceTree = "<group>"; };
297810FF297FAEF8007C886D /* kn */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = kn; path = kn.lproj/Localizable.strings; sourceTree = "<group>"; };
29781100297FAF06007C886D /* kn */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = kn; path = kn.lproj/Localizable.stringsdict; sourceTree = "<group>"; };
29B27FEF29062CF50036917E /* nl */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = nl; path = nl.lproj/ResourceInfoView.strings; sourceTree = "<group>"; };
29B27FF029062D100036917E /* nl */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = nl; path = nl.lproj/Localizable.strings; sourceTree = "<group>"; };
29B27FF129062D190036917E /* nl */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = nl; path = nl.lproj/Localizable.stringsdict; sourceTree = "<group>"; };
Expand Down Expand Up @@ -436,7 +437,6 @@
CE5C8BE424B5BD1C00FAFB7F /* QueryModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = QueryModelTests.swift; sourceTree = "<group>"; };
CE5EDDBB26522EA3001733AC /* DeviceKit.xcframework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcframework; name = DeviceKit.xcframework; path = ../../Carthage/Build/DeviceKit.xcframework; sourceTree = "<group>"; };
CE5EDDC026522EA9001733AC /* XCGLogger.xcframework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcframework; name = XCGLogger.xcframework; path = ../../Carthage/Build/XCGLogger.xcframework; sourceTree = "<group>"; };
CE5EDDC126522EA9001733AC /* Zip.xcframework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcframework; name = Zip.xcframework; path = ../../Carthage/Build/Zip.xcframework; sourceTree = "<group>"; };
CE5EDDC226522EAA001733AC /* Sentry.xcframework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcframework; name = Sentry.xcframework; path = ../../Carthage/Build/Sentry.xcframework; sourceTree = "<group>"; };
CE5EDDC326522EAA001733AC /* Reachability.xcframework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcframework; name = Reachability.xcframework; path = ../../Carthage/Build/Reachability.xcframework; sourceTree = "<group>"; };
CE5EDDC426522EAA001733AC /* ObjcExceptionBridging.xcframework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.xcframework; name = ObjcExceptionBridging.xcframework; path = ../../Carthage/Build/ObjcExceptionBridging.xcframework; sourceTree = "<group>"; };
Expand Down Expand Up @@ -557,8 +557,8 @@
isa = PBXFrameworksBuildPhase;
buildActionMask = 2147483647;
files = (
296EF2C72AFA26C700E3E384 /* ZIPFoundation.xcframework in Frameworks */,
CE5EDDC526522EAA001733AC /* XCGLogger.xcframework in Frameworks */,
CE5EDDC626522EAA001733AC /* Zip.xcframework in Frameworks */,
CE5EDDD52652372A001733AC /* DeviceKit.xcframework in Frameworks */,
CE5EDDC726522EAA001733AC /* Sentry.xcframework in Frameworks */,
CE5EDDC826522EAA001733AC /* Reachability.xcframework in Frameworks */,
Expand Down Expand Up @@ -946,11 +946,11 @@
F243888114BBD43000A3E055 /* Frameworks */ = {
isa = PBXGroup;
children = (
296EF2C62AFA26C700E3E384 /* ZIPFoundation.xcframework */,
CE5EDDC426522EAA001733AC /* ObjcExceptionBridging.xcframework */,
CE5EDDC326522EAA001733AC /* Reachability.xcframework */,
CE5EDDC226522EAA001733AC /* Sentry.xcframework */,
CE5EDDC026522EA9001733AC /* XCGLogger.xcframework */,
CE5EDDC126522EA9001733AC /* Zip.xcframework */,
CE5EDDBB26522EA3001733AC /* DeviceKit.xcframework */,
9A079DE52231A69D00581263 /* Foundation.framework */,
);
Expand Down Expand Up @@ -1609,7 +1609,6 @@
29BFA76C28294833009FCCC3 /* pl */,
29B27FF129062D190036917E /* nl */,
298566BB298028AC004ACA95 /* cs */,
29781100297FAF06007C886D /* kn */,
298566C72980C5CC004ACA95 /* sv */,
298566D32980D39F004ACA95 /* uk */,
298566DF2980E48C004ACA95 /* ru */,
Expand Down Expand Up @@ -1637,7 +1636,6 @@
29BFA76B28294813009FCCC3 /* pl */,
29B27FF029062D100036917E /* nl */,
298566BA298028A8004ACA95 /* cs */,
297810FF297FAEF8007C886D /* kn */,
298566C62980C5C7004ACA95 /* sv */,
298566D22980D39A004ACA95 /* uk */,
298566DE2980E486004ACA95 /* ru */,
Expand Down
39 changes: 30 additions & 9 deletions ios/engine/KMEI/KeymanEngine/Classes/KeymanPackage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
//

import Foundation
import Zip
import ZIPFoundation

// KMPErrors may be passed to UIAlertControllers, so they need localization.
public enum KMPError : String, Error {
Expand Down Expand Up @@ -443,24 +443,45 @@ 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) {
do {
let package = try KeymanPackage.parse(destination)
complete(package)
} catch {
SentryManager.captureAndLog(error, sentryLevel: .info)
complete(nil)
let fileManager = FileManager()
do {
try fileManager.unzipItem(at: fileUrl, to: destination)
let package = try KeymanPackage.parse(destination)
complete(package)
} catch {
SentryManager.captureAndLog(error, sentryLevel: .info)
complete(nil)
}
}

static public func clearDirectory(destination: URL) throws {
// First check to see if directory exists. If not, then do nothing.
var isDirectory: ObjCBool = false
if(FileManager.default.fileExists(atPath: destination.path, isDirectory: &isDirectory)){
if (isDirectory.boolValue) {
// it exists and is actually a directory, so remove every file it contains
log.info(" ***clearDirectory, destination exists:, \(destination)")
let fileArray = try FileManager.default.contentsOfDirectory(atPath: destination.path)
log.info(" ***clearDirectory, files to remove:, \(fileArray)")
try fileArray.forEach { file in
let fileUrl = destination.appendingPathComponent(file)
try FileManager.default.removeItem(atPath: fileUrl.path)
}
}
} else {
log.info(" ***clearDirectory, destination does not exist:, \(destination)")
}
}

static public func extract(fileUrl: URL, destination: URL) throws -> KeymanPackage? {
log.info(" ***extract, archiveUrl, \(fileUrl) \n to destination\(destination)")
try unzipFile(fileUrl: fileUrl, destination: destination)
return try KeymanPackage.parse(destination)
}

static public func unzipFile(fileUrl: URL, destination: URL, complete: @escaping () -> Void = {}) throws {
try Zip.unzipFile(fileUrl, destination: destination, overwrite: true, password: nil)
let fileManager = FileManager()
try fileManager.unzipItem(at: fileUrl, to: destination)
complete()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,9 @@ public class ResourceFileManager {
var extractionFolder = cacheDirectory
extractionFolder.appendPathComponent("temp/\(archiveUrl.lastPathComponent)")

// first clear extraction folder to avoid creating duplicates
try KeymanPackage.clearDirectory(destination: extractionFolder)

do {
if let package = try KeymanPackage.extract(fileUrl: archiveUrl, destination: extractionFolder) {
return package
Expand Down Expand Up @@ -372,6 +375,8 @@ public class ResourceFileManager {
do {
try copyWithOverwrite(from: package.sourceFolder,
to: Storage.active.packageDir(for: package)!)
let fileArray = try FileManager.default.contentsOfDirectory(atPath: Storage.active.packageDir(for: package)!.path)
log.info(" ***installed files in \(Storage.active.packageDir(for: package)!): \(fileArray)")
} catch {
log.error("Could not create installation directory and/or copy resources: \(error)")
throw KMPError.fileSystem
Expand Down
99 changes: 94 additions & 5 deletions ios/engine/KMEI/KeymanEngineTests/KeymanPackageTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,13 @@ class KeymanPackageTests: XCTestCase {
}
}

func testKeyboardPackageExtraction() throws {
func testKeyboardPackage_extractWithoutKmpExtension_succeeds() throws {
let cacheDirectory = FileManager.default.urls(for: .cachesDirectory, in: .userDomainMask)[0]
let khmerPackageZip = cacheDirectory.appendingPathComponent("khmer_angkor.kmp.zip")
let khmerPackageZip = cacheDirectory.appendingPathComponent("khmer_angkor.kmp")
try ResourceFileManager.shared.copyWithOverwrite(from: TestUtils.Keyboards.khmerAngkorKMP, to: khmerPackageZip)

let destinationFolderURL = cacheDirectory.appendingPathComponent("khmer_angkor")

// Requires that the source file is already .zip, not .kmp. It's a ZipUtils limitation.
do {
if let kmp = try KeymanPackage.extract(fileUrl: khmerPackageZip, destination: destinationFolderURL) {
// Run assertions on the package's kmp.info.
Expand All @@ -58,14 +57,104 @@ class KeymanPackageTests: XCTestCase {
}
}

func testKeyboardPackage_clearNonexistentDirectory_doesNothing() throws {
let cacheDirectory = FileManager.default.urls(for: .cachesDirectory, in: .userDomainMask)[0]
let destinationDirectory = cacheDirectory.appendingPathComponent("doesnotexist")

do {
// clear directory
try KeymanPackage.clearDirectory(destination: destinationDirectory)
} catch {
XCTFail("error clearing the nonexistent directory \(error)")
}
}

func testKeyboardPackage_clearEmptyDirectory_throwsNoError() throws {
let cacheDirectory = FileManager.default.urls(for: .cachesDirectory, in: .userDomainMask)[0]
let destinationDirectory = cacheDirectory.appendingPathComponent("destination")

do {
// create directory
try FileManager.default.createDirectory(
atPath: destinationDirectory.path,
withIntermediateDirectories: false,
attributes: nil
)

// clear directory
try KeymanPackage.clearDirectory(destination: destinationDirectory)
} catch {
XCTFail("error clearing the empty directory \(error)")
}

let fileArray = try FileManager.default.contentsOfDirectory(atPath: destinationDirectory.path)
XCTAssert(fileArray.count == 0, "directory still contains \(fileArray.count) items")
}

func testKeyboardPackage_clearNonEmptyDirectory_directoryIsEmpty() throws {
let cacheDirectory = FileManager.default.urls(for: .cachesDirectory, in: .userDomainMask)[0]
let destinationDirectory = cacheDirectory.appendingPathComponent("destination")

do {
// create directory
try FileManager.default.createDirectory(
atPath: destinationDirectory.path,
withIntermediateDirectories: false,
attributes: nil
)

// add some files
FileManager.default.createFile(atPath: destinationDirectory.appendingPathComponent("fileone").path, contents: nil)
FileManager.default.createFile(atPath: destinationDirectory.appendingPathComponent("filetwo").path, contents: nil)
FileManager.default.createFile(atPath: destinationDirectory.appendingPathComponent("filethree").path, contents: nil)

// clear directory
try KeymanPackage.clearDirectory(destination: destinationDirectory)
} catch {
XCTFail("error clearing the empty directory \(error)")
}

let fileArray = try FileManager.default.contentsOfDirectory(atPath: destinationDirectory.path)
XCTAssert(fileArray.count == 0, "directory still contains \(fileArray.count) items")
}

func testKeyboardPackage_extractTwice_noDuplicateFileError() throws {
let cacheDirectory = FileManager.default.urls(for: .cachesDirectory, in: .userDomainMask)[0]
let khmerPackageZip = cacheDirectory.appendingPathComponent("khmer_angkor.kmp")
try ResourceFileManager.shared.copyWithOverwrite(from: TestUtils.Keyboards.khmerAngkorKMP, to: khmerPackageZip)

let destinationFolderURL = cacheDirectory.appendingPathComponent("khmer_angkor")

do {
if let kmp = try KeymanPackage.extract(fileUrl: khmerPackageZip, destination: destinationFolderURL) {
log.info("*** first unzip of \(kmp.id)")
do {
// clear directory before second extract
try KeymanPackage.clearDirectory(destination: destinationFolderURL)

if let secondKmp = try KeymanPackage.extract(fileUrl: khmerPackageZip, destination: destinationFolderURL) {
log.info("*** second unzip of \(secondKmp.id)")
} else {
XCTAssert(false, "*** second unzip failed")
}
} catch {
XCTFail("unzip 2 failure with error \(error)")
}
} else {
XCTAssert(false, "*** first unzip failed")
}
} catch {
XCTFail("unzip 1 failure with error \(error)")
}
}

func testLexicalModelPackageExtraction() throws {
let cacheDirectory = FileManager.default.urls(for: .cachesDirectory, in: .userDomainMask)[0]
let mtntZip = cacheDirectory.appendingPathComponent("nrc.en.mtnt.kmp.zip")
let mtntZip = cacheDirectory.appendingPathComponent("nrc.en.mtnt.kmp")
try ResourceFileManager.shared.copyWithOverwrite(from: TestUtils.LexicalModels.mtntKMP, to: mtntZip)

let destinationFolderURL = cacheDirectory.appendingPathComponent("nrc.en.mtnt.model")

// Requires that the source file is already .zip, not .kmp. It's a ZipUtils limitation.
do {
if let kmp = try KeymanPackage.extract(fileUrl: mtntZip, destination: destinationFolderURL) {
// Run assertions on the package's kmp.info.
Expand Down
2 changes: 2 additions & 0 deletions ios/engine/KMEI/KeymanEngineTests/ResourceUpdateTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,12 @@ class ResourceUpdateTests: XCTestCase {

wait(for: [queryExpectation], timeout: 5)

/*
// Uses the XCTest 'attachment' system to retrieve the desired file.
self.add(try TestUtils.EngineStateBundler.createBundle(withName: "khmer_angkor update base"))
log.info("Bundle archived and attached to test's report.")
*/
}

func testCacheCurrent() {
Expand Down
Loading

0 comments on commit 0b02b39

Please sign in to comment.