From e012ff0c93eec3efcaa39db0e94f6ee6fb809e7c Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Sat, 16 Aug 2025 21:39:37 -0500 Subject: [PATCH 1/3] Refactor icon loading to handle more errors --- AddonCatalog.py | 10 -- AddonManager.py | 85 +-------------- addonmanager_icon_utilities.py | 187 +++++++++++++++++++++++++++++++++ 3 files changed, 191 insertions(+), 91 deletions(-) create mode 100644 addonmanager_icon_utilities.py diff --git a/AddonCatalog.py b/AddonCatalog.py index 50a5dcc0..c35daa6a 100644 --- a/AddonCatalog.py +++ b/AddonCatalog.py @@ -233,9 +233,6 @@ def _load_metadata_txt(repo: Addon, data: str): wb_name = wb.strip() if wb_name: repo.requires.add(wb_name) - fci.Console.PrintLog( - f"{repo.display_name} requires FreeCAD addon '{wb_name}'\n" - ) elif line.startswith("pylibs="): python_dependencies = line.split("=")[1].split(",") @@ -243,9 +240,6 @@ def _load_metadata_txt(repo: Addon, data: str): dep = pl.strip() if dep: repo.python_requires.add(dep) - fci.Console.PrintLog( - f"{repo.display_name} requires python package '{dep}'\n" - ) elif line.startswith("optionalpylibs="): optional_python_dependencies = line.split("=")[1].split(",") @@ -253,10 +247,6 @@ def _load_metadata_txt(repo: Addon, data: str): dep = pl.strip() if dep: repo.python_optional.add(dep) - fci.Console.PrintLog( - f"{repo.display_name} optionally imports python package" - + f" '{pl.strip()}'\n" - ) @staticmethod def _load_requirements_txt(repo: Addon, data: str): diff --git a/AddonManager.py b/AddonManager.py index f55c1ea9..42a516c1 100644 --- a/AddonManager.py +++ b/AddonManager.py @@ -29,7 +29,7 @@ import threading from typing import Dict, List -from PySideWrapper import QtGui, QtCore, QtWidgets, QtSvg +from PySideWrapper import QtGui, QtCore, QtWidgets from addonmanager_workers_startup import ( CreateAddonListWorker, @@ -38,6 +38,7 @@ GetAddonScoreWorker, ) from addonmanager_installer_gui import AddonInstallerGUI, MacroInstallerGUI +from addonmanager_icon_utilities import get_icon_for_addon from addonmanager_uninstaller_gui import AddonUninstallerGUI from addonmanager_update_all_gui import UpdateAllGUI import addonmanager_utilities as utils @@ -92,84 +93,6 @@ def QT_TRANSLATE_NOOP(_, txt): INSTANCE = None -class SvgIconEngine(QtGui.QIconEngine): - def __init__(self, svg_bytes: bytes): - super().__init__() - self.renderer = QtSvg.QSvgRenderer(QtCore.QByteArray(svg_bytes)) - - def paint(self, painter: QtGui.QPainter, rect: QtCore.QRect, mode, state): - self.renderer.render(painter, rect) - - def pixmap(self, size: QtCore.QSize, mode, state): - pixmap = QtGui.QPixmap(size) - pixmap.fill(QtCore.Qt.transparent) - painter = QtGui.QPainter(pixmap) - self.renderer.render(painter) - painter.end() - return pixmap - - -def scalable_icon_from_svg_bytes(svg_bytes: bytes) -> QtGui.QIcon: - engine = SvgIconEngine(svg_bytes) - return QtGui.QIcon(engine) - - -def get_icon(repo: Addon, update: bool = False) -> QtGui.QIcon: - """Returns an icon for an Addon. Uses a cached icon if possible, unless `update` is True, - in which case the icon is regenerated.""" - - icon_path = os.path.join(os.path.dirname(__file__), "Resources", "icons") - path = "" - - if not update: - if repo.icon and not repo.icon.isNull() and repo.icon.isValid(): - return repo.icon - elif repo.icon_data: - repo.icon = scalable_icon_from_svg_bytes(repo.icon_data) - return repo.icon - - default_icon = QtGui.QIcon(os.path.join(icon_path, "document-package.svg")) - if repo.repo_type == Addon.Kind.WORKBENCH: - default_icon = QtGui.QIcon(os.path.join(icon_path, "document-package.svg")) - elif repo.repo_type == Addon.Kind.MACRO: - if repo.macro and repo.macro.icon_data: - if repo.macro.icon_extension == "svg": - default_icon = scalable_icon_from_svg_bytes(repo.macro.icon_data) - else: - pixmap = QtGui.QPixmap() - pixmap.loadFromData(repo.macro.icon_data) - default_icon = QtGui.QIcon(pixmap) - elif repo.macro and repo.macro.xpm: - cache_path = fci.DataPaths().cache_dir - am_path = os.path.join(cache_path, "AddonManager", "MacroIcons") - os.makedirs(am_path, exist_ok=True) - path = os.path.join(am_path, repo.name + "_icon.xpm") - if not os.path.exists(path): - with open(path, "w") as f: - f.write(repo.macro.xpm) - default_icon = QtGui.QIcon(repo.macro.xpm) - else: - default_icon = QtGui.QIcon(os.path.join(icon_path, "document-python.svg")) - elif repo.repo_type == Addon.Kind.PACKAGE: - # The cache might not have been downloaded yet, check to see if it's there... - if repo.icon_data: - default_icon = scalable_icon_from_svg_bytes(repo.icon_data) - elif repo.contains_workbench(): - default_icon = QtGui.QIcon(os.path.join(icon_path, "document-package.svg")) - elif repo.contains_macro(): - default_icon = QtGui.QIcon(os.path.join(icon_path, "document-python.svg")) - else: - default_icon = QtGui.QIcon(os.path.join(icon_path, "document-package.svg")) - - if QtCore.QFile.exists(path): - addon_icon = QtGui.QIcon(path) - else: - addon_icon = default_icon - repo.icon = addon_icon - - return addon_icon - - class CommandAddonManager(QtCore.QObject): """The main Addon Manager class and FreeCAD command""" @@ -456,7 +379,7 @@ def on_package_updated(self, repo: Addon) -> None: """Called when the named package has either new metadata or a new icon (or both)""" with self.lock: - repo.icon = get_icon(repo, update=True) + repo.icon = get_icon_for_addon(repo, update=True) self.item_model.reload_item(repo) def select_addon(self) -> None: @@ -593,7 +516,7 @@ def add_addon_repo(self, addon_repo: Addon) -> None: """adds a workbench to the list""" if addon_repo.icon is None or addon_repo.icon.isNull(): - addon_repo.icon = get_icon(addon_repo) + addon_repo.icon = get_icon_for_addon(addon_repo) for repo in self.item_model.repos: if repo.name == addon_repo.name: # self.item_model.reload_item(repo) # If we want to have later additions superseded diff --git a/addonmanager_icon_utilities.py b/addonmanager_icon_utilities.py new file mode 100644 index 00000000..18221cc3 --- /dev/null +++ b/addonmanager_icon_utilities.py @@ -0,0 +1,187 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# *************************************************************************** +# * * +# * Copyright (c) 2025 The FreeCAD project association AISBL * +# * * +# * This file is part of FreeCAD. * +# * * +# * FreeCAD is free software: you can redistribute it and/or modify it * +# * under the terms of the GNU Lesser General Public License as * +# * published by the Free Software Foundation, either version 2.1 of the * +# * License, or (at your option) any later version. * +# * * +# * FreeCAD 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 * +# * Lesser General Public License for more details. * +# * * +# * You should have received a copy of the GNU Lesser General Public * +# * License along with FreeCAD. If not, see * +# * . * +# * * +# *************************************************************************** + +import re +import os +from typing import Optional +import zlib + +from PySideWrapper import QtCore, QtGui, QtSvg + +try: + # If this system provides a secure parser, use that: + import defusedxml.ElementTree as ET +except ImportError: + # Otherwise fall back to the Python standard parser + import xml.etree.ElementTree as ET + +from Addon import Addon +import addonmanager_freecad_interface as fci + +MAX_ICON_BYTES = 10 * 1024 * 1024 + +SVG_ROOT_RE = re.compile( + rb"""^\s*(?:\xEF\xBB\xBF)?(?:|\s|<\?xml[^>]*\?>|]*>)*<\s*svg(?=[\s>])""", + re.IGNORECASE | re.DOTALL | re.VERBOSE, +) + + +def icon_from_bytes(raw: bytes) -> QtGui.QIcon: + """Given raw bytes, try to create the best icon we can. If given SVG (either raw or compressed), + this will result in a QIcon backed by a scalable QIconEngine. Otherwise, it's just a bitmap.""" + if is_svg_bytes(raw): + return scalable_icon_from_svg_bytes(raw) + elif is_gzip(raw): + decompressed = decompress_gzip_limited(raw) + if decompressed is not None: + return scalable_icon_from_svg_bytes(raw) # Qt will handle the compressed data for us + icon = QtGui.QIcon(QtGui.QPixmap.fromImage(QtGui.QImage.fromData(raw))) + if icon.isNull(): + raise BadIconData("Icon data is not in a recognized image file format") + return icon + + +def is_valid_xml(svg_bytes: bytes) -> bool: + """Returns True if the given SVG bytes are at least a valid XML file, False otherwise.""" + try: + _ = ET.fromstring(svg_bytes.decode("utf-8")) + except ET.ParseError: + return False + except UnicodeDecodeError: + return False + except RuntimeError: + return False + return True + + +class BadIconData(Exception): + pass + + +def is_svg_bytes(raw: bytes) -> bool: + head = raw[:MAX_ICON_BYTES] + if SVG_ROOT_RE.search(head): + if is_valid_xml(raw): + return True + raise BadIconData("File header looks like SVG, but data is invalid") + return False + + +def is_gzip(data: bytes) -> bool: + return len(data) >= 2 and data[0] == 0x1F and data[1] == 0x8B + + +def decompress_gzip_limited(data: bytes) -> Optional[bytes]: + """Decompress GZIP safely with a max output cap; return None if it fails or exceeds cap.""" + try: + # wbits=16+MAX_WBITS tells zlib there’s a gzip wrapper; max_length caps output + return zlib.decompress(data, wbits=16 + zlib.MAX_WBITS, bufsize=MAX_ICON_BYTES) + except (zlib.error, TypeError, ValueError): + return None + + +class SvgIconEngine(QtGui.QIconEngine): + def __init__(self, svg_bytes: bytes): + super().__init__() + self.renderer = QtSvg.QSvgRenderer(QtCore.QByteArray(svg_bytes)) + + def paint(self, painter: QtGui.QPainter, rect: QtCore.QRect, mode, state): + self.renderer.render(painter, rect) + + def pixmap(self, size: QtCore.QSize, mode, state): + pixmap = QtGui.QPixmap(size) + pixmap.fill(QtCore.Qt.transparent) + painter = QtGui.QPainter(pixmap) + self.renderer.render(painter) + painter.end() + return pixmap + + +def scalable_icon_from_svg_bytes(svg_bytes: bytes) -> QtGui.QIcon: + engine = SvgIconEngine(svg_bytes) + return QtGui.QIcon(engine) + + +cached_default_icons = {} + + +def get_icon_for_addon(addon: Addon, update: bool = False) -> QtGui.QIcon: + """Returns an icon for an Addon. + :param addon: The addon to get an icon for. + :param update: If True, the icon will be updated even if it already exists. + :return: The QIcon for the addon. Scalable if it was SVG, otherwise a bitmap""" + + icon_path = os.path.join(os.path.dirname(__file__), "Resources", "icons") + + if not update and addon.icon and not addon.icon.isNull(): + return addon.icon + elif addon.icon_data: + if len(addon.icon_data) > MAX_ICON_BYTES: + fci.Console.PrintWarning( + f"WARNING: Icon file for addon '{addon.display_name}' is too large (max size is {MAX_ICON_BYTES} bytes)\n" + ) + try: + addon.icon = icon_from_bytes(addon.icon_data) + return addon.icon + except BadIconData as e: + fci.Console.PrintWarning( + f"Icon file for addon '{addon.display_name}' is invalid:\n{e}\n" + ) + elif addon.macro: + if addon.macro.icon_data: + if len(addon.macro.icon_data) > MAX_ICON_BYTES: + fci.Console.PrintWarning( + f"WARNING: Icon data for macro '{addon.display_name}' is too large (max size is {MAX_ICON_BYTES} bytes)\n" + ) + try: + addon.icon = icon_from_bytes(addon.macro.icon_data) + return addon.icon + except BadIconData as e: + fci.Console.PrintWarning( + f"Icon data for macro '{addon.display_name}' is invalid:\n{e}\n" + ) + elif addon.macro.xpm: + xpm = QtGui.QImage.fromData(addon.macro.xpm.strip().encode("utf-8"), format="XPM") + if xpm.isNull() or xpm.width() == 0 or xpm.height() == 0: + fci.Console.PrintWarning( + f"The XPM icon data for macro '{addon.display_name}' is invalid (please report this to the macro's author, {addon.macro.author})\n" + ) + else: + addon.icon = QtGui.QIcon(QtGui.QPixmap.fromImage(xpm)) + return addon.icon + + if not cached_default_icons: + cached_default_icons["package"] = QtGui.QIcon( + os.path.join(icon_path, "document-package.svg") + ) + cached_default_icons["macro"] = QtGui.QIcon(os.path.join(icon_path, "document-python.svg")) + cached_default_icons["workbench"] = QtGui.QIcon( + os.path.join(icon_path, "document-package.svg") + ) + + if addon.repo_type == Addon.Kind.WORKBENCH: + return cached_default_icons["package"] + if addon.repo_type == Addon.Kind.MACRO: + return cached_default_icons["macro"] + + return cached_default_icons["package"] From 5830d92baf0f413ce8862243e413a76b33c20052 Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Sat, 16 Aug 2025 22:38:11 -0500 Subject: [PATCH 2/3] Add unit tests for icon utilities --- AddonManagerTest/gui/test_icon_utilities.py | 435 ++++++++++++++++++++ addonmanager_icon_utilities.py | 29 +- 2 files changed, 457 insertions(+), 7 deletions(-) create mode 100644 AddonManagerTest/gui/test_icon_utilities.py diff --git a/AddonManagerTest/gui/test_icon_utilities.py b/AddonManagerTest/gui/test_icon_utilities.py new file mode 100644 index 00000000..750f0a49 --- /dev/null +++ b/AddonManagerTest/gui/test_icon_utilities.py @@ -0,0 +1,435 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +import gzip +import io +import unittest +from types import SimpleNamespace +from unittest.mock import patch + +import addonmanager_icon_utilities as iu + +from PySideWrapper import QtCore, QtGui + + +class TestIsValidXML(unittest.TestCase): + """Test the icon utilities. Many of these must be run with a QApplication running because the + functions use some features of QIcon that require it.""" + + def test_is_valid_xml_good_inputs(self): + valid_xml_entries = [ + b"", + b'', + b'', + b'', + b'text', + "Hello & world".encode("utf-8"), + "parsed but kept as text]]>".encode("utf-8"), + ( + "" + "" + "" + "t" + "" + ).encode("utf-8"), + ] + for entry in valid_xml_entries: + with self.subTest(entry=entry): + self.assertTrue(iu.is_valid_xml(entry)) + + def test_is_valid_xml_bad_inputs(self): + invalid_xml_entries = [ + b"", # empty -> ParseError + b"", # unclosed tag -> ParseError + b"", # mismatched tags -> ParseError + b"", # unclosed child -> ParseError + b"<", # truncated -> ParseError + b"not xml at all", # plain text -> ParseError + b"\x00", # NUL char -> ParseError + b"\xff", # invalid UTF-8 -> UnicodeDecodeError + b"\x80abc", # invalid UTF-8 lead byte -> UnicodeDecodeError + b"\xfe\xff<\x00s\x00v\x00g\x00/\x00>\x00", # UTF-16LE/BE bytes, not UTF-8 -> UnicodeDecodeError + b'\xa0', # claims latin-1 but decoded as UTF-8 -> UnicodeDecodeError + ] + for entry in invalid_xml_entries: + with self.subTest(entry=entry): + self.assertFalse(iu.is_valid_xml(entry)) + + +class TestIsSvgBytes(unittest.TestCase): + + def test_is_svg_bytes_detects_valid_svg(self): + raw = b'' + with patch("addonmanager_icon_utilities.is_valid_xml", return_value=True) as m: + self.assertTrue(iu.is_svg_bytes(raw)) + m.assert_called_once_with(raw) # full bytes passed through + + def test_is_svg_bytes_raises_on_svgish_but_invalid(self): + raw = b'' + with patch("addonmanager_icon_utilities.is_valid_xml", return_value=False): + with self.assertRaises(iu.BadIconData): + iu.is_svg_bytes(raw) + + def test_is_svg_bytes_returns_false_for_non_svg_header(self): + raw = b"\x89PNG\r\n\x1a\n\x00\x00\x00IHDR" # clearly not SVG header + with patch("addonmanager_icon_utilities.is_valid_xml") as m: + self.assertFalse(iu.is_svg_bytes(raw)) + m.assert_not_called() # no XML parse if header doesn't match + + def test_is_svg_bytes_sniffs_only_first_MAX_ICON_BYTES(self): + # Place "" + with patch("addonmanager_icon_utilities.MAX_ICON_BYTES", 64), patch( + "addonmanager_icon_utilities.is_valid_xml" + ) as m: + self.assertFalse(iu.is_svg_bytes(raw)) + m.assert_not_called() + + def test_is_svg_bytes_calls_is_valid_xml_with_full_raw_not_head(self): + # Ensure the function validates the full payload, not just the header slice. + raw = ( + b'' + b'' + b"A" * 500 + b"" + ) + calls = [] + + def fake_is_valid_xml(arg): + calls.append(arg) + return True + + with patch( + "addonmanager_icon_utilities.is_valid_xml", side_effect=fake_is_valid_xml + ), patch("addonmanager_icon_utilities.MAX_ICON_BYTES", 32): + self.assertTrue(iu.is_svg_bytes(raw)) + self.assertIs(calls[0], raw) + self.assertEqual(len(calls[0]), len(raw)) + + +class TestIsGzip(unittest.TestCase): + + def test_is_gzip_true_minimal(self): + self.assertTrue(iu.is_gzip(b"\x1f\x8b")) + + def test_is_gzip_true_with_extra_bytes(self): + # Typical gzip header starts with 0x1f 0x8b 0x08 ... + self.assertTrue(iu.is_gzip(b"\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\x03extra")) + + def test_is_gzip_false_empty(self): + self.assertFalse(iu.is_gzip(b"")) + + def test_is_gzip_false_one_byte(self): + self.assertFalse(iu.is_gzip(b"\x1f")) + + def test_is_gzip_false_wrong_first_byte(self): + self.assertFalse(iu.is_gzip(b"\x00\x8b")) + + def test_is_gzip_false_wrong_second_byte(self): + self.assertFalse(iu.is_gzip(b"\x1f\x00")) + + def test_is_gzip_false_swapped_bytes(self): + self.assertFalse(iu.is_gzip(b"\x8b\x1f")) + + def test_is_gzip_false_zlib_header(self): + # Common zlib/deflate header 0x78 0x9C should not be mistaken for gzip + self.assertFalse(iu.is_gzip(b"\x78\x9c\x00\x00")) + + +def gz(payload: bytes) -> bytes: + bio = io.BytesIO() + with gzip.GzipFile(fileobj=bio, mode="wb") as f: + f.write(payload) + return bio.getvalue() + + +class TestDecompressGzipLimited(unittest.TestCase): + def test_small_valid_within_ratio(self): + raw = b"hello world" + data = gz(raw) + with patch("addonmanager_icon_utilities.MAX_ICON_BYTES", len(data)), patch( + "addonmanager_icon_utilities.MAX_GZIP_EXPANSION_RATIO", 16 + ), patch("addonmanager_icon_utilities.MAX_GZIP_OUTPUT_ABS", 512 * 1024): + self.assertEqual(iu.decompress_gzip_limited(data), raw) + + def test_respects_input_cap_inclusive(self): + raw = b"a" * 64 + data = gz(raw) + with patch("addonmanager_icon_utilities.MAX_ICON_BYTES", len(data)): + self.assertEqual(iu.decompress_gzip_limited(data), raw) + + def test_rejects_when_compressed_exceeds_cap(self): + raw = b"a" * 64 + data = gz(raw) + with patch("addonmanager_icon_utilities.MAX_ICON_BYTES", len(data) - 1): + self.assertIsNone(iu.decompress_gzip_limited(data)) + + def test_overflows_ratio_returns_none(self): + # Choose caps so ratio*len(data) < len(uncompressed) + payload = b"x" * 129 + data = gz(payload) + with patch("addonmanager_icon_utilities.MAX_ICON_BYTES", len(data)), patch( + "addonmanager_icon_utilities.MAX_GZIP_EXPANSION_RATIO", 1 + ), patch( + "addonmanager_icon_utilities.MAX_GZIP_OUTPUT_ABS", 10**9 + ): # let ratio drive the bound + self.assertIsNone(iu.decompress_gzip_limited(data)) + + def test_overflows_absolute_cap_returns_none(self): + payload = b"x" * 600_000 # >512 KiB default abs cap + data = gz(payload) + with patch("addonmanager_icon_utilities.MAX_ICON_BYTES", len(data)), patch( + "addonmanager_icon_utilities.MAX_GZIP_EXPANSION_RATIO", 1000 + ), patch("addonmanager_icon_utilities.MAX_GZIP_OUTPUT_ABS", 512 * 1024): + self.assertIsNone(iu.decompress_gzip_limited(data)) + + def test_truncated_or_non_gzip_returns_none(self): + payload = b"abcdef" + data = gz(payload) + truncated = data[:-2] + with patch("addonmanager_icon_utilities.MAX_ICON_BYTES", len(data)): + self.assertIsNone(iu.decompress_gzip_limited(truncated)) + self.assertIsNone(iu.decompress_gzip_limited(b"not-gzip")) + + def test_type_error_returns_none(self): + with patch("addonmanager_icon_utilities.MAX_ICON_BYTES", 100): + self.assertIsNone(iu.decompress_gzip_limited(None)) # type: ignore[arg-type] + + +# Solid “fills-everything” SVG: easy to assert non-transparent pixels. +SOLID_SVG = ( + b'' + b'' +) + +# Empty SVG: renders nothing; pixmap should remain fully transparent. +EMPTY_SVG = b'' + + +def alpha_at(pm: QtGui.QPixmap, x: int, y: int) -> int: + img = pm.toImage() + return img.pixelColor(x, y).alpha() + + +class TestSvgIconEngine(unittest.TestCase): + def test_pixmap_size_and_opaque_content(self): + eng = iu.SvgIconEngine(SOLID_SVG) + size = QtCore.QSize(32, 32) + pm = eng.pixmap(size, QtGui.QIcon.Normal, QtGui.QIcon.Off) # type: ignore[arg-type] + + self.assertEqual(pm.size(), size) + self.assertTrue(pm.hasAlphaChannel()) + self.assertGreater(alpha_at(pm, 16, 16), 0) # center is painted + + def test_pixmap_starts_transparent_when_svg_empty(self): + eng = iu.SvgIconEngine(EMPTY_SVG) + pm = eng.pixmap(QtCore.QSize(20, 20), QtGui.QIcon.Normal, QtGui.QIcon.Off) # type: ignore[arg-type] + + # Spot-check a few pixels; all should be transparent + self.assertEqual(alpha_at(pm, 10, 10), 0) + self.assertEqual(alpha_at(pm, 0, 0), 0) + self.assertEqual(alpha_at(pm, 19, 19), 0) + + def test_paint_respects_rect(self): + eng = iu.SvgIconEngine(SOLID_SVG) + pm = QtGui.QPixmap(40, 40) + pm.fill(QtCore.Qt.transparent) # type: ignore[arg-type] + + painter = QtGui.QPainter(pm) + rect = QtCore.QRect(10, 10, 20, 20) # paint into the middle 20x20 square + eng.paint(painter, rect, QtGui.QIcon.Normal, QtGui.QIcon.Off) # type: ignore[arg-type] + painter.end() + + # Outside the rect stays transparent + self.assertEqual(alpha_at(pm, 5, 5), 0) + self.assertEqual(alpha_at(pm, 35, 35), 0) + + # Inside the rect is painted (non-transparent) + self.assertGreater(alpha_at(pm, 20, 20), 0) + + def test_renderer_is_valid(self): + eng = iu.SvgIconEngine(SOLID_SVG) + self.assertTrue(eng.renderer.isValid()) + + +def _qpix_icon(): + pm = QtGui.QPixmap(2, 2) + pm.fill(QtCore.Qt.white) # type: ignore[arg-type] + return QtGui.QIcon(pm) + + +def _valid_xpm_bytes(): + img = QtGui.QImage(2, 2, QtGui.QImage.Format_ARGB32) # type: ignore[arg-type] + img.fill(QtCore.Qt.black) # type: ignore[arg-type] + buf = QtCore.QBuffer() + buf.open(QtCore.QIODevice.WriteOnly) # type: ignore[arg-type] + img.save(buf, "XPM") + return bytes(buf.data()) # type: ignore[arg-type] + + +class TestGetIconForAddon(unittest.TestCase): + def setUp(self): + # Keep tests deterministic & small + self.maxbytes = patch("addonmanager_icon_utilities.MAX_ICON_BYTES", 16).start() + self.addCleanup(patch.stopall) + + # Reset cache every test + iu.cached_default_icons.clear() + + # Common mocks + self.warn = patch( + "addonmanager_icon_utilities.fci.Console.PrintWarning", autospec=True + ).start() + self.icon_from = patch("addonmanager_icon_utilities.icon_from_bytes", autospec=True).start() + + # Handy enum values + self.KIND_MACRO = iu.Addon.Kind.MACRO + self.KIND_WORKBENCH = iu.Addon.Kind.WORKBENCH + + # --- Early return / update flag --- + + def test_returns_existing_icon_when_not_updating(self): + existing = _qpix_icon() + addon = SimpleNamespace( + icon=existing, + icon_data=None, + macro=None, + repo_type=self.KIND_WORKBENCH, + display_name="A", + ) + out = iu.get_icon_for_addon(addon, update=False) # type: ignore[arg-type] + self.assertIs(out, existing) + self.icon_from.assert_not_called() + self.warn.assert_not_called() + + def test_update_true_ignores_existing_and_uses_icon_data(self): + existing = _qpix_icon() + new_icon = _qpix_icon() + self.icon_from.return_value = new_icon + data = b"x" * 8 + addon = SimpleNamespace( + icon=existing, + icon_data=data, + macro=None, + repo_type=self.KIND_WORKBENCH, + display_name="A", + ) + out = iu.get_icon_for_addon(addon, update=True) # type: ignore[arg-type] + self.assertIs(out, new_icon) + self.assertIs(addon.icon, new_icon) + self.icon_from.assert_called_once_with(data) + self.warn.assert_not_called() + + def test_addon_icon_data_under_limit_success(self): + data = b"x" * 8 + result_icon = _qpix_icon() + self.icon_from.return_value = result_icon + + addon = SimpleNamespace( + icon=None, icon_data=data, macro=None, repo_type=self.KIND_WORKBENCH, display_name="Pkg" + ) + out = iu.get_icon_for_addon(addon) # type: ignore[arg-type] + self.assertIs(out, result_icon) + self.assertIs(addon.icon, result_icon) + self.warn.assert_not_called() + + def test_addon_icon_data_over_limit_warns_then_uses_icon(self): + data = b"x" * 20 # > MAX_ICON_BYTES (patched to 16) + result_icon = _qpix_icon() + self.icon_from.return_value = result_icon + + addon = SimpleNamespace( + icon=None, icon_data=data, macro=None, repo_type=self.KIND_WORKBENCH, display_name="Pkg" + ) + out = iu.get_icon_for_addon(addon) # type: ignore[arg-type] + self.assertIs(out, result_icon) + self.warn.assert_called_once() + self.assertIn("too large", self.warn.call_args[0][0].lower()) + + def test_addon_icon_data_invalid_warns_and_falls_to_default(self): + self.icon_from.side_effect = iu.BadIconData("nope") + data = b"x" * 8 + addon = SimpleNamespace( + icon=None, icon_data=data, macro=None, repo_type=self.KIND_WORKBENCH, display_name="Pkg" + ) + + out = iu.get_icon_for_addon(addon) # type: ignore[arg-type] + # Default icons initialized and workbench → "package" + self.assertIn("package", iu.cached_default_icons) + self.assertIs(out, iu.cached_default_icons["package"]) + self.warn.assert_called_once() + self.assertIn("invalid", self.warn.call_args[0][0].lower()) + + def test_macro_icon_data_under_limit_success(self): + self.icon_from.return_value = _qpix_icon() + macro = SimpleNamespace(icon_data=b"x" * 8, xpm=None, author="Auth") + addon = SimpleNamespace( + icon=None, icon_data=None, macro=macro, repo_type=self.KIND_MACRO, display_name="Macro" + ) + out = iu.get_icon_for_addon(addon) # type: ignore[arg-type] + self.assertIs(out, addon.icon) + self.icon_from.assert_called_once_with(macro.icon_data) + self.warn.assert_not_called() + + def test_macro_icon_data_over_limit_warns_then_uses_icon(self): + self.icon_from.return_value = _qpix_icon() + macro = SimpleNamespace(icon_data=b"x" * 64, xpm=None, author="Auth") + addon = SimpleNamespace( + icon=None, icon_data=None, macro=macro, repo_type=self.KIND_MACRO, display_name="Macro" + ) + out = iu.get_icon_for_addon(addon) # type: ignore[arg-type] + self.assertIs(out, addon.icon) + self.warn.assert_called_once() + self.assertIn("too large", self.warn.call_args[0][0].lower()) + + def test_macro_icon_data_invalid_warns_then_falls_through(self): + self.icon_from.side_effect = iu.BadIconData("boom") + macro = SimpleNamespace(icon_data=b"x" * 8, xpm=None, author="Auth") + addon = SimpleNamespace( + icon=None, icon_data=None, macro=macro, repo_type=self.KIND_MACRO, display_name="Macro" + ) + out = iu.get_icon_for_addon(addon) # type: ignore[arg-type] + # With invalid macro icon_data and no XPM, we fall to defaults (macro kind) + self.assertIn("macro", iu.cached_default_icons) + self.assertIs(out, iu.cached_default_icons["macro"]) + self.warn.assert_called_once() + self.assertIn("invalid", self.warn.call_args[0][0].lower()) + + def test_macro_xpm_invalid_warns_and_fallback(self): + macro = SimpleNamespace(icon_data=None, xpm="not-xpm", author="Auth") + addon = SimpleNamespace( + icon=None, icon_data=None, macro=macro, repo_type=self.KIND_MACRO, display_name="Macro" + ) + out = iu.get_icon_for_addon(addon) # type: ignore[arg-type] + self.assertIs(out, iu.cached_default_icons["macro"]) + self.assertTrue(self.warn.called) + self.assertIn("invalid", self.warn.call_args[0][0].lower()) + + def test_macro_xpm_valid_returns_icon(self): + macro = SimpleNamespace( + icon_data=None, xpm=_valid_xpm_bytes().decode("utf-8"), author="Auth" + ) + addon = SimpleNamespace( + icon=None, icon_data=None, macro=macro, repo_type=self.KIND_MACRO, display_name="Macro" + ) + out = iu.get_icon_for_addon(addon) # type: ignore[arg-type] + self.assertIsInstance(out, QtGui.QIcon) + self.assertFalse(out.isNull()) + + def test_defaults_initialized_once_and_selected_by_repo_type(self): + # First call: init cache and return "package" for WORKBENCH + addon1 = SimpleNamespace( + icon=None, icon_data=None, macro=None, repo_type=self.KIND_WORKBENCH, display_name="WB" + ) + a1 = iu.get_icon_for_addon(addon1) # type: ignore[arg-type] + self.assertIs(a1, iu.cached_default_icons["package"]) + + # Second call: same cache, MACRO → "macro" + addon2 = SimpleNamespace( + icon=None, icon_data=None, macro=None, repo_type=self.KIND_MACRO, display_name="M" + ) + a2 = iu.get_icon_for_addon(addon2) # type: ignore[arg-type] + self.assertIs(a2, iu.cached_default_icons["macro"]) + + # Third: unknown kind → "package" + addon3 = SimpleNamespace( + icon=None, icon_data=None, macro=None, repo_type=None, display_name="Other" + ) + a3 = iu.get_icon_for_addon(addon3) # type: ignore[arg-type] + self.assertIs(a3, iu.cached_default_icons["package"]) diff --git a/addonmanager_icon_utilities.py b/addonmanager_icon_utilities.py index 18221cc3..9ce5aaae 100644 --- a/addonmanager_icon_utilities.py +++ b/addonmanager_icon_utilities.py @@ -24,7 +24,6 @@ import re import os from typing import Optional -import zlib from PySideWrapper import QtCore, QtGui, QtSvg @@ -91,12 +90,28 @@ def is_gzip(data: bytes) -> bool: return len(data) >= 2 and data[0] == 0x1F and data[1] == 0x8B +MAX_GZIP_EXPANSION_RATIO = 16 +MAX_GZIP_OUTPUT_ABS = 512 * 1024 # 512 KiB + + def decompress_gzip_limited(data: bytes) -> Optional[bytes]: - """Decompress GZIP safely with a max output cap; return None if it fails or exceeds cap.""" + """Allow compressed size ≤ MAX_ICON_BYTES; read at most a small, bounded amount. + Returns None on failure or if output would excede the bound.""" + if not isinstance(data, (bytes, bytearray, memoryview)): + return None + if len(data) > MAX_ICON_BYTES: + return None + + import io, gzip, zlib + + max_out = min(MAX_GZIP_OUTPUT_ABS, MAX_GZIP_EXPANSION_RATIO * len(data)) try: - # wbits=16+MAX_WBITS tells zlib there’s a gzip wrapper; max_length caps output - return zlib.decompress(data, wbits=16 + zlib.MAX_WBITS, bufsize=MAX_ICON_BYTES) - except (zlib.error, TypeError, ValueError): + with gzip.GzipFile(fileobj=io.BytesIO(data)) as f: + out = f.read(max_out + 1) # stream, don’t inflate unbounded + if len(out) > max_out: + return None + return out + except (OSError, EOFError, zlib.error, ValueError, TypeError): return None @@ -110,7 +125,7 @@ def paint(self, painter: QtGui.QPainter, rect: QtCore.QRect, mode, state): def pixmap(self, size: QtCore.QSize, mode, state): pixmap = QtGui.QPixmap(size) - pixmap.fill(QtCore.Qt.transparent) + pixmap.fill(QtCore.Qt.transparent) # type: ignore[arg-type] painter = QtGui.QPainter(pixmap) self.renderer.render(painter) painter.end() @@ -161,7 +176,7 @@ def get_icon_for_addon(addon: Addon, update: bool = False) -> QtGui.QIcon: f"Icon data for macro '{addon.display_name}' is invalid:\n{e}\n" ) elif addon.macro.xpm: - xpm = QtGui.QImage.fromData(addon.macro.xpm.strip().encode("utf-8"), format="XPM") + xpm = QtGui.QImage.fromData(addon.macro.xpm.strip().encode("utf-8"), format="XPM") # type: ignore[arg-type] if xpm.isNull() or xpm.width() == 0 or xpm.height() == 0: fci.Console.PrintWarning( f"The XPM icon data for macro '{addon.display_name}' is invalid (please report this to the macro's author, {addon.macro.author})\n" From 11c414567b21b254003d46013da30e19d60bdb4e Mon Sep 17 00:00:00 2001 From: Chris Hennes Date: Sat, 16 Aug 2025 22:42:26 -0500 Subject: [PATCH 3/3] Cleanup from review --- CMakeLists.txt | 1 + addonmanager_icon_utilities.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6461d2fb..1237948d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -16,6 +16,7 @@ SET(AddonManager_SRCS addonmanager_firstrun.py addonmanager_freecad_interface.py addonmanager_git.py + addonmanager_icon_utilities.py addonmanager_installation_manifest.py addonmanager_installer_gui.py addonmanager_installer.py diff --git a/addonmanager_icon_utilities.py b/addonmanager_icon_utilities.py index 9ce5aaae..201cad27 100644 --- a/addonmanager_icon_utilities.py +++ b/addonmanager_icon_utilities.py @@ -96,7 +96,7 @@ def is_gzip(data: bytes) -> bool: def decompress_gzip_limited(data: bytes) -> Optional[bytes]: """Allow compressed size ≤ MAX_ICON_BYTES; read at most a small, bounded amount. - Returns None on failure or if output would excede the bound.""" + Returns None on failure or if output would exceed the bound.""" if not isinstance(data, (bytes, bytearray, memoryview)): return None if len(data) > MAX_ICON_BYTES: