From 82e24430491591d905ea64c7662c2b714398b159 Mon Sep 17 00:00:00 2001 From: toofar Date: Fri, 27 Oct 2023 17:44:22 +1300 Subject: fix non 6.6 resource entry lookup We need to pass objects, not indexes. Test with Qt 6.5 to exercise this path. --- qutebrowser/misc/pakjoy.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/qutebrowser/misc/pakjoy.py b/qutebrowser/misc/pakjoy.py index 5fc1d2816..cdf2b4744 100644 --- a/qutebrowser/misc/pakjoy.py +++ b/qutebrowser/misc/pakjoy.py @@ -130,11 +130,11 @@ class PakParser: if manifest is not None: return suspected_entry, manifest - # didn't find it via the prevously known ID, let's search them all... - for entry in entries: - manifest = self._maybe_get_hangouts_manifest(entry) + # didn't find it via the previously known ID, let's search them all... + for id_ in entries: + manifest = self._maybe_get_hangouts_manifest(entries[id_]) if manifest is not None: - return entry, manifest + return entries[id_], manifest raise binparsing.ParseError("Couldn't find hangouts manifest") -- cgit v1.2.3-54-g00ecf From 3bde102fe0a317dc97b459da7cfeaffc1c439932 Mon Sep 17 00:00:00 2001 From: toofar Date: Fri, 27 Oct 2023 17:54:08 +1300 Subject: run pakjoy for real at application boot It all just works! Changes from the `__main__` implementation down below: * copy the whole resources directory instead of just the one file: looking at the implementation around QTWEBENGINE_RESOURCES_PATH it uses the main resources file to find the one directory and then loads the other resources from that dir. So I'm assuming it was crashing because it couldn't find the other resources. Stack trace was: #1 0x00007fffe95f1dca in content::ContentMainRunnerImpl::Initialize(content::ContentMainParams) () from /usr/local/Qt-6.6.0/lib/libQt6WebEngineCore.so.6 #2 0x00007fffe628cf09 in QtWebEngineCore::WebEngineContext::WebEngineContext() () from /usr/local/Qt-6.6.0/lib/libQt6WebEngineCore.so.6 #3 0x00007fffe628dfa4 in QtWebEngineCore::WebEngineContext::current() () from /usr/local/Qt-6.6.0/lib/libQt6WebEngineCore.so.6 #4 0x00000210b845c780 in ?? () #5 0x00000210b845c780 in ?? () #6 0x00007fffffffd480 in ?? () #7 0x00007fffe62391bb in QtWebEngineCore::ProfileAdapter::ProfileAdapter(QString const&) () from /usr/local/Qt-6.6.0/lib/libQt6WebEngineCore.so.6 * write stuff to our cache dir, not tmp ;) I haven't actually checked this fixes an affected version of Qt (need to rebuild or get from riverbank pypi). But I unpacked the pak file and it has the right fake URL in it. --- qutebrowser/app.py | 6 +++++- qutebrowser/misc/pakjoy.py | 25 +++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/qutebrowser/app.py b/qutebrowser/app.py index 778c248c2..4a6284eb1 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -50,7 +50,7 @@ from qutebrowser.keyinput import macros, eventfilter from qutebrowser.mainwindow import mainwindow, prompt, windowundo from qutebrowser.misc import (ipc, savemanager, sessions, crashsignal, earlyinit, sql, cmdhistory, backendproblem, - objects, quitter, nativeeventfilter) + objects, quitter, nativeeventfilter, pakjoy) from qutebrowser.utils import (log, version, message, utils, urlutils, objreg, resources, usertypes, standarddir, error, qtutils, debug) @@ -76,6 +76,10 @@ def run(args): log.init.debug("Initializing config...") configinit.early_init(args) + # Run after we've imported Qt but before webengine is initialized. + # TODO: check: backend, Qt version, frozen + pakjoy.patch() + log.init.debug("Initializing application...") app = Application(args) objects.qapp = app diff --git a/qutebrowser/misc/pakjoy.py b/qutebrowser/misc/pakjoy.py index cdf2b4744..7e0d139c4 100644 --- a/qutebrowser/misc/pakjoy.py +++ b/qutebrowser/misc/pakjoy.py @@ -26,10 +26,14 @@ This is a "best effort" parser. If it errors out, we don't apply the workaround instead of crashing. """ +import os +import shutil +import pathlib import dataclasses from typing import ClassVar, IO, Optional, Dict, Tuple from qutebrowser.misc import binparsing +from qutebrowser.utils import qtutils, standarddir HANGOUTS_MARKER = b"// Extension ID: nkeimhogjdpnpccoofpliimaahmaaome" HANGOUTS_ID = 36197 # as found by toofar @@ -139,6 +143,27 @@ class PakParser: raise binparsing.ParseError("Couldn't find hangouts manifest") +def patch(): + resources_path = qtutils.library_path(qtutils.LibraryPath.data) / "resources" + work_dir = pathlib.Path(standarddir.cache()) / "webengine_resources_pak_quirk" + patched_file = work_dir / "qtwebengine_resources.pak" + + print(f"{work_dir=} {work_dir.exists()=}") + print(f"{resources_path=}") + if work_dir.exists(): + shutil.rmtree(work_dir) + + shutil.copytree(resources_path, work_dir) + + with open(patched_file, "r+b") as f: + parser = PakParser(f) + offset = parser.find_patch_offset() + f.seek(offset) + f.write(REPLACEMENT_URL) + + os.environ["QTWEBENGINE_RESOURCES_PATH"] = str(work_dir) + + if __name__ == "__main__": import shutil shutil.copy("/usr/share/qt6/resources/qtwebengine_resources.pak", "/tmp/test.pak") -- cgit v1.2.3-54-g00ecf From 931b65330136f5400a021f07d73e9f38381db669 Mon Sep 17 00:00:00 2001 From: toofar Date: Sat, 28 Oct 2023 15:58:09 +1300 Subject: move resources patching later We need to set the environment variable before webengine reads the resources file. It could be unhelpful to do the patching before the IPC / open-in-existing-instance stuff, because that could lead to removing the resources file the running instance is using, any later than that is just down to developer preference I think. I chose to move it as late as possible for now, just for clarity on where in the lifecycle it's necessary. (This also means we can skip the separate backend check, but that is pretty minor.) I moved the patching later and later at init and verified that if it is done before the profiles are initialized it successfully prevents the crash. If done after the profiles are initialized we still crash. I do remember looking at the profile initialization when trying to disable extensions so that makes sense. --- qutebrowser/app.py | 6 +----- qutebrowser/browser/webengine/webenginesettings.py | 5 +++++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/qutebrowser/app.py b/qutebrowser/app.py index 4a6284eb1..778c248c2 100644 --- a/qutebrowser/app.py +++ b/qutebrowser/app.py @@ -50,7 +50,7 @@ from qutebrowser.keyinput import macros, eventfilter from qutebrowser.mainwindow import mainwindow, prompt, windowundo from qutebrowser.misc import (ipc, savemanager, sessions, crashsignal, earlyinit, sql, cmdhistory, backendproblem, - objects, quitter, nativeeventfilter, pakjoy) + objects, quitter, nativeeventfilter) from qutebrowser.utils import (log, version, message, utils, urlutils, objreg, resources, usertypes, standarddir, error, qtutils, debug) @@ -76,10 +76,6 @@ def run(args): log.init.debug("Initializing config...") configinit.early_init(args) - # Run after we've imported Qt but before webengine is initialized. - # TODO: check: backend, Qt version, frozen - pakjoy.patch() - log.init.debug("Initializing application...") app = Application(args) objects.qapp = app diff --git a/qutebrowser/browser/webengine/webenginesettings.py b/qutebrowser/browser/webengine/webenginesettings.py index d0b6b5beb..168fb8280 100644 --- a/qutebrowser/browser/webengine/webenginesettings.py +++ b/qutebrowser/browser/webengine/webenginesettings.py @@ -24,6 +24,7 @@ from qutebrowser.browser.webengine import (spell, webenginequtescheme, cookies, webenginedownloads, notification) from qutebrowser.config import config, websettings from qutebrowser.config.websettings import AttributeInfo as Attr +from qutebrowser.misc import pakjoy from qutebrowser.utils import (standarddir, qtutils, message, log, urlmatch, usertypes, objreg, version) if TYPE_CHECKING: @@ -546,6 +547,10 @@ def init(): _global_settings = WebEngineSettings(_SettingsWrapper()) log.init.debug("Initializing profiles...") + + # Apply potential resource patches before initializing profiles. + pakjoy.patch() + _init_default_profile() init_private_profile() config.instance.changed.connect(_update_settings) -- cgit v1.2.3-54-g00ecf From 25e0d02de4b2c397b9e97daeffb842f7d93e13c6 Mon Sep 17 00:00:00 2001 From: toofar Date: Sat, 28 Oct 2023 16:15:45 +1300 Subject: Only patch resources on 6.6.0 The only affected version is 6.6.0. I'm passing `avoid_init` because this patching is done right before the user agent is parsed (which only happens on Qt5). It's been proposed that we don't need to do this patching for users running on distros that backported the fix to their webengine. Which I think means that we would only do the patching on our bundled installer version for window and mac. But I'm not sure I agree with that. I would rather not have had to re-compile the fix into my build, and I don't know what other distributors position on backporting patches is either. For example I'm on debian, if they had 6.6.0, would they have backported this fix? It looks like they've only got build related patches on the current version for example: https://sources.debian.org/patches/qt6-webengine/6.4.2-final+dfsg-12/ And there's the flatpak version etc. Anyway, if we want to add that check in too I think it would look like: if not hasattr(sys, 'frozen'): # Assume if we aren't frozen we are running against a patched Qt # version. So only apply this quirk in our bundled installers. return --- qutebrowser/misc/pakjoy.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/qutebrowser/misc/pakjoy.py b/qutebrowser/misc/pakjoy.py index 7e0d139c4..c2647c478 100644 --- a/qutebrowser/misc/pakjoy.py +++ b/qutebrowser/misc/pakjoy.py @@ -33,7 +33,7 @@ import dataclasses from typing import ClassVar, IO, Optional, Dict, Tuple from qutebrowser.misc import binparsing -from qutebrowser.utils import qtutils, standarddir +from qutebrowser.utils import qtutils, standarddir, version, utils HANGOUTS_MARKER = b"// Extension ID: nkeimhogjdpnpccoofpliimaahmaaome" HANGOUTS_ID = 36197 # as found by toofar @@ -144,6 +144,10 @@ class PakParser: def patch(): + versions = version.qtwebengine_versions(avoid_init=True) + if versions.webengine != utils.VersionNumber(6, 6): + return + resources_path = qtutils.library_path(qtutils.LibraryPath.data) / "resources" work_dir = pathlib.Path(standarddir.cache()) / "webengine_resources_pak_quirk" patched_file = work_dir / "qtwebengine_resources.pak" -- cgit v1.2.3-54-g00ecf From b161ec1feade62411e882853a45677e56289eb2e Mon Sep 17 00:00:00 2001 From: toofar Date: Fri, 3 Nov 2023 13:17:32 +1300 Subject: Fix lint, move resource dir copying to method Fixes lint: shadows variables, docstrings, unused attribute Move resource dir copying to its own method: * will probably help with unit tests * allows for cleaner overriding of what file to patch (instead of a big nested block) Adds a sneaky `assert` statement in there because the pak file should definitely exist at that location. Although it not existing shouldn't really be fatal... Maintains the thing in `__main__`, although I'm not sure if we need to keep that? --- qutebrowser/misc/pakjoy.py | 67 ++++++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/qutebrowser/misc/pakjoy.py b/qutebrowser/misc/pakjoy.py index c2647c478..ba0c216c4 100644 --- a/qutebrowser/misc/pakjoy.py +++ b/qutebrowser/misc/pakjoy.py @@ -1,4 +1,3 @@ - # SPDX-FileCopyrightText: Florian Bruhin (The-Compiler) # # SPDX-License-Identifier: GPL-3.0-or-later @@ -50,7 +49,7 @@ class Pak5Header: encoding: int # uint32 resource_count: int # uint16 - alias_count: int # uint16 + _alias_count: int # uint16 _FORMAT: ClassVar[str] = ' None: """Parse the .pak file from the given file object.""" - version = binparsing.unpack(" int: + """Return byte offset of TARGET_URL into the pak file.""" try: return self.manifest_entry.file_offset + self.manifest.index(TARGET_URL) except ValueError: @@ -143,45 +145,46 @@ class PakParser: raise binparsing.ParseError("Couldn't find hangouts manifest") -def patch(): - versions = version.qtwebengine_versions(avoid_init=True) - if versions.webengine != utils.VersionNumber(6, 6): - return - - resources_path = qtutils.library_path(qtutils.LibraryPath.data) / "resources" +def copy_webengine_resources(): + """Copy qtwebengine resources to local dir for patching.""" + resources_dir = qtutils.library_path(qtutils.LibraryPath.data) / "resources" work_dir = pathlib.Path(standarddir.cache()) / "webengine_resources_pak_quirk" - patched_file = work_dir / "qtwebengine_resources.pak" - print(f"{work_dir=} {work_dir.exists()=}") - print(f"{resources_path=}") if work_dir.exists(): + # TODO: make backup? shutil.rmtree(work_dir) - shutil.copytree(resources_path, work_dir) - - with open(patched_file, "r+b") as f: - parser = PakParser(f) - offset = parser.find_patch_offset() - f.seek(offset) - f.write(REPLACEMENT_URL) + shutil.copytree(resources_dir, work_dir) os.environ["QTWEBENGINE_RESOURCES_PATH"] = str(work_dir) + return work_dir -if __name__ == "__main__": - import shutil - shutil.copy("/usr/share/qt6/resources/qtwebengine_resources.pak", "/tmp/test.pak") - with open("/tmp/test.pak", "r+b") as f: +def patch(file_to_patch: pathlib.Path = None): + """Apply any patches to webengine resource pak files.""" + versions = version.qtwebengine_versions(avoid_init=True) + if versions.webengine != utils.VersionNumber(6, 6): + return + + if not file_to_patch: + file_to_patch = copy_webengine_resources() / "qtwebengine_resources.pak" + assert file_to_patch.exists() + + with open(file_to_patch, "r+b") as f: parser = PakParser(f) - print(parser.manifest_entry) - print(parser.manifest) offset = parser.find_patch_offset() f.seek(offset) f.write(REPLACEMENT_URL) - with open("/tmp/test.pak", "rb") as f: - parser = PakParser(f) - print(parser.manifest_entry) - print(parser.manifest) +if __name__ == "__main__": + output_test_file = pathlib.Path("/tmp/test.pak") + shutil.copy("/usr/share/qt6/resources/qtwebengine_resources.pak", output_test_file) + patch(output_test_file) + + with open(output_test_file, "rb") as fd: + reparsed = PakParser(fd) + + print(reparsed.manifest_entry) + print(reparsed.manifest) -- cgit v1.2.3-54-g00ecf From c7c856ec6d52e8a57216b611cf0115b2a2b2e87b Mon Sep 17 00:00:00 2001 From: toofar Date: Fri, 3 Nov 2023 13:43:31 +1300 Subject: Add log messages, catch parse errors. The "doesn't exist at expected location" error message is a bit misleading, it actually prints the location in the directory we copied stuff to instead of the source one. Oh well, it's good enough. --- qutebrowser/misc/pakjoy.py | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/qutebrowser/misc/pakjoy.py b/qutebrowser/misc/pakjoy.py index ba0c216c4..ce856bea7 100644 --- a/qutebrowser/misc/pakjoy.py +++ b/qutebrowser/misc/pakjoy.py @@ -32,7 +32,7 @@ import dataclasses from typing import ClassVar, IO, Optional, Dict, Tuple from qutebrowser.misc import binparsing -from qutebrowser.utils import qtutils, standarddir, version, utils +from qutebrowser.utils import qtutils, standarddir, version, utils, log HANGOUTS_MARKER = b"// Extension ID: nkeimhogjdpnpccoofpliimaahmaaome" HANGOUTS_ID = 36197 # as found by toofar @@ -150,6 +150,11 @@ def copy_webengine_resources(): resources_dir = qtutils.library_path(qtutils.LibraryPath.data) / "resources" work_dir = pathlib.Path(standarddir.cache()) / "webengine_resources_pak_quirk" + log.misc.debug( + "Copying webengine resources for quirk patching: " + f"{resources_dir} -> {work_dir}" + ) + if work_dir.exists(): # TODO: make backup? shutil.rmtree(work_dir) @@ -169,13 +174,23 @@ def patch(file_to_patch: pathlib.Path = None): if not file_to_patch: file_to_patch = copy_webengine_resources() / "qtwebengine_resources.pak" - assert file_to_patch.exists() + + if not file_to_patch.exists(): + log.misc.error( + "Resource pak doesn't exist at expected location! " + f"Not applying quirks. Expected location: {file_to_patch}" + ) + return with open(file_to_patch, "r+b") as f: - parser = PakParser(f) - offset = parser.find_patch_offset() - f.seek(offset) - f.write(REPLACEMENT_URL) + try: + parser = PakParser(f) + log.misc.debug(f"Patching pak entry: {parser.manifest_entry}") + offset = parser.find_patch_offset() + f.seek(offset) + f.write(REPLACEMENT_URL) + except binparsing.ParseError: + log.misc.exception("Failed to apply quirk to resources pak.") if __name__ == "__main__": -- cgit v1.2.3-54-g00ecf From f26a477d7cb61404f5c426fee346d2dffc5c7d45 Mon Sep 17 00:00:00 2001 From: toofar Date: Sun, 5 Nov 2023 14:07:57 +1300 Subject: add tests for pakjoy There is one test which does does the whole run through with the real resources file from the Qt install, patches is to the cache dir, reads it back and checks the json. All the other tests either use constructed pak files or stop on earlier error cases. For testing the actual parsing of the pak files I threw together a quick factor method to make them. I went back and forth over the parse code and looked for more error cases, but it looks pretty solid to me --- qutebrowser/misc/binparsing.py | 1 - qutebrowser/misc/pakjoy.py | 10 +- tests/unit/misc/test_pakjoy.py | 277 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 284 insertions(+), 4 deletions(-) create mode 100644 tests/unit/misc/test_pakjoy.py diff --git a/qutebrowser/misc/binparsing.py b/qutebrowser/misc/binparsing.py index 7627ef6cf..81e2e6dbb 100644 --- a/qutebrowser/misc/binparsing.py +++ b/qutebrowser/misc/binparsing.py @@ -41,4 +41,3 @@ def safe_seek(fobj: IO[bytes], pos: int) -> None: fobj.seek(pos) except (OSError, OverflowError) as e: raise ParseError(e) - diff --git a/qutebrowser/misc/pakjoy.py b/qutebrowser/misc/pakjoy.py index ce856bea7..84ab5218b 100644 --- a/qutebrowser/misc/pakjoy.py +++ b/qutebrowser/misc/pakjoy.py @@ -77,8 +77,7 @@ class PakEntry: class PakParser: - """Parse webengine pak and find patch location to disable Google Meet extension. - """ + """Parse webengine pak and find patch location to disable Google Meet extension.""" def __init__(self, fobj: IO[bytes]) -> None: """Parse the .pak file from the given file object.""" @@ -173,7 +172,11 @@ def patch(file_to_patch: pathlib.Path = None): return if not file_to_patch: - file_to_patch = copy_webengine_resources() / "qtwebengine_resources.pak" + try: + file_to_patch = copy_webengine_resources() / "qtwebengine_resources.pak" + except OSError: + log.misc.exception("Failed to copy webengine resources, not applying quirk") + return if not file_to_patch.exists(): log.misc.error( @@ -195,6 +198,7 @@ def patch(file_to_patch: pathlib.Path = None): if __name__ == "__main__": output_test_file = pathlib.Path("/tmp/test.pak") + #shutil.copy("/opt/google/chrome/resources.pak", output_test_file) shutil.copy("/usr/share/qt6/resources/qtwebengine_resources.pak", output_test_file) patch(output_test_file) diff --git a/tests/unit/misc/test_pakjoy.py b/tests/unit/misc/test_pakjoy.py new file mode 100644 index 000000000..5c35d0111 --- /dev/null +++ b/tests/unit/misc/test_pakjoy.py @@ -0,0 +1,277 @@ +# SPDX-FileCopyrightText: Florian Bruhin (The-Compiler) +# +# SPDX-License-Identifier: GPL-3.0-or-later + +import os +import io +import json +import struct +import pathlib +import logging + +import pytest + +from qutebrowser.misc import pakjoy, binparsing +from qutebrowser.utils import utils, version, standarddir + + +pytest.importorskip("qutebrowser.qt.webenginecore") + + +versions = version.qtwebengine_versions(avoid_init=True) + + +@pytest.fixture +def skipifneeded(): + """Used to skip happy path tests with the real resources file. + + Since we don't know how reliably the Google Meet hangouts extensions is + reliably in the resource files, and this quirk is only targeting 6.6 + anyway. + """ + if versions.webengine != utils.VersionNumber(6, 6): + raise pytest.skip("Code under test only runs on 6.6") + + +@pytest.fixture(autouse=True) +def clean_env(): + yield + if "QTWEBENGINE_RESOURCES_PATH" in os.environ: + del os.environ["QTWEBENGINE_RESOURCES_PATH"] + + +def patch_version(monkeypatch, *args): + monkeypatch.setattr( + pakjoy.version, + "qtwebengine_versions", + lambda **kwargs: version.WebEngineVersions( + webengine=utils.VersionNumber(*args), + chromium=None, + source="unittest", + ) + ) + + +@pytest.fixture +def unaffected_version(monkeypatch): + patch_version(monkeypatch, 6, 6, 1) + + +@pytest.fixture +def affected_version(monkeypatch): + patch_version(monkeypatch, 6, 6) + + +def test_version_gate(unaffected_version, mocker): + + fake_open = mocker.patch("qutebrowser.misc.pakjoy.open") + pakjoy.patch() + assert not fake_open.called + + +@pytest.fixture(autouse=True) +def tmp_cache(tmp_path, monkeypatch): + monkeypatch.setattr(pakjoy.standarddir, "cache", lambda: tmp_path) + return str(tmp_path) + + +def json_without_comments(bytestring): + str_without_comments = "\n".join( + [ + line + for line in + bytestring.decode("utf-8").split("\n") + if not line.strip().startswith("//") + ] + ) + return json.loads(str_without_comments) + + +@pytest.mark.usefixtures("affected_version") +class TestWithRealResourcesFile: + """Tests that use the real pak file form the Qt installation.""" + + def test_happy_path(self, skipifneeded): + # Go through the full patching processes with the real resources file from + # the current installation. Make sure our replacement string is in it + # afterwards. + pakjoy.patch() + + patched_resources = pathlib.Path(os.environ["QTWEBENGINE_RESOURCES_PATH"]) + + with open(patched_resources / "qtwebengine_resources.pak", "rb") as fd: + reparsed = pakjoy.PakParser(fd) + + json_manifest = json_without_comments(reparsed.manifest) + + assert pakjoy.REPLACEMENT_URL.decode("utf-8") in json_manifest[ + "externally_connectable" + ]["matches"] + + def test_copying_resources(self): + # Test we managed to copy some files over + work_dir = pakjoy.copy_webengine_resources() + + assert work_dir.exists() + assert work_dir == standarddir.cache() / "webengine_resources_pak_quirk" + assert (work_dir / "qtwebengine_resources.pak").exists() + assert len(list(work_dir.glob("*"))) > 1 + + def test_copying_resources_overwrites(self): + work_dir = pakjoy.copy_webengine_resources() + tmpfile = work_dir / "tmp.txt" + tmpfile.touch() + + pakjoy.copy_webengine_resources() + assert not tmpfile.exists() + + @pytest.mark.parametrize("osfunc", ["copytree", "rmtree"]) + def test_copying_resources_oserror(self, monkeypatch, caplog, osfunc): + # Test errors from the calls to shutil are handled + pakjoy.copy_webengine_resources() # run twice so we hit rmtree too + caplog.clear() + + def raiseme(err): + raise err + + monkeypatch.setattr(pakjoy.shutil, osfunc, lambda *_args: raiseme(PermissionError(osfunc))) + with caplog.at_level(logging.ERROR, "misc"): + pakjoy.patch() + assert caplog.messages == ["Failed to copy webengine resources, not applying quirk"] + + def test_expected_file_not_found(self, tmp_cache, monkeypatch, caplog): + with caplog.at_level(logging.ERROR, "misc"): + pakjoy.patch(pathlib.Path(tmp_cache) / "doesntexist") + assert caplog.messages[-1].startswith( + "Resource pak doesn't exist at expected location! " + "Not applying quirks. Expected location: " + ) + + +def json_manifest_factory(extension_id=pakjoy.HANGOUTS_MARKER, url=pakjoy.TARGET_URL): + assert isinstance(extension_id, bytes) + assert isinstance(url, bytes) + + return f""" + {{ + {extension_id.decode("utf-8")} + "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDAQt2ZDdPfoSe/JI6ID5bgLHRCnCu9T36aYczmhw/tnv6QZB2I6WnOCMZXJZlRdqWc7w9jo4BWhYS50Vb4weMfh/I0On7VcRwJUgfAxW2cHB+EkmtI1v4v/OU24OqIa1Nmv9uRVeX0GjhQukdLNhAE6ACWooaf5kqKlCeK+1GOkQIDAQAB", + "name": "Google Hangouts", + // Note: Always update the version number when this file is updated. Chrome + // triggers extension preferences update on the version increase. + "version": "1.3.21", + "manifest_version": 2, + "externally_connectable": {{ + "matches": [ + "{url.decode("utf-8")}", + "http://localhost:*/*" + ] + }} + }} + """.strip().encode("utf-8") + + +def pak_factory(version=5, entries=None, encoding=1, sentinel_position=-1): + if entries is None: + entries = [json_manifest_factory()] + + buffer = io.BytesIO() + buffer.write(struct.pack(" Date: Sun, 5 Nov 2023 17:16:34 +1300 Subject: Fix resource pak file location on mac. On CI I'm seeing: No such file or directory: '/Users/runner/work/qutebrowser/qutebrowser/.tox/py39-pyqt515/lib/python3.9/site-packages/PyQt5/Qt5/resources' Downloading the PyQt6_WebEngine_Qt6 wheel for mac from PyPI I can see the pak file is at: ./PyQt6/Qt6/lib/QtWebEngineCore.framework/Resources/qtwebengine_resources.pak And on a qutebrowser install made by pyinstaller it is at (symlinks in curly braces): /Applications/qutebrowser.app/Contents/{Resources,Frameworks}/PyQt6/Qt6/lib/QtWebEngineCore.framework/{Resources,Versions/A/Resources}/qtwebengine_resources.pak And the Qt data path for reference is: /Applications/qutebrowser.app/Contents/Frameworks/PyQt6/Qt6 So it looks like essentially we need to add a "lib/QtWebEngineCore.framework" in there and capitalise "Resources". A bit annoying to have the special case and hardocde paths like this. But it should be pretty stable? I had a look at importlib_resources to try to see if it could fine this stuff for us but I don't think it can. I've checked with the pyinstaller windows builds and with the windows wheel from PyPI and neither seem to need any special handling. --- qutebrowser/misc/pakjoy.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/qutebrowser/misc/pakjoy.py b/qutebrowser/misc/pakjoy.py index 84ab5218b..52e50e0f8 100644 --- a/qutebrowser/misc/pakjoy.py +++ b/qutebrowser/misc/pakjoy.py @@ -146,7 +146,15 @@ class PakParser: def copy_webengine_resources(): """Copy qtwebengine resources to local dir for patching.""" - resources_dir = qtutils.library_path(qtutils.LibraryPath.data) / "resources" + resources_dir = qtutils.library_path(qtutils.LibraryPath.data) + if utils.is_mac: + # I'm not sure how to arrive at this path without hardcoding it + # ourselves. importlib_resources("PyQt6.Qt6") can serve as a + # replacement for the qtutils bit but it doesn't seem to help find the + # actually Resources folder. + resources_dir /= "lib" / "QtWebEngineCore.framework" / "Resources" + else: + resources_dir /= "resources" work_dir = pathlib.Path(standarddir.cache()) / "webengine_resources_pak_quirk" log.misc.debug( -- cgit v1.2.3-54-g00ecf From cbcf941c838fd6895c6bd6104ca5fe5996de4949 Mon Sep 17 00:00:00 2001 From: toofar Date: Sun, 5 Nov 2023 17:20:26 +1300 Subject: Use safe_seek() I have no idea in what case these errors would crop up. But vulture said the function was unused, not it's used. --- qutebrowser/misc/pakjoy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/misc/pakjoy.py b/qutebrowser/misc/pakjoy.py index 52e50e0f8..8ad932bad 100644 --- a/qutebrowser/misc/pakjoy.py +++ b/qutebrowser/misc/pakjoy.py @@ -198,7 +198,7 @@ def patch(file_to_patch: pathlib.Path = None): parser = PakParser(f) log.misc.debug(f"Patching pak entry: {parser.manifest_entry}") offset = parser.find_patch_offset() - f.seek(offset) + binparsing.safe_seek(f, offset) f.write(REPLACEMENT_URL) except binparsing.ParseError: log.misc.exception("Failed to apply quirk to resources pak.") -- cgit v1.2.3-54-g00ecf From 30cd758d14dba8c22cd957068cb6859c1c694d19 Mon Sep 17 00:00:00 2001 From: toofar Date: Sun, 5 Nov 2023 17:31:44 +1300 Subject: fix lint --- qutebrowser/misc/pakjoy.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/qutebrowser/misc/pakjoy.py b/qutebrowser/misc/pakjoy.py index 8ad932bad..ac07e3f76 100644 --- a/qutebrowser/misc/pakjoy.py +++ b/qutebrowser/misc/pakjoy.py @@ -128,7 +128,7 @@ class PakParser: return {entry.resource_id: entry for entry in entries} - def _find_manifest(self, entries: Dict[int, PakEntry]) -> Tuple[PakEntry, str]: + def _find_manifest(self, entries: Dict[int, PakEntry]) -> Tuple[PakEntry, bytes]: if HANGOUTS_ID in entries: suspected_entry = entries[HANGOUTS_ID] manifest = self._maybe_get_hangouts_manifest(suspected_entry) @@ -144,7 +144,7 @@ class PakParser: raise binparsing.ParseError("Couldn't find hangouts manifest") -def copy_webengine_resources(): +def copy_webengine_resources() -> pathlib.Path: """Copy qtwebengine resources to local dir for patching.""" resources_dir = qtutils.library_path(qtutils.LibraryPath.data) if utils.is_mac: @@ -173,7 +173,7 @@ def copy_webengine_resources(): return work_dir -def patch(file_to_patch: pathlib.Path = None): +def patch(file_to_patch: pathlib.Path = None) -> None: """Apply any patches to webengine resource pak files.""" versions = version.qtwebengine_versions(avoid_init=True) if versions.webengine != utils.VersionNumber(6, 6): -- cgit v1.2.3-54-g00ecf From 9656f43a09931be784285af378cb2fbd4657827c Mon Sep 17 00:00:00 2001 From: toofar Date: Sun, 5 Nov 2023 19:25:36 +1300 Subject: help mypy deal with pathlib Seems mypy and I agree on something. It thinks that pathlib is abusing the division operation and making python a less intuitive language. Not sure why it is complaining though really, doing `Path / "one" / "two"` seems to work fine in practice, and its on the pathlib documentation page. I was seeing this error locally and on CI qutebrowser/misc/pakjoy.py:155: error: Unsupported left operand type for / ("str") [operator] resources_dir /= "lib" / "QtWebEngineCore.framework" / "Resources" --- qutebrowser/misc/pakjoy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qutebrowser/misc/pakjoy.py b/qutebrowser/misc/pakjoy.py index ac07e3f76..084ab46bd 100644 --- a/qutebrowser/misc/pakjoy.py +++ b/qutebrowser/misc/pakjoy.py @@ -152,7 +152,7 @@ def copy_webengine_resources() -> pathlib.Path: # ourselves. importlib_resources("PyQt6.Qt6") can serve as a # replacement for the qtutils bit but it doesn't seem to help find the # actually Resources folder. - resources_dir /= "lib" / "QtWebEngineCore.framework" / "Resources" + resources_dir /= pathlib.Path("lib", "QtWebEngineCore.framework", "Resources") else: resources_dir /= "resources" work_dir = pathlib.Path(standarddir.cache()) / "webengine_resources_pak_quirk" -- cgit v1.2.3-54-g00ecf