From 685eaa6ffc63a733066945168f7ce6377b61e3aa Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Fri, 28 Jan 2022 18:22:35 +0100 Subject: [PATCH 1/3] fix(linux): Don't crash if we lack permissions Also deal with other file I/O related exceptions and report them as warnings. This fixes #6171. --- .../keyman-config/keyman_config/convertico.py | 52 +++++++++++-------- linux/keyman-config/keyman_config/get_kmp.py | 10 ++-- .../keyman_config/install_kmp.py | 17 ++++-- .../keyman_config/keyboard_details.py | 14 +++-- .../keyman_config/kmpmetadata.py | 7 ++- linux/keyman-config/keyman_config/kvk2ldml.py | 41 ++++++++------- .../keyman_config/list_installed_kmp.py | 20 ++++--- linux/keyman-config/km-package-install | 13 +++-- 8 files changed, 106 insertions(+), 68 deletions(-) diff --git a/linux/keyman-config/keyman_config/convertico.py b/linux/keyman-config/keyman_config/convertico.py index b4ba469e55..c3e24f3d6f 100755 --- a/linux/keyman-config/keyman_config/convertico.py +++ b/linux/keyman-config/keyman_config/convertico.py @@ -69,34 +69,42 @@ def extractico(kmxfile): """ name, ext = os.path.splitext(kmxfile) imagefilename = name - with open(kmxfile, mode='rb') as file: # b is important -> binary - fileContent = file.read() + bitmap = None + try: + with open(kmxfile, mode='rb') as file: # b is important -> binary + fileContent = file.read() - kmxstart = struct.unpack_from("<16I", fileContent, 0) - if kmxstart[0] != 0x5354584B: - logging.debug("bad kmx identifier") - return False - bitmapOffset = kmxstart[14] - bitmapSize = kmxstart[15] - logging.debug("bitmap offset is %d", bitmapOffset) - logging.debug("bitmap size is %d", bitmapSize) - file.seek(bitmapOffset, 0) - bitmap = file.read(bitmapSize) - if not bitmap or file.tell() != bitmapOffset + bitmapSize: - logging.debug("unreadable bitmap in kmx") - return False + kmxstart = struct.unpack_from("<16I", fileContent, 0) + if kmxstart[0] != 0x5354584B: + logging.debug("bad kmx identifier") + return False + bitmapOffset = kmxstart[14] + bitmapSize = kmxstart[15] + logging.debug("bitmap offset is %d", bitmapOffset) + logging.debug("bitmap size is %d", bitmapSize) + file.seek(bitmapOffset, 0) + bitmap = file.read(bitmapSize) + if not bitmap or file.tell() != bitmapOffset + bitmapSize: + logging.debug("unreadable bitmap in kmx") + return False + except Exception as e: + logging.warning('Exception %s extracting icon %s %s', type(e), kmxfile, e.args) + return False - # Read first two bytes to determine if icon is .bmp or .ico - if bitmap.startswith(b'BM'): - imagefilename = imagefilename + ".bmp" - else: - imagefilename = imagefilename + ".ico" + # Read first two bytes to determine if icon is .bmp or .ico + if bitmap.startswith(b'BM'): + imagefilename = imagefilename + ".bmp" + else: + imagefilename = imagefilename + ".ico" + try: with open(imagefilename, mode='wb') as imagefile: imagefile.write(bitmap) checkandsaveico(imagefilename) - - return True + except Exception as e: + logging.warning('Exception %s writing imagefile %s %s', type(e), imagefilename, e.args) + return False + return True def main(argv): diff --git a/linux/keyman-config/keyman_config/get_kmp.py b/linux/keyman-config/keyman_config/get_kmp.py index 8462a8925f..1377d5ba23 100755 --- a/linux/keyman-config/keyman_config/get_kmp.py +++ b/linux/keyman-config/keyman_config/get_kmp.py @@ -206,9 +206,13 @@ def download_kmp_file(url, kmpfile, cache=False): requests_cache.uninstall_cache() if response.status_code == 200: - with open(kmpfile, 'wb') as f: - f.write(response.content) - downloadfile = kmpfile + try: + with open(kmpfile, 'wb') as f: + f.write(response.content) + downloadfile = kmpfile + except Exception as e: + logging.warning('Exception %s writing downloaded file %s %s', type(e), kmpfile, e.args) + return None return downloadfile diff --git a/linux/keyman-config/keyman_config/install_kmp.py b/linux/keyman-config/keyman_config/install_kmp.py index 75891f99bd..df0329a7d2 100755 --- a/linux/keyman-config/keyman_config/install_kmp.py +++ b/linux/keyman-config/keyman_config/install_kmp.py @@ -249,12 +249,19 @@ def extract_kmp(kmpfile, directory): def process_keyboard_data(keyboardID, packageDir): kbdata = get_keyboard_data(keyboardID) if kbdata: - if not os.path.isdir(packageDir): - os.makedirs(packageDir) + if not os.path.isdir(packageDir) and os.access(os.path.join(packageDir, os.pardir), os.X_OK | os.W_OK): + try: + os.makedirs(packageDir) + except Exception as e: + logging.warning('Exception %s creating %s %s', type(e), packageDir, e.args) - with open(os.path.join(packageDir, keyboardID + '.json'), 'w') as outfile: - json.dump(kbdata, outfile) - logging.info("Installing api data file %s.json as keyman file", keyboardID) + if os.access(packageDir, os.X_OK | os.W_OK): + try: + with open(os.path.join(packageDir, keyboardID + '.json'), 'w') as outfile: + json.dump(kbdata, outfile) + logging.info("Installing api data file %s.json as keyman file", keyboardID) + except Exception as e: + logging.warning('Exception %s writing %s/%s.json %s', type(e), packageDir, keyboardID, e.args) # else: # message = "install_kmp.py: error: cannot download keyboard data so not installing." # rmtree(kbdir) diff --git a/linux/keyman-config/keyman_config/keyboard_details.py b/linux/keyman-config/keyman_config/keyboard_details.py index 461f065fce..d5acdb9723 100644 --- a/linux/keyman-config/keyman_config/keyboard_details.py +++ b/linux/keyman-config/keyman_config/keyboard_details.py @@ -61,8 +61,11 @@ class KeyboardDetailsView(Gtk.Dialog): kbdata = None jsonfile = path.join(packageDir, kmp['packageID'] + ".json") if path.isfile(jsonfile): - with open(jsonfile, "r") as read_file: - kbdata = json.load(read_file) + try: + with open(jsonfile, "r") as read_file: + kbdata = json.load(read_file) + except Exception as e: + logging.warning('Exception %s reading %s %s', type(e), jsonfile, e.args) grid = Gtk.Grid() # grid.set_column_homogeneous(True) @@ -165,8 +168,11 @@ class KeyboardDetailsView(Gtk.Dialog): kbdata = None jsonfile = path.join(packageDir, kbd['id'] + ".json") if path.isfile(jsonfile): - with open(jsonfile, "r") as read_file: - kbdata = json.load(read_file) + try: + with open(jsonfile, "r") as read_file: + kbdata = json.load(read_file) + except Exception as e: + logging.warning('Exception %s reading %s %s', type(e), jsonfile, e.args) # start with padding lbl_pad = Gtk.Label() diff --git a/linux/keyman-config/keyman_config/kmpmetadata.py b/linux/keyman-config/keyman_config/kmpmetadata.py index 34dab455c8..bf51f8f337 100755 --- a/linux/keyman-config/keyman_config/kmpmetadata.py +++ b/linux/keyman-config/keyman_config/kmpmetadata.py @@ -535,8 +535,11 @@ def get_and_convert_infdata(tmpdirname): info, system, options, keyboards, files = parseinfdata(kmpinf, False) j = infmetadata_to_json(info, system, options, keyboards, files) kmpjson = os.path.join(tmpdirname, "kmp.json") - with open(kmpjson, "w") as write_file: - print(j, file=write_file) + try: + with open(kmpjson, "w") as write_file: + print(j, file=write_file) + except Exception as e: + logging.warning('Exception %s writing metadata %s %s', type(e), kmpjson, e.args) return info, system, options, keyboards, files else: return None, None, None, None, None diff --git a/linux/keyman-config/keyman_config/kvk2ldml.py b/linux/keyman-config/keyman_config/kvk2ldml.py index c6ae7551c5..da692b8e35 100755 --- a/linux/keyman-config/keyman_config/kvk2ldml.py +++ b/linux/keyman-config/keyman_config/kvk2ldml.py @@ -373,28 +373,31 @@ def output_ldml(ldmlfile, ldml): def parse_kvk_file(kvkfile): kvkData = KVKData() - with open(kvkfile, mode='rb') as file: # b is important -> binary - fileContent = file.read() + try: + with open(kvkfile, mode='rb') as file: # b is important -> binary + fileContent = file.read() - kvkstart = struct.unpack_from("<4s4cc", fileContent, 0) - kvkData.version = (kvkstart[1], kvkstart[2], kvkstart[3], kvkstart[4]) - kvkData.flags = kvkstart[5] - kvkData.key102 = bytecheck(kvkData.flags[0], kvkk102key) - kvkData.DisplayUnderlying = bytecheck(kvkData.flags[0], kvkkDisplayUnderlying) - kvkData.UseUnderlying = bytecheck(kvkData.flags[0], kvkkUseUnderlying) - kvkData.AltGr = bytecheck(kvkData.flags[0], kvkkAltGr) + kvkstart = struct.unpack_from("<4s4cc", fileContent, 0) + kvkData.version = (kvkstart[1], kvkstart[2], kvkstart[3], kvkstart[4]) + kvkData.flags = kvkstart[5] + kvkData.key102 = bytecheck(kvkData.flags[0], kvkk102key) + kvkData.DisplayUnderlying = bytecheck(kvkData.flags[0], kvkkDisplayUnderlying) + kvkData.UseUnderlying = bytecheck(kvkData.flags[0], kvkkUseUnderlying) + kvkData.AltGr = bytecheck(kvkData.flags[0], kvkkAltGr) - kvkData.AssociatedKeyboard, newoffset = get_nstring(file, fileContent, struct.calcsize("<4s4cc")) - kvkData.AnsiFont, newoffset = get_nfont(file, fileContent, newoffset) - kvkData.UnicodeFont, newoffset = get_nfont(file, fileContent, newoffset) - numkeys = struct.unpack_from("I", fileContent, newoffset) - kvkData.KeyCount = numkeys[0] - newoffset = newoffset + struct.calcsize("I") + kvkData.AssociatedKeyboard, newoffset = get_nstring(file, fileContent, struct.calcsize("<4s4cc")) + kvkData.AnsiFont, newoffset = get_nfont(file, fileContent, newoffset) + kvkData.UnicodeFont, newoffset = get_nfont(file, fileContent, newoffset) + numkeys = struct.unpack_from("I", fileContent, newoffset) + kvkData.KeyCount = numkeys[0] + newoffset = newoffset + struct.calcsize("I") - for num in range(numkeys[0]): - nkey, newoffset = get_nkey(file, fileContent, newoffset) - nkey.number = num - kvkData.Keys.append(nkey) + for num in range(numkeys[0]): + nkey, newoffset = get_nkey(file, fileContent, newoffset) + nkey.number = num + kvkData.Keys.append(nkey) + except Exception as e: + logging.warning('Exception %s parsing kvk file %s %s', type(e), kvkfile, e.args) return kvkData diff --git a/linux/keyman-config/keyman_config/list_installed_kmp.py b/linux/keyman-config/keyman_config/list_installed_kmp.py index 1745a10dba..71edb45789 100755 --- a/linux/keyman-config/keyman_config/list_installed_kmp.py +++ b/linux/keyman-config/keyman_config/list_installed_kmp.py @@ -1,5 +1,6 @@ #!/usr/bin/python3 +import logging import os import json from keyman_config.kmpmetadata import parsemetadata, parseinfdata @@ -70,14 +71,17 @@ def get_installed_kmp_paths(check_paths): has_kbjson = False kbjson = os.path.join(keymanpath, o, o + ".json") if os.path.isfile(kbjson): - with open(kbjson, "r") as read_file: - kbdata = json.load(read_file) - if kbdata: - if 'description' in kbdata: - description = kbdata['description'] - version = kbdata['version'] - name = kbdata['name'] - has_kbjson = True + try: + with open(kbjson, "r") as read_file: + kbdata = json.load(read_file) + if kbdata: + if 'description' in kbdata: + description = kbdata['description'] + version = kbdata['version'] + name = kbdata['name'] + has_kbjson = True + except Exception as e: + logging.warning('Exception %s loading %s %s', type(e), kbjson, e.args) if info: md_version = info['version']['description'] md_name = info['name']['description'] diff --git a/linux/keyman-config/km-package-install b/linux/keyman-config/km-package-install index a4cffddb72..e23d85b118 100755 --- a/linux/keyman-config/km-package-install +++ b/linux/keyman-config/km-package-install @@ -57,11 +57,14 @@ def get_keyboard_list(): def write_kmpdirlist(kmpdirfile): - with open(kmpdirfile, 'wt') as kmpdirlist: - keyboards = get_keyboard_list() - if keyboards: - for kb in get_keyboard_list(): - print(kb, file=kmpdirlist) + try: + with open(kmpdirfile, 'wt') as kmpdirlist: + keyboards = get_keyboard_list() + if keyboards: + for kb in get_keyboard_list(): + print(kb, file=kmpdirlist) + except Exception as e: + logging.warning('Exception %s writing %s %s', type(e), kmpdirfile, e.args) def list_keyboards(): From 0b8e62881294b15e4ad905d9162b842c7179910a Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Fri, 28 Jan 2022 18:37:20 +0100 Subject: [PATCH 2/3] chore(linux): Improve robustness Don't crash if we encounter a corrupt file. --- linux/keyman-config/keyman_config/__init__.py | 14 + linux/keyman-config/keyman_config/get_info.py | 13 +- .../keyman_config/gnome_keyboards_util.py | 2 + .../keyman_config/install_kmp.py | 6 +- .../keyman_config/install_window.py | 40 +-- .../keyman_config/keyboard_details.py | 263 +++++++++--------- .../keyman_config/kmpmetadata.py | 22 +- .../keyman_config/list_installed_kmp.py | 5 +- linux/keyman-config/km-package-install | 11 +- 9 files changed, 201 insertions(+), 175 deletions(-) diff --git a/linux/keyman-config/keyman_config/__init__.py b/linux/keyman-config/keyman_config/__init__.py index fb2cd256b1..7c3972c247 100644 --- a/linux/keyman-config/keyman_config/__init__.py +++ b/linux/keyman-config/keyman_config/__init__.py @@ -23,6 +23,20 @@ def _(txt): return translation +def secure_lookup(data, key1, key2 = None): + """ + Return data[key1][key2] while dealing with data being None or key1 or key2 not existing + """ + if not data: + return None + if key1 in data: + if not key2: + return data[key1] + if key2 in data[key1]: + return data[key1][key2] + return None + + gettext.bindtextdomain('keyman-config', '/usr/share/locale') gettext.textdomain('keyman-config') diff --git a/linux/keyman-config/keyman_config/get_info.py b/linux/keyman-config/keyman_config/get_info.py index a8aab36423..d5a224d24f 100644 --- a/linux/keyman-config/keyman_config/get_info.py +++ b/linux/keyman-config/keyman_config/get_info.py @@ -1,7 +1,9 @@ #!/usr/bin/python3 +import logging import os import threading +from keyman_config import secure_lookup from keyman_config.install_kmp import process_keyboard_data from keyman_config.kmpmetadata import parsemetadata @@ -27,10 +29,15 @@ class GetInfo(object): rather then from the download window. """ for kmp in self.kmp_list: - packageDir = os.path.join(kmp['areapath'], kmp['packageID']) - process_keyboard_data(kmp['packageID'], packageDir) + packageId = secure_lookup(kmp, 'packageID') + areapath = secure_lookup(kmp, 'areapath') + if not areapath or not packageId: + logging.warning('corrupt kmp list') + return + packageDir = os.path.join(areapath, packageId) + process_keyboard_data(packageId, packageDir) info, system, options, keyboards, files = parsemetadata(packageDir, "kmp.json") if keyboards: for kb in keyboards: - if kb['id'] != kmp['packageID']: + if kb['id'] != packageId: process_keyboard_data(kb['id'], packageDir) diff --git a/linux/keyman-config/keyman_config/gnome_keyboards_util.py b/linux/keyman-config/keyman_config/gnome_keyboards_util.py index b17d754fe7..617eb6f296 100644 --- a/linux/keyman-config/keyman_config/gnome_keyboards_util.py +++ b/linux/keyman-config/keyman_config/gnome_keyboards_util.py @@ -39,6 +39,8 @@ def _reset_gnome_shell(): def get_ibus_keyboard_id(keyboard, packageDir, language=None, ignore_language=False): + if not keyboard: + return None kmx_file = os.path.join(packageDir, keyboard['id'] + ".kmx") if ignore_language: return kmx_file diff --git a/linux/keyman-config/keyman_config/install_kmp.py b/linux/keyman-config/keyman_config/install_kmp.py index df0329a7d2..4f024ca980 100755 --- a/linux/keyman-config/keyman_config/install_kmp.py +++ b/linux/keyman-config/keyman_config/install_kmp.py @@ -7,7 +7,7 @@ import zipfile from shutil import rmtree from enum import Enum -from keyman_config import _ +from keyman_config import _, secure_lookup from keyman_config.canonical_language_code_utils import CanonicalLanguageCodeUtils from keyman_config.fcitx_util import is_fcitx_running, restart_fcitx from keyman_config.get_kmp import get_keyboard_data, get_keyboard_dir, get_keyman_doc_dir @@ -119,7 +119,7 @@ class InstallKmp(): info, _, _, keyboards, files = get_metadata(self.packageDir) if keyboards: - logging.info("Installing %s", info['name']['description']) + logging.info("Installing %s", secure_lookup(info, 'name', 'description')) if online: process_keyboard_data(self.packageID, self.packageDir) for kb in keyboards: @@ -199,7 +199,7 @@ class InstallKmp(): def install_keyboards(self, keyboards, packageDir, language=None): firstKeyboard = keyboards[0] - if firstKeyboard and 'languages' in firstKeyboard and len(firstKeyboard['languages']) > 0: + if secure_lookup(firstKeyboard, 'languages') and len(firstKeyboard['languages']) > 0: language = self._normalize_language(firstKeyboard['languages'], language) if is_fcitx_running(): diff --git a/linux/keyman-config/keyman_config/install_window.py b/linux/keyman-config/keyman_config/install_window.py index 600f54a0e6..149cafa3b0 100755 --- a/linux/keyman-config/keyman_config/install_window.py +++ b/linux/keyman-config/keyman_config/install_window.py @@ -16,7 +16,7 @@ gi.require_version('WebKit2', '4.0') from gi.repository import Gtk, WebKit2 from distutils.version import StrictVersion -from keyman_config import _ +from keyman_config import _, secure_lookup from keyman_config.fcitx_util import is_fcitx_running from keyman_config.install_kmp import install_kmp, extract_kmp, get_metadata, InstallError, InstallStatus from keyman_config.list_installed_kmp import get_kmp_version @@ -80,8 +80,8 @@ class InstallKmpWindow(Gtk.Dialog): self.kbname = keyboardid self.checkcontinue = True - if installed_kmp_ver and info and 'version' in info and 'description' in info['version']: - if info['version']['description'] == installed_kmp_ver: + if installed_kmp_ver and secure_lookup('version', 'description'): + if secure_lookup(info, 'version', 'description') == installed_kmp_ver: dialog = Gtk.MessageDialog( viewkmp, 0, Gtk.MessageType.QUESTION, Gtk.ButtonsType.YES_NO, _("Keyboard is installed already")) @@ -100,9 +100,9 @@ class InstallKmpWindow(Gtk.Dialog): return else: try: - logging.info("package version %s", info['version']['description']) + logging.info("package version %s", secure_lookup(info, 'version', 'description')) logging.info("installed kmp version %s", installed_kmp_ver) - if StrictVersion(info['version']['description']) <= StrictVersion(installed_kmp_ver): + if StrictVersion(secure_lookup(info, 'version', 'description')) <= StrictVersion(installed_kmp_ver): dialog = Gtk.MessageDialog( viewkmp, 0, Gtk.MessageType.QUESTION, Gtk.ButtonsType.YES_NO, _("Keyboard is installed already")) @@ -110,7 +110,7 @@ class InstallKmpWindow(Gtk.Dialog): _("The {name} keyboard is already installed with a newer version {installedversion}. " "Do you want to uninstall it and install the older version {version}?") .format(name=self.kbname, installedversion=installed_kmp_ver, - version=info['version']['description'])) + version=secure_lookup(info, 'version', 'description'))) response = dialog.run() dialog.destroy() if response == Gtk.ResponseType.YES: @@ -125,7 +125,7 @@ class InstallKmpWindow(Gtk.Dialog): pass image = Gtk.Image() - if options and "graphicFile" in options: + if secure_lookup(options, 'graphicFile'): image.set_from_file(os.path.join(tmpdirname, options['graphicFile'])) else: img_default = find_keyman_image("defaultpackage.gif") @@ -186,13 +186,13 @@ class InstallKmpWindow(Gtk.Dialog): grid.attach_next_to(label3, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) prevlabel = label3 label = Gtk.Label() - if info and 'version' in info and 'description' in info['version']: - label.set_text(info['version']['description']) + if secure_lookup(info, 'version', 'description'): + label.set_text(secure_lookup(info, 'version', 'description')) label.set_halign(Gtk.Align.START) label.set_selectable(True) grid.attach_next_to(label, label3, Gtk.PositionType.RIGHT, 1, 1) - if info and 'author' in info: + if secure_lookup(info, 'author'): author = info['author'] label4 = Gtk.Label() label4.set_text(_("Author: ")) @@ -200,17 +200,17 @@ class InstallKmpWindow(Gtk.Dialog): grid.attach_next_to(label4, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) prevlabel = label4 label = Gtk.Label() - if 'url' in author and 'description' in author: + if secure_lookup(author, 'url') and secure_lookup(author, 'description'): label.set_markup( "" + author['description'] + "") - elif 'description' in author: + elif secure_lookup(author, 'description'): label.set_text(author['description']) label.set_halign(Gtk.Align.START) label.set_selectable(True) grid.attach_next_to(label, label4, Gtk.PositionType.RIGHT, 1, 1) - if info and 'website' in info: + if secure_lookup(info, 'website'): label5 = Gtk.Label() # Website is optional and may be a mailto for the author label5.set_text(_("Website: ")) @@ -218,22 +218,22 @@ class InstallKmpWindow(Gtk.Dialog): grid.attach_next_to(label5, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) prevlabel = label5 label = Gtk.Label() - if 'description' in info['website']: + if secure_lookup(info, 'website', 'description'): label.set_markup( - "" + - info['website']['description'] + "") + "" + + secure_lookup(info, 'website', 'description') + "") label.set_halign(Gtk.Align.START) label.set_selectable(True) grid.attach_next_to(label, label5, Gtk.PositionType.RIGHT, 1, 1) - if info and 'copyright' in info: + if secure_lookup(info, 'copyright'): label6 = Gtk.Label() label6.set_text(_("Copyright: ")) label6.set_halign(Gtk.Align.END) grid.attach_next_to(label6, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) label = Gtk.Label() - if 'description' in info['copyright']: - label.set_text(info['copyright']['description']) + if secure_lookup(info, 'copyright', 'description'): + label.set_text(secure_lookup(info, 'copyright', 'description')) label.set_halign(Gtk.Align.START) label.set_selectable(True) grid.attach_next_to(label, label6, Gtk.PositionType.RIGHT, 1, 1) @@ -242,7 +242,7 @@ class InstallKmpWindow(Gtk.Dialog): webview = WebKit2.WebView() webview.connect("decide-policy", self.doc_policy) - if options and "readmeFile" in options: + if secure_lookup(options, 'readmeFile'): self.readme = options['readmeFile'] else: self.readme = "noreadme" diff --git a/linux/keyman-config/keyman_config/keyboard_details.py b/linux/keyman-config/keyman_config/keyboard_details.py index d5acdb9723..fc99f86b82 100644 --- a/linux/keyman-config/keyman_config/keyboard_details.py +++ b/linux/keyman-config/keyman_config/keyboard_details.py @@ -2,6 +2,7 @@ # Keyboard details window +import logging import json from os import path import qrcode @@ -10,7 +11,7 @@ import tempfile import gi from gi.repository import Gtk -from keyman_config import KeymanComUrl, _ +from keyman_config import KeymanComUrl, _, secure_lookup from keyman_config.accelerators import init_accel from keyman_config.kmpmetadata import parsemetadata @@ -80,8 +81,8 @@ class KeyboardDetailsView(Gtk.Dialog): grid.add(lbl_pkg_name) prevlabel = lbl_pkg_name label = Gtk.Label() - if info['name']['description']: - label.set_text(info['name']['description']) + if secure_lookup(info, 'name', 'description'): + label.set_text(secure_lookup(info, 'name', 'description')) label.set_halign(Gtk.Align.START) label.set_selectable(True) grid.attach_next_to(label, lbl_pkg_name, Gtk.PositionType.RIGHT, 1, 1) @@ -92,7 +93,7 @@ class KeyboardDetailsView(Gtk.Dialog): grid.attach_next_to(lbl_pkg_id, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) prevlabel = lbl_pkg_id label = Gtk.Label() - if kmp['packageID']: + if secure_lookup(kmp, 'packageID'): label.set_text(kmp['packageID']) label.set_halign(Gtk.Align.START) label.set_selectable(True) @@ -104,48 +105,47 @@ class KeyboardDetailsView(Gtk.Dialog): grid.attach_next_to(lbl_pkg_vrs, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) prevlabel = lbl_pkg_vrs label = Gtk.Label() - if info['version']['description']: - label.set_text(info['version']['description']) + if secure_lookup(info, 'version', 'description'): + label.set_text(secure_lookup(info, 'version', 'description')) label.set_halign(Gtk.Align.START) label.set_selectable(True) grid.attach_next_to(label, lbl_pkg_vrs, Gtk.PositionType.RIGHT, 1, 1) - if kbdata and kbdata.get('description'): + if secure_lookup(kbdata, 'description'): lbl_pkg_desc = Gtk.Label() lbl_pkg_desc.set_text(_("Package description: ")) lbl_pkg_desc.set_halign(Gtk.Align.END) grid.attach_next_to(lbl_pkg_desc, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) prevlabel = lbl_pkg_desc label = Gtk.Label() - if kbdata.get('description'): - label.set_text(kbdata.get('description')) + label.set_text(kbdata['description']) label.set_halign(Gtk.Align.START) label.set_selectable(True) label.set_line_wrap(80) grid.attach_next_to(label, lbl_pkg_desc, Gtk.PositionType.RIGHT, 1, 1) - if "author" in info: + if secure_lookup(info, "author"): lbl_pkg_auth = Gtk.Label() lbl_pkg_auth.set_text(_("Package author: ")) lbl_pkg_auth.set_halign(Gtk.Align.END) grid.attach_next_to(lbl_pkg_auth, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) prevlabel = lbl_pkg_auth label = Gtk.Label() - if info['author']['description']: - label.set_text(info['author']['description']) + if secure_lookup(info, 'author', 'description'): + label.set_text(secure_lookup(info, 'author', 'description')) label.set_halign(Gtk.Align.START) label.set_selectable(True) grid.attach_next_to(label, lbl_pkg_auth, Gtk.PositionType.RIGHT, 1, 1) - if "copyright" in info: + if secure_lookup(info, "copyright"): lbl_pkg_cpy = Gtk.Label() lbl_pkg_cpy.set_text(_("Package copyright: ")) lbl_pkg_cpy.set_halign(Gtk.Align.END) grid.attach_next_to(lbl_pkg_cpy, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) prevlabel = lbl_pkg_cpy label = Gtk.Label() - if info['copyright']['description']: - label.set_text(info['copyright']['description']) + if secure_lookup(info, 'copyright', 'description'): + label.set_text(secure_lookup(info, 'copyright', 'description')) label.set_halign(Gtk.Align.START) label.set_selectable(True) grid.attach_next_to(label, lbl_pkg_cpy, Gtk.PositionType.RIGHT, 1, 1) @@ -194,133 +194,132 @@ class KeyboardDetailsView(Gtk.Dialog): label.set_selectable(True) grid.attach_next_to(label, lbl_kbd_file, Gtk.PositionType.RIGHT, 1, 1) - if kbdata: - if kbdata['id'] != kmp['packageID']: - lbl_kbd_name = Gtk.Label() - lbl_kbd_name.set_text(_("Keyboard name: ")) - lbl_kbd_name.set_halign(Gtk.Align.END) - grid.attach_next_to(lbl_kbd_name, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) - prevlabel = lbl_kbd_name + if kbdata and secure_lookup(kbdata, 'id') != secure_lookup(kmp, 'packageID'): + lbl_kbd_name = Gtk.Label() + lbl_kbd_name.set_text(_("Keyboard name: ")) + lbl_kbd_name.set_halign(Gtk.Align.END) + grid.attach_next_to(lbl_kbd_name, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) + prevlabel = lbl_kbd_name + label = Gtk.Label() + if secure_lookup(kbdata, 'name'): + label.set_text(kbdata['name']) + label.set_halign(Gtk.Align.START) + label.set_selectable(True) + grid.attach_next_to(label, lbl_kbd_name, Gtk.PositionType.RIGHT, 1, 1) + + lbl_kbd_id = Gtk.Label() + lbl_kbd_id.set_text(_("Keyboard id: ")) + lbl_kbd_id.set_halign(Gtk.Align.END) + grid.attach_next_to(lbl_kbd_id, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) + prevlabel = lbl_kbd_id + label = Gtk.Label() + if secure_lookup(kbdata, 'id'): + label.set_text(kbdata['id']) + label.set_halign(Gtk.Align.START) + label.set_selectable(True) + grid.attach_next_to(label, lbl_kbd_id, Gtk.PositionType.RIGHT, 1, 1) + + lbl_kbd_vrs = Gtk.Label() + lbl_kbd_vrs.set_text(_("Keyboard version: ")) + lbl_kbd_vrs.set_halign(Gtk.Align.END) + grid.attach_next_to(lbl_kbd_vrs, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) + prevlabel = lbl_kbd_vrs + label = Gtk.Label() + if secure_lookup(kbdata, 'version'): + label.set_text(kbdata['version']) + label.set_halign(Gtk.Align.START) + label.set_selectable(True) + grid.attach_next_to(label, lbl_kbd_vrs, Gtk.PositionType.RIGHT, 1, 1) + + if secure_lookup(info, "author"): + lbl_kbd_auth = Gtk.Label() + lbl_kbd_auth.set_text(_("Keyboard author: ")) + lbl_kbd_auth.set_halign(Gtk.Align.END) + grid.attach_next_to(lbl_kbd_auth, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) + prevlabel = lbl_kbd_auth label = Gtk.Label() - if kbdata['name']: - label.set_text(kbdata['name']) + if secure_lookup(kbdata, 'authorName'): + label.set_text(kbdata['authorName']) label.set_halign(Gtk.Align.START) label.set_selectable(True) - grid.attach_next_to(label, lbl_kbd_name, Gtk.PositionType.RIGHT, 1, 1) + grid.attach_next_to(label, lbl_kbd_auth, Gtk.PositionType.RIGHT, 1, 1) - lbl_kbd_id = Gtk.Label() - lbl_kbd_id.set_text(_("Keyboard id: ")) - lbl_kbd_id.set_halign(Gtk.Align.END) - grid.attach_next_to(lbl_kbd_id, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) - prevlabel = lbl_kbd_id - label = Gtk.Label() - if kbdata['id']: - label.set_text(kbdata['id']) - label.set_halign(Gtk.Align.START) - label.set_selectable(True) - grid.attach_next_to(label, lbl_kbd_id, Gtk.PositionType.RIGHT, 1, 1) + lbl_kbd_lic = Gtk.Label() + lbl_kbd_lic.set_text(_("Keyboard license: ")) + lbl_kbd_lic.set_halign(Gtk.Align.END) + grid.attach_next_to(lbl_kbd_lic, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) + prevlabel = lbl_kbd_lic + label = Gtk.Label() + if secure_lookup(kbdata, 'license'): + label.set_text(kbdata['license']) + label.set_halign(Gtk.Align.START) + label.set_selectable(True) + grid.attach_next_to(label, lbl_kbd_lic, Gtk.PositionType.RIGHT, 1, 1) - lbl_kbd_vrs = Gtk.Label() - lbl_kbd_vrs.set_text(_("Keyboard version: ")) - lbl_kbd_vrs.set_halign(Gtk.Align.END) - grid.attach_next_to(lbl_kbd_vrs, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) - prevlabel = lbl_kbd_vrs - label = Gtk.Label() - if kbdata['version']: - label.set_text(kbdata['version']) - label.set_halign(Gtk.Align.START) - label.set_selectable(True) - grid.attach_next_to(label, lbl_kbd_vrs, Gtk.PositionType.RIGHT, 1, 1) + lbl_kbd_desc = Gtk.Label() + lbl_kbd_desc.set_text(_("Keyboard description: ")) + lbl_kbd_desc.set_halign(Gtk.Align.END) + grid.attach_next_to(lbl_kbd_desc, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) + prevlabel = lbl_kbd_desc + label = Gtk.Label() + if secure_lookup(kbdata, 'description'): + label.set_text(kbdata['description']) + label.set_halign(Gtk.Align.START) + label.set_selectable(True) + label.set_line_wrap(80) + grid.attach_next_to(label, lbl_kbd_desc, Gtk.PositionType.RIGHT, 1, 1) - if "author" in info: - lbl_kbd_auth = Gtk.Label() - lbl_kbd_auth.set_text(_("Keyboard author: ")) - lbl_kbd_auth.set_halign(Gtk.Align.END) - grid.attach_next_to(lbl_kbd_auth, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) - prevlabel = lbl_kbd_auth - label = Gtk.Label() - if kbdata['authorName']: - label.set_text(kbdata['authorName']) - label.set_halign(Gtk.Align.START) - label.set_selectable(True) - grid.attach_next_to(label, lbl_kbd_auth, Gtk.PositionType.RIGHT, 1, 1) + # Padding and full width horizontal divider + lbl_pad = Gtk.Label() + lbl_pad.set_text("") + lbl_pad.set_halign(Gtk.Align.END) + grid.attach_next_to(lbl_pad, prevlabel, Gtk.PositionType.BOTTOM, 2, 1) + prevlabel = lbl_pad - lbl_kbd_lic = Gtk.Label() - lbl_kbd_lic.set_text(_("Keyboard license: ")) - lbl_kbd_lic.set_halign(Gtk.Align.END) - grid.attach_next_to(lbl_kbd_lic, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) - prevlabel = lbl_kbd_lic - label = Gtk.Label() - if kbdata['license']: - label.set_text(kbdata['license']) - label.set_halign(Gtk.Align.START) - label.set_selectable(True) - grid.attach_next_to(label, lbl_kbd_lic, Gtk.PositionType.RIGHT, 1, 1) + divider_pkg = Gtk.HSeparator() + grid.attach_next_to(divider_pkg, prevlabel, Gtk.PositionType.BOTTOM, 2, 1) - lbl_kbd_desc = Gtk.Label() - lbl_kbd_desc.set_text(_("Keyboard description: ")) - lbl_kbd_desc.set_halign(Gtk.Align.END) - grid.attach_next_to(lbl_kbd_desc, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) - prevlabel = lbl_kbd_desc - label = Gtk.Label() - if kbdata['description']: - label.set_text(kbdata['description']) - label.set_halign(Gtk.Align.START) - label.set_selectable(True) - label.set_line_wrap(80) - grid.attach_next_to(label, lbl_kbd_desc, Gtk.PositionType.RIGHT, 1, 1) + # label7 = Gtk.Label() + # label7.set_text(_("On Screen Keyboard: ")) + # label7.set_halign(Gtk.Align.END) + # grid.attach_next_to(label7, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) + # prevlabel = label7 + # # label = Gtk.Label() + # # label.set_text(secure_lookup(info, 'version', 'description')) + # # label.set_halign(Gtk.Align.START) + # # label.set_selectable(True) + # # grid.attach_next_to(label, label7, Gtk.PositionType.RIGHT, 1, 1) - # Padding and full width horizontal divider - lbl_pad = Gtk.Label() - lbl_pad.set_text("") - lbl_pad.set_halign(Gtk.Align.END) - grid.attach_next_to(lbl_pad, prevlabel, Gtk.PositionType.BOTTOM, 2, 1) - prevlabel = lbl_pad + # label8 = Gtk.Label() + # label8.set_text(_("Documentation: ")) + # label8.set_halign(Gtk.Align.END) + # grid.attach_next_to(label8, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) + # prevlabel = label8 + # #TODO need to know which area keyboard is installed in to show this + # # label = Gtk.Label() + # # welcome_file = path.join("/usr/local/share/doc/keyman", kmp["id"], "welcome.htm") + # # if path.isfile(welcome_file): + # # label.set_text(_("Installed")) + # # else: + # # label.set_text(_("Not installed")) + # # label.set_halign(Gtk.Align.START) + # # label.set_selectable(True) + # # grid.attach_next_to(label, label8, Gtk.PositionType.RIGHT, 1, 1) - divider_pkg = Gtk.HSeparator() - grid.attach_next_to(divider_pkg, prevlabel, Gtk.PositionType.BOTTOM, 2, 1) - - # label7 = Gtk.Label() - # label7.set_text(_("On Screen Keyboard: ")) - # label7.set_halign(Gtk.Align.END) - # grid.attach_next_to(label7, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) - # prevlabel = label7 - # # label = Gtk.Label() - # # label.set_text(info['version']['description']) - # # label.set_halign(Gtk.Align.START) - # # label.set_selectable(True) - # # grid.attach_next_to(label, label7, Gtk.PositionType.RIGHT, 1, 1) - - # label8 = Gtk.Label() - # label8.set_text(_("Documentation: ")) - # label8.set_halign(Gtk.Align.END) - # grid.attach_next_to(label8, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) - # prevlabel = label8 - # #TODO need to know which area keyboard is installed in to show this - # # label = Gtk.Label() - # # welcome_file = path.join("/usr/local/share/doc/keyman", kmp["id"], "welcome.htm") - # # if path.isfile(welcome_file): - # # label.set_text(_("Installed")) - # # else: - # # label.set_text(_("Not installed")) - # # label.set_halign(Gtk.Align.START) - # # label.set_selectable(True) - # # grid.attach_next_to(label, label8, Gtk.PositionType.RIGHT, 1, 1) - - # label9 = Gtk.Label() - # # stored in kmx - # label9.set_text(_("Message: ")) - # label9.set_halign(Gtk.Align.END) - # grid.attach_next_to(label9, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) - # prevlabel = label9 - # label = Gtk.Label() - # label.set_line_wrap(True) - # label.set_text( - # "This keyboard is distributed under the MIT license (MIT) as described somewhere") - # #label.set_text(kmp["description"]) - # label.set_halign(Gtk.Align.START) - # label.set_selectable(True) - # grid.attach_next_to(label, label9, Gtk.PositionType.RIGHT, 1, 1) + # label9 = Gtk.Label() + # # stored in kmx + # label9.set_text(_("Message: ")) + # label9.set_halign(Gtk.Align.END) + # grid.attach_next_to(label9, prevlabel, Gtk.PositionType.BOTTOM, 1, 1) + # prevlabel = label9 + # label = Gtk.Label() + # label.set_line_wrap(True) + # label.set_text( + # "This keyboard is distributed under the MIT license (MIT) as described somewhere") + # #label.set_text(kmp["description"]) + # label.set_halign(Gtk.Align.START) + # label.set_selectable(True) + # grid.attach_next_to(label, label9, Gtk.PositionType.RIGHT, 1, 1) # Add an entire row of padding lbl_pad = Gtk.Label() diff --git a/linux/keyman-config/keyman_config/kmpmetadata.py b/linux/keyman-config/keyman_config/kmpmetadata.py index bf51f8f337..c7ba5cca9d 100755 --- a/linux/keyman-config/keyman_config/kmpmetadata.py +++ b/linux/keyman-config/keyman_config/kmpmetadata.py @@ -9,6 +9,8 @@ import magic from enum import Enum from json.decoder import JSONDecodeError +from keyman_config import secure_lookup + class KMFileTypes(Enum): KM_ICON = 1 @@ -32,18 +34,18 @@ def print_info(info): print("---- Info ----") if not info: return - print("Name: ", info['name']['description']) - print("Copyright: ", info['copyright']['description']) + print("Name: ", secure_lookup(info, 'name', 'description')) + print("Copyright: ", secure_lookup(info, 'copyright', 'description')) if 'version' in info: - print("Version: ", info['version']['description']) + print("Version: ", secure_lookup(info, 'version', 'description')) if 'author' in info: - print("Author: ", info['author']['description']) - if 'url' in info['author']: - print("Author URL: ", info['author']['url']) + print("Author: ", secure_lookup(info, 'author', 'description')) + if secure_lookup(info, 'author', 'url'): + print("Author URL: ", secure_lookup(info, 'author', 'url')) if 'website' in info: - print("Website description: ", info['website']['description']) - if 'url' in info['website']: - print("Website URL: ", info['website']['url']) + print("Website description: ", secure_lookup(info, 'website', 'description')) + if secure_lookup(info, 'website', 'url'): + print("Website URL: ", secure_lookup(info, 'website', 'url')) except Exception as e: print(type(e)) # the exception instance print(e.args) # arguments stored in .args @@ -390,7 +392,7 @@ def parseinfdata(inffile, verbose=False): keyboards = [{ 'name': id, 'id': id, - 'version': info['version']['description'] + 'version': secure_lookup(info, 'version', 'description') }] kblist = [] diff --git a/linux/keyman-config/keyman_config/list_installed_kmp.py b/linux/keyman-config/keyman_config/list_installed_kmp.py index 71edb45789..ef2adeeaba 100755 --- a/linux/keyman-config/keyman_config/list_installed_kmp.py +++ b/linux/keyman-config/keyman_config/list_installed_kmp.py @@ -3,6 +3,7 @@ import logging import os import json +from keyman_config import secure_lookup from keyman_config.kmpmetadata import parsemetadata, parseinfdata from keyman_config.get_kmp import get_keyman_dir, InstallLocation from keyman_config.deprecated_decorator import deprecated @@ -83,8 +84,8 @@ def get_installed_kmp_paths(check_paths): except Exception as e: logging.warning('Exception %s loading %s %s', type(e), kbjson, e.args) if info: - md_version = info['version']['description'] - md_name = info['name']['description'] + md_version = secure_lookup(info, 'version', 'description') + md_name = secure_lookup(info, 'name', 'description') if keyboards: keyboardID = keyboards[0]['id'] else: diff --git a/linux/keyman-config/km-package-install b/linux/keyman-config/km-package-install index e23d85b118..f638484cd0 100755 --- a/linux/keyman-config/km-package-install +++ b/linux/keyman-config/km-package-install @@ -4,7 +4,7 @@ import argparse import logging import sys import os -from keyman_config import __version__, KeymanApiUrl +from keyman_config import __version__, KeymanApiUrl, secure_lookup from keyman_config.uninstall_kmp import uninstall_kmp import datetime @@ -140,17 +140,18 @@ def main(): if not kbdata: logging.error("km-package-install: error: Could not download keyboard data for %s", args.package) sys.exit(3) - if installed_kmp_ver: - if kbdata['version'] == installed_kmp_ver: + kbdata_version = secure_lookup(kbdata, 'version') + if installed_kmp_ver and kbdata_version: + if kbdata_version == installed_kmp_ver: logging.error( "km-package-install: The %s version of the %s keyboard package is already installed.", installed_kmp_ver, args.package) sys.exit(1) - elif float(kbdata['version']) > float(installed_kmp_ver): + elif float(kbdata_version) > float(installed_kmp_ver): logging.error( "km-package-install: A newer version of %s keyboard package is available. " + "Uninstalling old version %s then downloading and installing new version %s.", - args.package, installed_kmp_ver, kbdata['version']) + args.package, installed_kmp_ver, kbdata_version) uninstall_kmp(args.package, args.shared) kmpfile = get_kmp(args.package) From 85d33cefcfa2793f8edaa8631f642c42fb5436ec Mon Sep 17 00:00:00 2001 From: Eberhard Beilharz Date: Mon, 31 Jan 2022 16:21:49 +0100 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Marc Durdin --- linux/keyman-config/keyman_config/install_window.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux/keyman-config/keyman_config/install_window.py b/linux/keyman-config/keyman_config/install_window.py index 149cafa3b0..6f71f1b909 100755 --- a/linux/keyman-config/keyman_config/install_window.py +++ b/linux/keyman-config/keyman_config/install_window.py @@ -80,7 +80,7 @@ class InstallKmpWindow(Gtk.Dialog): self.kbname = keyboardid self.checkcontinue = True - if installed_kmp_ver and secure_lookup('version', 'description'): + if installed_kmp_ver and secure_lookup(info, 'version', 'description'): if secure_lookup(info, 'version', 'description') == installed_kmp_ver: dialog = Gtk.MessageDialog( viewkmp, 0, Gtk.MessageType.QUESTION,