From d48b5194e1f716c9aa350965b98ee163931da13f Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Mon, 16 Aug 2021 10:58:50 +0200 Subject: [PATCH 1/3] fix(linux): Check for valid kmp file Before trying to install we now check if it's a valid zip file. This fixes #5575. --- linux/keyman-config/keyman_config/handle_install.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/linux/keyman-config/keyman_config/handle_install.py b/linux/keyman-config/keyman_config/handle_install.py index 87541df3da..f332ef6717 100644 --- a/linux/keyman-config/keyman_config/handle_install.py +++ b/linux/keyman-config/keyman_config/handle_install.py @@ -5,6 +5,7 @@ from urllib.parse import parse_qs, urlparse from keyman_config import KeymanComUrl, __tier__ from keyman_config.install_window import InstallKmpWindow from keyman_config.get_kmp import get_download_folder, download_kmp_file +from zipfile import is_zipfile def download_and_install_package(url): @@ -42,6 +43,10 @@ def download_and_install_package(url): logging.critical("Invalid URL: " + url) return + if not is_zipfile(packageFile): + logging.critical("Not a valid KMP package: " + url) + return + if packageFile and not _install_package(packageFile, bcp47): logging.log(severity, "Can't find file " + url) From 58742b1a468b36ca5e6f9d0ca6fd05716d5690b6 Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Mon, 16 Aug 2021 11:03:23 +0200 Subject: [PATCH 2/3] fix(linux): Check for valid zip file in km-package-install as well --- linux/keyman-config/km-package-install | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/linux/keyman-config/km-package-install b/linux/keyman-config/km-package-install index 9a26d32b75..a4cffddb72 100755 --- a/linux/keyman-config/km-package-install +++ b/linux/keyman-config/km-package-install @@ -9,6 +9,7 @@ from keyman_config.uninstall_kmp import uninstall_kmp import datetime import time +from zipfile import is_zipfile def get_languages(): @@ -120,7 +121,7 @@ def main(): if args.file: name, ext = os.path.splitext(args.file) - if ext != ".kmp": + if ext != ".kmp" or not is_zipfile(args.file): logging.error("km-package-install: Input file %s is not a kmp file.", args.file) logging.error("km-package-install -f ") sys.exit(2) From a98300f769dd6dbb2bb914e818ab1cec6f334018 Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Mon, 16 Aug 2021 11:56:05 +0200 Subject: [PATCH 3/3] fix(linux): Fix failing tests --- .../tests/test_gnome_keyboards_util.py | 4 +- .../tests/test_handle_install.py | 47 +++++++++++++++---- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/linux/keyman-config/tests/test_gnome_keyboards_util.py b/linux/keyman-config/tests/test_gnome_keyboards_util.py index 67bf01b74e..ad6d116fbe 100644 --- a/linux/keyman-config/tests/test_gnome_keyboards_util.py +++ b/linux/keyman-config/tests/test_gnome_keyboards_util.py @@ -71,14 +71,14 @@ class GnomeKeyboardsUtilTests(unittest.TestCase): self.MockSettingsClass.return_value.set_value.assert_called_once_with( "sources", keyboards) - @patch('keyman_config.install_kmp.os.system') + @patch('keyman_config.os.system') def test_IsGnomeShell_RunningGnomeShell(self, mockSystem): # Setup mockSystem.return_value = 0 # Execute/Verify self.assertEqual(is_gnome_shell(), True) - @patch('keyman_config.install_kmp.os.system') + @patch('keyman_config.os.system') def test_IsGnomeShell_NotRunningGnomeShell(self, mockSystem): # Setup mockSystem.return_value = 1 diff --git a/linux/keyman-config/tests/test_handle_install.py b/linux/keyman-config/tests/test_handle_install.py index 981e38a41e..4e78bbeeb6 100644 --- a/linux/keyman-config/tests/test_handle_install.py +++ b/linux/keyman-config/tests/test_handle_install.py @@ -8,53 +8,79 @@ from keyman_config.handle_install import download_and_install_package class HandleInstallTests(unittest.TestCase): + @patch('keyman_config.handle_install.is_zipfile') @patch('keyman_config.handle_install._install_package') - def test_downloadAndInstallPackage_file(self, installPackageMethod): + def test_downloadAndInstallPackage_file(self, installPackageMethod, mockIsZipfile): + # Setup + mockIsZipfile.return_value = True + # Execute download_and_install_package('/tmp/keyboard/sil_euro_latin.kmp') # Verify installPackageMethod.assert_called_with('/tmp/keyboard/sil_euro_latin.kmp', '') + @patch('keyman_config.handle_install.is_zipfile') @patch('keyman_config.handle_install._install_package') - def test_downloadAndInstallPackage_fileUrl(self, installPackageMethod): + def test_downloadAndInstallPackage_fileUrl(self, installPackageMethod, mockIsZipfile): + # Setup + mockIsZipfile.return_value = True + # Execute download_and_install_package('file:///tmp/keyboard/sil_euro_latin.kmp') # Verify installPackageMethod.assert_called_with('/tmp/keyboard/sil_euro_latin.kmp', '') + @patch('keyman_config.handle_install.is_zipfile') @patch('keyman_config.handle_install._install_package') - def test_downloadAndInstallPackage_fileUrlWithBcp47(self, installPackageMethod): + def test_downloadAndInstallPackage_fileUrlWithBcp47(self, installPackageMethod, + mockIsZipfile): + # Setup + mockIsZipfile.return_value = True + # Execute download_and_install_package('file:///tmp/keyboard/sil_euro_latin.kmp?bcp47=dyo') # Verify installPackageMethod.assert_called_with('/tmp/keyboard/sil_euro_latin.kmp', 'dyo') + @patch('keyman_config.handle_install.is_zipfile') @patch('keyman_config.handle_install._install_package') - def test_downloadAndInstallPackage_fileUrlWithBcp47AndVersion(self, installPackageMethod): + def test_downloadAndInstallPackage_fileUrlWithBcp47AndVersion(self, installPackageMethod, + mockIsZipfile): + # Setup + mockIsZipfile.return_value = True + # Execute download_and_install_package('file:///tmp/keyboard/sil_euro_latin.kmp?bcp47=dyo&version=1') # Verify installPackageMethod.assert_called_with('/tmp/keyboard/sil_euro_latin.kmp', 'dyo') + @patch('keyman_config.handle_install.is_zipfile') @patch('keyman_config.handle_install._install_package') - def test_downloadAndInstallPackage_InvalidUrl(self, installPackageMethod): + def test_downloadAndInstallPackage_InvalidUrl(self, installPackageMethod, mockIsZipfile): + # Setup + mockIsZipfile.return_value = True + # Execute download_and_install_package('http://localhost/keyboard/sil_euro_latin.kmp') # Verify installPackageMethod.assert_not_called() + @patch('keyman_config.handle_install.is_zipfile') @patch('keyman_config.handle_install._install_package') @patch('keyman_config.get_kmp.download_kmp_file') def test_downloadAndInstallPackage_invalidUrl(self, downloadKmpFileMethod, - installPackageMethod): + installPackageMethod, mockIsZipfile): for url in ['foo://download/keyboard/sil_euro_latin', 'keyman://keyboard/sil_euro_latin', 'keyman://download/sil_euro_latin', 'keyman://download/keyboard/']: with self.subTest(url=url): + # Setup + mockIsZipfile.return_value = True + # Execute download_and_install_package(url) @@ -62,15 +88,18 @@ class HandleInstallTests(unittest.TestCase): downloadKmpFileMethod.assert_not_called() installPackageMethod.assert_not_called() + @patch('keyman_config.handle_install.is_zipfile') @patch('keyman_config.handle_install._install_package') @patch('keyman_config.get_kmp.keyman_cache_dir') @patch('keyman_config.handle_install.download_kmp_file') def test_downloadAndInstallPackage_keymanUrl(self, downloadKmpFileMethod, - keymanCacheDirMethod, installPackageMethod): + keymanCacheDirMethod, installPackageMethod, + mockIsZipfile): # Setup mockPackagePath = '/tmp/sil_euro_latin' keymanCacheDirMethod.return_value = '/tmp' downloadKmpFileMethod.return_value = mockPackagePath + mockIsZipfile.return_value = True # Execute download_and_install_package('keyman://download/keyboard/sil_euro_latin') @@ -81,16 +110,18 @@ class HandleInstallTests(unittest.TestCase): mockPackagePath) installPackageMethod.assert_called_with(mockPackagePath, '') + @patch('keyman_config.handle_install.is_zipfile') @patch('keyman_config.handle_install._install_package') @patch('keyman_config.get_kmp.keyman_cache_dir') @patch('keyman_config.handle_install.download_kmp_file') def test_downloadAndInstallPackage_keymanUrlWithBcp47(self, downloadKmpFileMethod, keymanCacheDirMethod, - installPackageMethod): + installPackageMethod, mockIsZipfile): # Setup mockPackagePath = '/tmp/sil_euro_latin' keymanCacheDirMethod.return_value = '/tmp' downloadKmpFileMethod.return_value = mockPackagePath + mockIsZipfile.return_value = True # Execute download_and_install_package('keyman://download/keyboard/sil_euro_latin?bcp47=de')