From 722644b111b1fc858aee12bbbbb4f7600b57f563 Mon Sep 17 00:00:00 2001 From: dgtlmoon Date: Sat, 11 Apr 2026 15:35:27 +1000 Subject: [PATCH] Handle inline favicons (#4047 #3891 ) --- .../content_fetchers/res/favicon-fetcher.js | 23 +++++- changedetectionio/model/Watch.py | 77 ++++++++++++------ changedetectionio/tests/test_security.py | 79 +++++++++++++++++++ changedetectionio/worker.py | 3 +- 4 files changed, 154 insertions(+), 28 deletions(-) diff --git a/changedetectionio/content_fetchers/res/favicon-fetcher.js b/changedetectionio/content_fetchers/res/favicon-fetcher.js index 3f7a0ad84..afd564dbc 100644 --- a/changedetectionio/content_fetchers/res/favicon-fetcher.js +++ b/changedetectionio/content_fetchers/res/favicon-fetcher.js @@ -38,26 +38,39 @@ if (a.size !== b.size) { return b.size - a.size; } - + // Second priority: apple-touch-icon over regular icon const isAppleA = /apple-touch-icon/.test(a.rel); const isAppleB = /apple-touch-icon/.test(b.rel); if (isAppleA && !isAppleB) return -1; if (!isAppleA && isAppleB) return 1; - + // Third priority: icons with no size attribute (fallback icons) last const hasNoSizeA = !a.hasSizes; const hasNoSizeB = !b.hasSizes; if (hasNoSizeA && !hasNoSizeB) return 1; if (!hasNoSizeA && hasNoSizeB) return -1; - + return 0; }); const timeoutMs = 2000; + // 1 MB — matches the server-side limit in bump_favicon() + const MAX_BYTES = 1 * 1024 * 1024; for (const icon of icons) { try { + // Inline data URI — no network fetch needed, data is already here + if (icon.href.startsWith('data:')) { + const match = icon.href.match(/^data:([^;]+);base64,([A-Za-z0-9+/=]+)$/); + if (!match) continue; + const mime_type = match[1]; + const base64 = match[2]; + // Rough size check: base64 is ~4/3 the binary size + if (base64.length * 0.75 > MAX_BYTES) continue; + return { url: icon.href, mime_type, base64 }; + } + const controller = new AbortController(); const timeout = setTimeout(() => controller.abort(), timeoutMs); @@ -74,12 +87,15 @@ const blob = await resp.blob(); + if (blob.size > MAX_BYTES) continue; + // Convert blob to base64 const reader = new FileReader(); return await new Promise(resolve => { reader.onloadend = () => { resolve({ url: icon.href, + mime_type: blob.type, base64: reader.result.split(",")[1] }); }; @@ -98,4 +114,3 @@ // Auto-execute and return result for page.evaluate() return await window.getFaviconAsBlob(); })(); - diff --git a/changedetectionio/model/Watch.py b/changedetectionio/model/Watch.py index 4b09415d7..6f0a9a955 100644 --- a/changedetectionio/model/Watch.py +++ b/changedetectionio/model/Watch.py @@ -798,24 +798,50 @@ class model(EntityPersistenceMixin, watch_base): # Also in the case that the file didnt exist return True - def bump_favicon(self, url, favicon_base_64: str) -> None: + def bump_favicon(self, url, favicon_base_64: str, mime_type: str = None) -> None: from urllib.parse import urlparse import base64 import binascii - decoded = None + import re - if url: + MAX_FAVICON_BYTES = 1 * 1024 * 1024 # 1 MB + + MIME_TO_EXT = { + 'image/png': 'png', + 'image/x-icon': 'ico', + 'image/vnd.microsoft.icon': 'ico', + 'image/jpeg': 'jpg', + 'image/gif': 'gif', + 'image/svg+xml': 'svg', + 'image/webp': 'webp', + 'image/bmp': 'bmp', + } + + extension = None + + # If the caller already resolved the MIME type (e.g. from blob.type or a data URI), + # use that directly — it's more reliable than guessing from a URL path. + if mime_type: + extension = MIME_TO_EXT.get(mime_type.lower().split(';')[0].strip(), None) + + # Fall back to extracting extension from URL path, unless it's a data URI. + if not extension and url and not url.startswith('data:'): try: parsed = urlparse(url) filename = os.path.basename(parsed.path) - (base, extension) = filename.lower().strip().rsplit('.', 1) + (_base, ext) = filename.lower().strip().rsplit('.', 1) + extension = ext except ValueError: - logger.error(f"UUID: {self.get('uuid')} Cant work out file extension from '{url}'") - return None - else: - # Assume favicon.ico - base = "favicon" - extension = "ico" + logger.warning(f"UUID: {self.get('uuid')} Cant work out file extension from '{url}', defaulting to ico") + + # Handle data URIs: extract MIME type from the URI itself when not already known + if not extension and url and url.startswith('data:'): + m = re.match(r'^data:([^;]+);base64,', url) + if m: + extension = MIME_TO_EXT.get(m.group(1).lower(), None) + + if not extension: + extension = 'ico' fname = os.path.join(self.data_dir, f"favicon.{extension}") @@ -824,22 +850,27 @@ class model(EntityPersistenceMixin, watch_base): decoded = base64.b64decode(favicon_base_64, validate=True) except (binascii.Error, ValueError) as e: logger.warning(f"UUID: {self.get('uuid')} FavIcon save data (Base64) corrupt? {str(e)}") - else: - if decoded: - try: - with open(fname, 'wb') as f: - f.write(decoded) + return None - # Invalidate module-level favicon filename cache for this watch - _FAVICON_FILENAME_CACHE.pop(self.data_dir, None) + if len(decoded) > MAX_FAVICON_BYTES: + logger.warning(f"UUID: {self.get('uuid')} Favicon too large ({len(decoded)} bytes), skipping") + return None - # A signal that could trigger the socket server to update the browser also - watch_check_update = signal('watch_favicon_bump') - if watch_check_update: - watch_check_update.send(watch_uuid=self.get('uuid')) + try: + with open(fname, 'wb') as f: + f.write(decoded) - except Exception as e: - logger.warning(f"UUID: {self.get('uuid')} error saving FavIcon to {fname} - {str(e)}") + # Invalidate module-level favicon filename cache for this watch + _FAVICON_FILENAME_CACHE.pop(self.data_dir, None) + + # A signal that could trigger the socket server to update the browser also + watch_check_update = signal('watch_favicon_bump') + if watch_check_update: + watch_check_update.send(watch_uuid=self.get('uuid')) + + except Exception as e: + logger.warning(f"UUID: {self.get('uuid')} error saving FavIcon to {fname} - {str(e)}") + return None # @todo - Store some checksum and only write when its different logger.debug(f"UUID: {self.get('uuid')} updated favicon to at {fname}") diff --git a/changedetectionio/tests/test_security.py b/changedetectionio/tests/test_security.py index 04eb1236f..076ec2ccb 100644 --- a/changedetectionio/tests/test_security.py +++ b/changedetectionio/tests/test_security.py @@ -50,6 +50,85 @@ def test_favicon(client, live_server, measure_memory_usage, datastore_path): res = client.get(url_for('static_content', group='js', filename='../styles/styles.css')) assert res.status_code != 200 +def test_favicon_inline_data_uri(client, live_server, measure_memory_usage, datastore_path): + """ + bump_favicon() must handle a data URI as the url parameter. + Previously this logged "Cant work out file extension from 'data:image/png;base64,...'" and bailed. + The mime_type from the data URI should be used to pick the correct extension. + """ + import base64 + import os + + # 1x1 transparent PNG (minimal valid PNG bytes) + PNG_BYTES = ( + b'\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01' + b'\x08\x06\x00\x00\x00\x1f\x15\xc4\x89\x00\x00\x00\nIDATx\x9cc\x00\x01' + b'\x00\x00\x05\x00\x01\r\n-\xb4\x00\x00\x00\x00IEND\xaeB`\x82' + ) + png_b64 = base64.b64encode(PNG_BYTES).decode() + data_uri = f"data:image/png;base64,{png_b64}" + + uuid = client.application.config.get('DATASTORE').add_watch(url='https://localhost') + watch = live_server.app.config['DATASTORE'].data['watching'][uuid] + + # Should NOT raise / bail — must save as favicon.png + watch.bump_favicon(url=data_uri, favicon_base_64=png_b64, mime_type='image/png') + + favicon_fname = watch.get_favicon_filename() + assert favicon_fname is not None, "Favicon should have been saved" + assert favicon_fname.endswith('.png'), f"Expected .png extension, got: {favicon_fname}" + + full_path = os.path.join(watch.data_dir, favicon_fname) + assert os.path.getsize(full_path) == len(PNG_BYTES) + + # Also verify it's served correctly via the static route + res = client.get(url_for('static_content', group='favicon', filename=uuid)) + assert res.status_code == 200 + assert res.data == PNG_BYTES + + +def test_favicon_mime_type_overrides_url_extension(client, live_server, measure_memory_usage, datastore_path): + """ + mime_type parameter takes precedence over the URL path extension. + A URL ending in .ico but with mime_type='image/png' should save as .png. + """ + import base64 + import os + + PNG_BYTES = ( + b'\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01' + b'\x08\x06\x00\x00\x00\x1f\x15\xc4\x89\x00\x00\x00\nIDATx\x9cc\x00\x01' + b'\x00\x00\x05\x00\x01\r\n-\xb4\x00\x00\x00\x00IEND\xaeB`\x82' + ) + png_b64 = base64.b64encode(PNG_BYTES).decode() + + uuid = client.application.config.get('DATASTORE').add_watch(url='https://localhost') + watch = live_server.app.config['DATASTORE'].data['watching'][uuid] + + watch.bump_favicon(url='https://example.com/favicon.ico', favicon_base_64=png_b64, mime_type='image/png') + + favicon_fname = watch.get_favicon_filename() + assert favicon_fname is not None + assert favicon_fname.endswith('.png'), f"mime_type should override URL extension, got: {favicon_fname}" + + +def test_favicon_oversized_rejected(client, live_server, measure_memory_usage, datastore_path): + """Favicons larger than 1 MB must be silently dropped.""" + import base64 + import os + + oversized = b'\x00' * (1 * 1024 * 1024 + 1) + oversized_b64 = base64.b64encode(oversized).decode() + + uuid = client.application.config.get('DATASTORE').add_watch(url='https://localhost') + watch = live_server.app.config['DATASTORE'].data['watching'][uuid] + + result = watch.bump_favicon(url='https://example.com/big.png', favicon_base_64=oversized_b64, mime_type='image/png') + + assert result is None, "bump_favicon should return None for oversized favicon" + assert watch.get_favicon_filename() is None, "No favicon file should have been written" + + def test_bad_access(client, live_server, measure_memory_usage, datastore_path): res = client.post( diff --git a/changedetectionio/worker.py b/changedetectionio/worker.py index 41bcd29cb..074291ec8 100644 --- a/changedetectionio/worker.py +++ b/changedetectionio/worker.py @@ -484,7 +484,8 @@ async def async_update_worker(worker_id, q, notification_q, app, datastore, exec # Store favicon if necessary if update_handler.fetcher.favicon_blob and update_handler.fetcher.favicon_blob.get('base64'): watch.bump_favicon(url=update_handler.fetcher.favicon_blob.get('url'), - favicon_base_64=update_handler.fetcher.favicon_blob.get('base64') + favicon_base_64=update_handler.fetcher.favicon_blob.get('base64'), + mime_type=update_handler.fetcher.favicon_blob.get('mime_type') ) datastore.update_watch(uuid=uuid, update_obj=final_updates)