From 4e7e21ff6cf10e22bd75f974e8547e26ab95cfaf Mon Sep 17 00:00:00 2001 From: Bananaman <38923130+Bananaman@users.noreply.github.com> Date: Fri, 24 Mar 2023 21:19:51 +0100 Subject: [PATCH 1/5] Ignore flatpak-builder cache --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 1ead167..4b0947a 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ /subprojects/blueprint-compiler /.flatpak +/.flatpak-builder /.vscode \ No newline at end of file From eda22c7ea72f6d4a232215c7dcf9d235a2b84ab4 Mon Sep 17 00:00:00 2001 From: Bananaman <38923130+Bananaman@users.noreply.github.com> Date: Fri, 24 Mar 2023 21:21:24 +0100 Subject: [PATCH 2/5] Move game data JSON formatter to re-usable location --- src/meson.build | 1 + src/utils/game_data_to_json.py | 25 +++++++++++++++++++++++++ src/utils/save_games.py | 4 +++- 3 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 src/utils/game_data_to_json.py diff --git a/src/meson.build b/src/meson.build index 78d6144..f2c7981 100644 --- a/src/meson.build +++ b/src/meson.build @@ -30,6 +30,7 @@ cartridges_sources = [ 'utils/get_cover.py', 'utils/save_games.py', 'utils/save_cover.py', + 'utils/game_data_to_json.py', 'utils/toggle_hidden.py', 'utils/create_dialog.py', 'utils/create_details_window.py' diff --git a/src/utils/game_data_to_json.py b/src/utils/game_data_to_json.py new file mode 100644 index 0000000..f927809 --- /dev/null +++ b/src/utils/game_data_to_json.py @@ -0,0 +1,25 @@ +# game_data_to_json.py +# +# Copyright 2022-2023 kramo +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# +# SPDX-License-Identifier: GPL-3.0-or-later + +import json +import os + + +def game_data_to_json(data): + return json.dumps(data, indent=4, sort_keys=True) diff --git a/src/utils/save_games.py b/src/utils/save_games.py index 858f08c..fdc6e77 100644 --- a/src/utils/save_games.py +++ b/src/utils/save_games.py @@ -20,6 +20,8 @@ import json import os +from .game_data_to_json import game_data_to_json + def save_games(games): games_dir = os.path.join( @@ -34,5 +36,5 @@ def save_games(games): for game in games: with open(os.path.join(games_dir, f"{game}.json"), "w") as open_file: - open_file.write(json.dumps(games[game], indent=4, sort_keys=True)) + open_file.write(game_data_to_json(games[game])) open_file.close() From 703c65395cf1603ebc9c49c68b28e5a3a9d87fc5 Mon Sep 17 00:00:00 2001 From: Bananaman <38923130+Bananaman@users.noreply.github.com> Date: Fri, 24 Mar 2023 21:44:31 +0100 Subject: [PATCH 3/5] Improved game executable launcher, and added argument validation Now uses proper shell escaping and parsing for all executable arguments, for more robust game launching. All existing game JSON files with the old string values will automatically be converted to the new format on app launch. The executable parsing uses the "shlex" library, which guarantees accurate parsing. We now also use direct process launching (without any intermediary shell) by default, but the old "shell"-based launch method still exists in the code via an alternative flag in `run_command.py` (if we ever need to restore it for some reason). Furthermore, if the user attempts to manually write an improperly escaped argument into the game's details (such as missing closing quotation marks), the GUI will now alert the user that their executable argument is invalid, along with telling them the exact reason why it's invalid. --- src/utils/bottles_parser.py | 5 ++-- src/utils/create_details_window.py | 20 +++++++++++++-- src/utils/get_games.py | 29 ++++++++++++++++++++-- src/utils/heroic_parser.py | 12 ++++----- src/utils/run_command.py | 39 ++++++++++++++++++++++-------- src/utils/steam_parser.py | 4 +-- 6 files changed, 85 insertions(+), 24 deletions(-) diff --git a/src/utils/bottles_parser.py b/src/utils/bottles_parser.py index 85fd3c1..18778a1 100644 --- a/src/utils/bottles_parser.py +++ b/src/utils/bottles_parser.py @@ -112,9 +112,10 @@ def bottles_parser(parent_widget, action): continue values["name"] = game["name"] - values["executable"] = "xdg-open " + shlex.quote( + values["executable"] = [ + "xdg-open", f'bottles:run/{game["bottle"]["name"]}/{game["name"]}' - ) + ] values["hidden"] = False values["source"] = "bottles" values["added"] = current_time diff --git a/src/utils/create_details_window.py b/src/utils/create_details_window.py index 0ca5137..40fc371 100644 --- a/src/utils/create_details_window.py +++ b/src/utils/create_details_window.py @@ -19,6 +19,7 @@ import json import os +import shlex import time from gi.repository import Adw, GdkPixbuf, Gio, GLib, GObject, Gtk @@ -52,7 +53,7 @@ def create_details_window(parent_widget, game_id=None): ) name = Gtk.Entry.new_with_buffer(Gtk.EntryBuffer.new(games[game_id].name, -1)) executable = Gtk.Entry.new_with_buffer( - Gtk.EntryBuffer.new((games[game_id].executable), -1) + Gtk.EntryBuffer.new(shlex.join(games[game_id].executable), -1) ) apply_button = Gtk.Button.new_with_label(_("Apply")) @@ -201,6 +202,21 @@ def create_details_window(parent_widget, game_id=None): final_developer = developer.get_buffer().get_text() final_executable = executable.get_buffer().get_text() + try: + # Attempt to parse using shell parsing rules (doesn't verify executable existence). + final_executable_split = shlex.split( + final_executable, comments=False, posix=True + ) + except Exception as e: + create_dialog( + window, + _("Couldn't Add Game") + if game_id is None + else _("Couldn't Apply Preferences"), + f'{_("Executable")}: {e}.', # e = Shell parsing error message. Not translatable. + ) + return + if game_id is None: if final_name == "": create_dialog( @@ -252,7 +268,7 @@ def create_details_window(parent_widget, game_id=None): values["name"] = final_name values["developer"] = final_developer or None - values["executable"] = final_executable + values["executable"] = final_executable_split path = os.path.join( os.path.join( diff --git a/src/utils/get_games.py b/src/utils/get_games.py index d2e0423..73acfe9 100644 --- a/src/utils/get_games.py +++ b/src/utils/get_games.py @@ -19,6 +19,9 @@ import json import os +import shlex + +from .game_data_to_json import game_data_to_json def get_games(game_ids=None): @@ -39,8 +42,30 @@ def get_games(game_ids=None): game_files = os.listdir(games_dir) for game in game_files: - with open(os.path.join(games_dir, game), "r") as open_file: + with open(os.path.join(games_dir, game), "r+") as open_file: data = json.loads(open_file.read()) + + # Convert any outdated JSON values to our newest data format. + needs_rewrite = False + if "executable" in data and isinstance(data["executable"], str): + needs_rewrite = True + try: + # Use shell parsing to determine what the individual components are. + executable_split = shlex.split( + data["executable"], comments=False, posix=True + ) + except: + # Fallback: Split once at earliest space (1 part if no spaces, else 2 parts). + executable_split = data["executable"].split(" ", 1) + data["executable"] = executable_split + + if needs_rewrite: + open_file.seek(0) + open_file.truncate() + open_file.write(game_data_to_json(data)) + open_file.close() - games[data["game_id"]] = data + + games[data["game_id"]] = data + return games diff --git a/src/utils/heroic_parser.py b/src/utils/heroic_parser.py index 494f6c4..a579391 100644 --- a/src/utils/heroic_parser.py +++ b/src/utils/heroic_parser.py @@ -129,9 +129,9 @@ def heroic_parser(parent_widget, action): values["name"] = game["title"] values["developer"] = game["developer"] values["executable"] = ( - f"start heroic://launch/{app_name}" + ["start", f"heroic://launch/{app_name}"] if os.name == "nt" - else f"xdg-open heroic://launch/{app_name}" + else ["xdg-open", f"heroic://launch/{app_name}"] ) values["hidden"] = False values["source"] = "heroic_epic" @@ -195,9 +195,9 @@ def heroic_parser(parent_widget, action): break values["executable"] = ( - f"start heroic://launch/{app_name}" + ["start", f"heroic://launch/{app_name}"] if os.name == "nt" - else f"xdg-open heroic://launch/{app_name}" + else ["xdg-open", f"heroic://launch/{app_name}"] ) values["hidden"] = False values["source"] = "heroic_gog" @@ -230,9 +230,9 @@ def heroic_parser(parent_widget, action): values["name"] = item["title"] values["executable"] = ( - f"start heroic://launch/{app_name}" + ["start", f"heroic://launch/{app_name}"] if os.name == "nt" - else f"xdg-open heroic://launch/{app_name}" + else ["xdg-open", f"heroic://launch/{app_name}"] ) values["hidden"] = False values["source"] = "heroic_sideload" diff --git a/src/utils/run_command.py b/src/utils/run_command.py index 1400128..4614c2d 100644 --- a/src/utils/run_command.py +++ b/src/utils/run_command.py @@ -18,6 +18,7 @@ # SPDX-License-Identifier: GPL-3.0-or-later import os +import shlex import subprocess import sys @@ -25,15 +26,33 @@ from gi.repository import Gio def run_command(executable): - subprocess.Popen( - [f"flatpak-spawn --host {executable}"] - if os.getenv("FLATPAK_ID") == "hu.kramo.Cartridges" - else executable.split() - if os.name == "nt" - else [executable], - shell=True, - start_new_session=True, - creationflags=subprocess.CREATE_NEW_PROCESS_GROUP if os.name == "nt" else 0, - ) + use_shell = False + if not use_shell: + # The host environment is automatically passed through by Popen. + subprocess.Popen( + ["flatpak-spawn", "--host", *executable] # Flatpak + if os.getenv("FLATPAK_ID") == "hu.kramo.Cartridges" + else executable # Windows + if os.name == "nt" + else executable, # Linux/Others + shell=False, # If true, the extra arguments would incorrectly be given to the shell instead. + start_new_session=True, + creationflags=subprocess.CREATE_NEW_PROCESS_GROUP if os.name == "nt" else 0, + ) + else: + # When launching as a shell, we must pass 1 string with the exact command + # line exactly as we would type it in a shell (with escaped arguments). + subprocess.Popen( + shlex.join( + ["flatpak-spawn", "--host", *executable] # Flatpak + if os.getenv("FLATPAK_ID") == "hu.kramo.Cartridges" + else executable # Windows + if os.name == "nt" + else executable # Linux/Others + ), + shell=True, + start_new_session=True, + creationflags=subprocess.CREATE_NEW_PROCESS_GROUP if os.name == "nt" else 0, + ) if Gio.Settings.new("hu.kramo.Cartridges").get_boolean("exit-after-launch"): sys.exit() diff --git a/src/utils/steam_parser.py b/src/utils/steam_parser.py index bfcc1ef..f99a8e3 100644 --- a/src/utils/steam_parser.py +++ b/src/utils/steam_parser.py @@ -64,9 +64,9 @@ def get_game(task, datatypes, current_time, parent_widget, appmanifest, steam_di return values["executable"] = ( - f'start steam://rungameid/{values["appid"]}' + ["start", f'steam://rungameid/{values["appid"]}'] if os.name == "nt" - else f'xdg-open steam://rungameid/{values["appid"]}' + else ["xdg-open", f'steam://rungameid/{values["appid"]}'] ) values["hidden"] = False values["source"] = "steam" From 214687c9ce2285320125b07a64be59ea34476dac Mon Sep 17 00:00:00 2001 From: Bananaman <38923130+Bananaman@users.noreply.github.com> Date: Fri, 24 Mar 2023 22:26:24 +0100 Subject: [PATCH 4/5] Cleanup: Remove backwards-compatible code Since we aren't interested in backwards compatibility this early in development, let's remove those code chunks to keep the code shorter. --- src/meson.build | 1 - src/utils/game_data_to_json.py | 25 ---------------------- src/utils/get_games.py | 25 +--------------------- src/utils/run_command.py | 39 ++++++++++------------------------ src/utils/save_games.py | 4 +--- 5 files changed, 13 insertions(+), 81 deletions(-) delete mode 100644 src/utils/game_data_to_json.py diff --git a/src/meson.build b/src/meson.build index f2c7981..78d6144 100644 --- a/src/meson.build +++ b/src/meson.build @@ -30,7 +30,6 @@ cartridges_sources = [ 'utils/get_cover.py', 'utils/save_games.py', 'utils/save_cover.py', - 'utils/game_data_to_json.py', 'utils/toggle_hidden.py', 'utils/create_dialog.py', 'utils/create_details_window.py' diff --git a/src/utils/game_data_to_json.py b/src/utils/game_data_to_json.py deleted file mode 100644 index f927809..0000000 --- a/src/utils/game_data_to_json.py +++ /dev/null @@ -1,25 +0,0 @@ -# game_data_to_json.py -# -# Copyright 2022-2023 kramo -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . -# -# SPDX-License-Identifier: GPL-3.0-or-later - -import json -import os - - -def game_data_to_json(data): - return json.dumps(data, indent=4, sort_keys=True) diff --git a/src/utils/get_games.py b/src/utils/get_games.py index 73acfe9..9125107 100644 --- a/src/utils/get_games.py +++ b/src/utils/get_games.py @@ -19,9 +19,6 @@ import json import os -import shlex - -from .game_data_to_json import game_data_to_json def get_games(game_ids=None): @@ -42,28 +39,8 @@ def get_games(game_ids=None): game_files = os.listdir(games_dir) for game in game_files: - with open(os.path.join(games_dir, game), "r+") as open_file: + with open(os.path.join(games_dir, game), "r") as open_file: data = json.loads(open_file.read()) - - # Convert any outdated JSON values to our newest data format. - needs_rewrite = False - if "executable" in data and isinstance(data["executable"], str): - needs_rewrite = True - try: - # Use shell parsing to determine what the individual components are. - executable_split = shlex.split( - data["executable"], comments=False, posix=True - ) - except: - # Fallback: Split once at earliest space (1 part if no spaces, else 2 parts). - executable_split = data["executable"].split(" ", 1) - data["executable"] = executable_split - - if needs_rewrite: - open_file.seek(0) - open_file.truncate() - open_file.write(game_data_to_json(data)) - open_file.close() games[data["game_id"]] = data diff --git a/src/utils/run_command.py b/src/utils/run_command.py index 4614c2d..78b1d0f 100644 --- a/src/utils/run_command.py +++ b/src/utils/run_command.py @@ -26,33 +26,16 @@ from gi.repository import Gio def run_command(executable): - use_shell = False - if not use_shell: - # The host environment is automatically passed through by Popen. - subprocess.Popen( - ["flatpak-spawn", "--host", *executable] # Flatpak - if os.getenv("FLATPAK_ID") == "hu.kramo.Cartridges" - else executable # Windows - if os.name == "nt" - else executable, # Linux/Others - shell=False, # If true, the extra arguments would incorrectly be given to the shell instead. - start_new_session=True, - creationflags=subprocess.CREATE_NEW_PROCESS_GROUP if os.name == "nt" else 0, - ) - else: - # When launching as a shell, we must pass 1 string with the exact command - # line exactly as we would type it in a shell (with escaped arguments). - subprocess.Popen( - shlex.join( - ["flatpak-spawn", "--host", *executable] # Flatpak - if os.getenv("FLATPAK_ID") == "hu.kramo.Cartridges" - else executable # Windows - if os.name == "nt" - else executable # Linux/Others - ), - shell=True, - start_new_session=True, - creationflags=subprocess.CREATE_NEW_PROCESS_GROUP if os.name == "nt" else 0, - ) + # The host environment vars are automatically passed through by Popen. + subprocess.Popen( + ["flatpak-spawn", "--host", *executable] # Flatpak + if os.getenv("FLATPAK_ID") == "hu.kramo.Cartridges" + else executable # Windows + if os.name == "nt" + else executable, # Linux/Others + shell=False, # If true, the extra arguments would incorrectly be given to the shell instead. + start_new_session=True, + creationflags=subprocess.CREATE_NEW_PROCESS_GROUP if os.name == "nt" else 0, + ) if Gio.Settings.new("hu.kramo.Cartridges").get_boolean("exit-after-launch"): sys.exit() diff --git a/src/utils/save_games.py b/src/utils/save_games.py index fdc6e77..858f08c 100644 --- a/src/utils/save_games.py +++ b/src/utils/save_games.py @@ -20,8 +20,6 @@ import json import os -from .game_data_to_json import game_data_to_json - def save_games(games): games_dir = os.path.join( @@ -36,5 +34,5 @@ def save_games(games): for game in games: with open(os.path.join(games_dir, f"{game}.json"), "w") as open_file: - open_file.write(game_data_to_json(games[game])) + open_file.write(json.dumps(games[game], indent=4, sort_keys=True)) open_file.close() From 973ec01b385de15d5676d23a5c378ad12f70383d Mon Sep 17 00:00:00 2001 From: Bananaman <38923130+Bananaman@users.noreply.github.com> Date: Fri, 24 Mar 2023 22:37:01 +0100 Subject: [PATCH 5/5] Cleanup: Remove pointless calls to close() Since "with open()" automatically closes the files anyway. --- src/utils/bottles_parser.py | 1 - src/utils/create_details_window.py | 1 - src/utils/get_games.py | 2 -- src/utils/heroic_parser.py | 4 ---- src/utils/save_games.py | 1 - src/utils/steam_parser.py | 1 - src/utils/toggle_hidden.py | 1 - 7 files changed, 11 deletions(-) diff --git a/src/utils/bottles_parser.py b/src/utils/bottles_parser.py index 18778a1..a2426ab 100644 --- a/src/utils/bottles_parser.py +++ b/src/utils/bottles_parser.py @@ -95,7 +95,6 @@ def bottles_parser(parent_widget, action): with open(os.path.join(bottles_dir, "library.yml"), "r") as open_file: data = open_file.read() - open_file.close() library = yaml.load(data, Loader=yaml.Loader) diff --git a/src/utils/create_details_window.py b/src/utils/create_details_window.py index 40fc371..96b71df 100644 --- a/src/utils/create_details_window.py +++ b/src/utils/create_details_window.py @@ -283,7 +283,6 @@ def create_details_window(parent_widget, game_id=None): if os.path.exists(path): with open(path, "r") as open_file: data = json.loads(open_file.read()) - open_file.close() data.update(values) save_games({game_id: data}) else: diff --git a/src/utils/get_games.py b/src/utils/get_games.py index 9125107..dd40ed8 100644 --- a/src/utils/get_games.py +++ b/src/utils/get_games.py @@ -41,8 +41,6 @@ def get_games(game_ids=None): for game in game_files: with open(os.path.join(games_dir, game), "r") as open_file: data = json.loads(open_file.read()) - open_file.close() - games[data["game_id"]] = data return games diff --git a/src/utils/heroic_parser.py b/src/utils/heroic_parser.py index a579391..8597f7f 100644 --- a/src/utils/heroic_parser.py +++ b/src/utils/heroic_parser.py @@ -107,7 +107,6 @@ def heroic_parser(parent_widget, action): os.path.join(heroic_dir, "lib-cache", "library.json"), "r" ) as open_file: data = open_file.read() - open_file.close() library = json.loads(data) try: @@ -160,7 +159,6 @@ def heroic_parser(parent_widget, action): os.path.join(heroic_dir, "gog_store", "installed.json"), "r" ) as open_file: data = open_file.read() - open_file.close() installed = json.loads(data) for item in installed["installed"]: values = {} @@ -179,7 +177,6 @@ def heroic_parser(parent_widget, action): os.path.join(heroic_dir, "gog_store", "library.json"), "r" ) as open_file: data = open_file.read() - open_file.close() library = json.loads(data) for game in library["games"]: if game["app_name"] == app_name: @@ -214,7 +211,6 @@ def heroic_parser(parent_widget, action): os.path.join(heroic_dir, "sideload_apps", "library.json"), "r" ) as open_file: data = open_file.read() - open_file.close() library = json.loads(data) for item in library["games"]: values = {} diff --git a/src/utils/save_games.py b/src/utils/save_games.py index 858f08c..854c140 100644 --- a/src/utils/save_games.py +++ b/src/utils/save_games.py @@ -35,4 +35,3 @@ def save_games(games): for game in games: with open(os.path.join(games_dir, f"{game}.json"), "w") as open_file: open_file.write(json.dumps(games[game], indent=4, sort_keys=True)) - open_file.close() diff --git a/src/utils/steam_parser.py b/src/utils/steam_parser.py index f99a8e3..0f65860 100644 --- a/src/utils/steam_parser.py +++ b/src/utils/steam_parser.py @@ -49,7 +49,6 @@ def get_game(task, datatypes, current_time, parent_widget, appmanifest, steam_di with open(appmanifest, "r") as open_file: data = open_file.read() - open_file.close() for datatype in datatypes: value = re.findall(f'"{datatype}"\t\t"(.*)"\n', data) values[datatype] = value[0] diff --git a/src/utils/toggle_hidden.py b/src/utils/toggle_hidden.py index 10aa284..21dc9b1 100644 --- a/src/utils/toggle_hidden.py +++ b/src/utils/toggle_hidden.py @@ -36,7 +36,6 @@ def toggle_hidden(game): with open(os.path.join(games_dir, f"{game}.json"), "r") as open_file: data = json.loads(open_file.read()) - open_file.close() data["hidden"] = not data["hidden"]