From 6d9f3fc8fb2ef7cdb0529973be96c8a8f4dd9a57 Mon Sep 17 00:00:00 2001 From: Vito Date: Fri, 16 Oct 2026 05:35:03 -0700 Subject: [PATCH] fix: preserve gallery image identity and ignore stale metadata (#175) * fix: preserve gallery root identity across media and metadata Fixes #173 * fix: ignore stale gallery metadata responses Fixes #174 * test: use platform-valid gallery filenames on Windows --- database/operations.py | 7 +- py/api/images.py | 26 +++++-- tests/js/gallery-metadata.test.js | 76 ++++++++++++++++++++ tests/js/gallery-root-metadata.test.js | 30 ++++++++ tests/js/image-helpers.test.js | 13 ++++ tests/test_api_images.py | 99 ++++++++++++++++++++++++-- tools/vendor/tests/viewer.spec.mjs | 25 +++++++ web/js/gallery.js | 20 ++++-- web/js/image-helpers.js | 18 ++++- 9 files changed, 295 insertions(+), 19 deletions(-) create mode 100644 tests/js/gallery-metadata.test.js create mode 100644 tests/js/gallery-root-metadata.test.js diff --git a/database/operations.py b/database/operations.py index af53cb3..e037826 100644 --- a/database/operations.py +++ b/database/operations.py @@ -1967,7 +1967,9 @@ class PromptDatabase: conn.commit() return count - def get_image_prompt_info(self, image_path: str) -> Optional[Dict[str, Any]]: + def get_image_prompt_info( + self, image_path: str, allow_filename_fallback: bool = True + ) -> Optional[Dict[str, Any]]: """Look up prompt + image metadata for a given image path. Returns a dict with prompt_id, text, category, tags, rating, notes, @@ -1981,10 +1983,11 @@ class PromptDatabase: FROM generated_images gi JOIN prompts p ON gi.prompt_id = p.id WHERE gi.image_path = ? OR gi.file_path = ? - OR gi.image_path LIKE ? ESCAPE '\\'""", + OR (? AND gi.image_path LIKE ? ESCAPE '\\')""", ( image_path, normalize_image_path(image_path), + allow_filename_fallback, f"%{escape_like(os.path.basename(image_path))}", ), ) diff --git a/py/api/images.py b/py/api/images.py index 2cbdb0c..18cb296 100644 --- a/py/api/images.py +++ b/py/api/images.py @@ -316,6 +316,9 @@ def _output_image_entry(media_path, output_path, root_index, public_root=None): f"{'.jpg' if is_video else extension}" ) thumbnail_url = thumbnail_url_for(thumb_rel, output_path / thumb_rel) + if thumbnail_url is not None: + thumbnail_url += f"&root={root_index}" + encoded_path = urllib.parse.quote(rel_path.as_posix(), safe="/") return { "id": hashlib.sha1(str(media_path).encode("utf-8")).hexdigest()[:16], @@ -324,7 +327,7 @@ def _output_image_entry(media_path, output_path, root_index, public_root=None): "relative_path": str(rel_path), "root_dir": public_root, "root_index": root_index, - "url": f"/prompt_manager/images/serve/{rel_path.as_posix()}", + "url": f"/prompt_manager/images/serve/{encoded_path}?root={root_index}", "thumbnail_url": thumbnail_url, "size": stat.st_size, "modified_time": stat.st_mtime, @@ -703,10 +706,10 @@ class ImageRoutesMixin: try: idx = int(root_param) except ValueError: - return output_dirs + return [] if 0 <= idx < len(output_dirs): return [output_dirs[idx]] - return output_dirs + return [] async def serve_output_image(self, request): """Serve a media file from the ComfyUI output folder(s).""" @@ -1434,7 +1437,7 @@ class ImageRoutesMixin: try: # Get the image path from URL raw_image_path = request.match_info.get("image_path", "") - image_path = urllib.parse.unquote(raw_image_path) + image_path = raw_image_path if not image_path: return web.json_response( @@ -1442,15 +1445,26 @@ class ImageRoutesMixin: ) # Convert relative path to absolute if needed - if not os.path.isabs(image_path): + if "root" in request.query: + roots = self._select_root( + self._get_all_output_dirs(), request.query["root"] + ) + if not roots: + return _forbidden() + if not os.path.isabs(image_path): + image_path = str(Path(roots[0]) / image_path) + if not _is_within(image_path, roots[0]): + return _forbidden() + elif not os.path.isabs(image_path): output_dir = self._find_comfyui_output_dir() if output_dir: image_path = str(Path(output_dir) / image_path) # Look up the image in generated_images table try: + args = (image_path, False) if "root" in request.query else (image_path,) prompt_data = await self._run_in_executor( - self.db.get_image_prompt_info, image_path + self.db.get_image_prompt_info, *args ) if prompt_data: prompt_data["image_path"] = _public_path(image_path) diff --git a/tests/js/gallery-metadata.test.js b/tests/js/gallery-metadata.test.js new file mode 100644 index 0000000..3d4f16e --- /dev/null +++ b/tests/js/gallery-metadata.test.js @@ -0,0 +1,76 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const vm = require('node:vm'); +const path = require('node:path'); + +function deferred() { + let resolve, reject; + const promise = new Promise((yes, no) => { resolve = yes; reject = no; }); + return { promise, resolve, reject }; +} + +function fixture() { + const context = vm.createContext({ document: { addEventListener() {}, getElementById() {} }, + window: {}, console: { error() {} }, escapeHtml: value => value }); + const Manager = vm.runInContext(fs.readFileSync(path.join(__dirname, '../../web/js/gallery.js'), 'utf8') + '\nGalleryManager;', context); + const manager = Object.create(Manager.prototype); + const requests = new Map(); + manager.extractImageMetadata = url => { + const pending = deferred(); + requests.set(url, pending); + return pending.promise; + }; + manager.displayMetadata = (metadata, panel) => { + panel.innerHTML = metadata?.positivePrompt || 'No metadata'; + manager.currentMetadata = metadata; + }; + manager.stopViewerSlideshow = () => {}; + return { manager, requests, panel: { innerHTML: '', isConnected: true } }; +} + +test('a late response cannot overwrite the selected image or copy state', async () => { + const { manager, requests, panel } = fixture(); + const first = manager.loadImageMetadata('old.png', panel); + const second = manager.loadImageMetadata('new.png', panel); + requests.get('new.png').resolve({ positivePrompt: 'new prompt' }); + await second; + requests.get('old.png').resolve({ positivePrompt: 'old prompt' }); + await first; + assert.equal(panel.innerHTML, 'new prompt'); + assert.equal(manager.currentMetadata.positivePrompt, 'new prompt'); +}); + +test('navigation clears old action data while the new request is pending', async () => { + const { manager, requests, panel } = fixture(); + manager.currentMetadata = { positivePrompt: 'old prompt' }; + const pending = manager.loadImageMetadata('new.png', panel); + assert.equal(manager.currentMetadata, null); + requests.get('new.png').resolve(null); + await pending; + assert.equal(manager.currentMetadata, null); +}); + +test('a stale error cannot replace newer metadata', async () => { + const { manager, requests, panel } = fixture(); + const first = manager.loadImageMetadata('old.png', panel); + const second = manager.loadImageMetadata('new.png', panel); + requests.get('new.png').resolve({ positivePrompt: 'new prompt' }); + await second; + requests.get('old.png').reject(new Error('old request failed')); + await first; + assert.equal(panel.innerHTML, 'new prompt'); +}); + +for (const close of ['destroy viewer', 'remove modal']) { + test(`closing via ${close} prevents pending metadata from restoring action data`, async () => { + const { manager, requests, panel } = fixture(); + const pending = manager.loadImageMetadata('old.png', panel); + if (close === 'destroy viewer') manager.destroyViewer(); + else panel.isConnected = false; + requests.get('old.png').resolve({ positivePrompt: 'closed prompt' }); + await pending; + assert.equal(manager.currentMetadata, null); + assert.notEqual(panel.innerHTML, 'closed prompt'); + }); +} diff --git a/tests/js/gallery-root-metadata.test.js b/tests/js/gallery-root-metadata.test.js new file mode 100644 index 0000000..cf220c9 --- /dev/null +++ b/tests/js/gallery-root-metadata.test.js @@ -0,0 +1,30 @@ +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const vm = require('node:vm'); +const path = require('node:path'); +const ImageHelpers = require('../../web/js/image-helpers.js'); + +test('gallery metadata requests retain root identity without including cache parameters in filenames', async () => { + const requests = []; + const context = vm.createContext({ document: { addEventListener() {} }, window: {}, console, + ImageHelpers, ApiClient: { fetchUrl: async url => { + requests.push(url); + return { ok: true, json: async () => ({ success: true, prompt: { text: url } }) }; + } }, + }); + const Manager = vm.runInContext(fs.readFileSync(path.join(__dirname, '../../web/js/gallery.js'), 'utf8') + '\nGalleryManager;', context); + const manager = Object.create(Manager.prototype); + manager.extractImageMetadata = async () => null; + manager.displayMetadata = (metadata, panel) => { panel.prompt = metadata.positivePrompt; }; + for (const root of [0, 1]) { + const panel = {}; + const expected = `/prompt_manager/images/prompt/sub%2Fsame.png?root=${root}`; + await manager.loadImageMetadata(`/prompt_manager/images/serve/sub/same.png?v=123&root=${root}`, panel); + assert.equal(panel.prompt, expected); + } + assert.deepEqual(requests, [ + '/prompt_manager/images/prompt/sub%2Fsame.png?root=0', + '/prompt_manager/images/prompt/sub%2Fsame.png?root=1', + ]); +}); diff --git a/tests/js/image-helpers.test.js b/tests/js/image-helpers.test.js index 0b96278..0439330 100644 --- a/tests/js/image-helpers.test.js +++ b/tests/js/image-helpers.test.js @@ -108,3 +108,16 @@ test("fallbackImageSource gives up when the original itself failed or was alread assert.equal(fallbackImageSource("/serve/t.png", {}), null); assert.equal(fallbackImageSource("/serve/t.png", undefined), null); }); + +const { metadataLookupUrl } = require('../../web/js/image-helpers.js'); + +test('metadata lookup keeps root identity and strips thumbnail cache query parameters', () => { + assert.equal(metadataLookupUrl('/prompt_manager/images/serve/sub/a.png?root=1&v=123'), '/prompt_manager/images/prompt/sub%2Fa.png?root=1'); + assert.equal(metadataLookupUrl('/prompt_manager/images/serve/a.png?root=0'), '/prompt_manager/images/prompt/a.png?root=0'); +}); + +test('metadata lookup preserves encoded filename characters and works behind a proxy', () => { + assert.equal(metadataLookupUrl('https://example.test/ComfyBackendDirect/prompt_manager/images/serve/space%20%23%3F%2520%20caf%C3%A9.png?root=1'), '/prompt_manager/images/prompt/space%20%23%3F%2520%20caf%C3%A9.png?root=1'); + assert.equal(metadataLookupUrl('/prompt_manager/images/serve/a.png?v=123'), '/prompt_manager/images/prompt/a.png'); + assert.equal(metadataLookupUrl('/elsewhere/a.png'), null); +}); diff --git a/tests/test_api_images.py b/tests/test_api_images.py index 06b88b9..2a820ad 100644 --- a/tests/test_api_images.py +++ b/tests/test_api_images.py @@ -224,7 +224,7 @@ class TestServeOutputImage(ImageAPITestCase): self.assertEqual(hit.status, 200) self.assertEqual(miss.status, 404) - self.assertEqual(bad_index.status, 200) + self.assertEqual(bad_index.status, 403) for resp in (hit, miss, bad_index): await resp.read() # release the served file before the directory goes @@ -849,7 +849,7 @@ class TestThumbnailFilesAreNeverSeenHalfWritten(ThumbnailTestCase): ) self.assertRegex( entry["thumbnail_url"], - r"^/prompt_manager/images/serve/thumbnails/a_thumb\.png\?v=\d+$", + r"^/prompt_manager/images/serve/thumbnails/a_thumb\.png\?v=\d+&root=0$", ) first = int(re.search(r"v=(\d+)", entry["thumbnail_url"]).group(1)) self.assertEqual(first, int(thumb.stat().st_mtime)) @@ -1189,12 +1189,12 @@ class TestServeErrorBranches(ImageRouteCoverageCase): status, _ = await self._json("GET", "/prompt_manager/images/serve/x.png") self.assertEqual(status, 500) - async def test_out_of_range_root_index_searches_all_roots(self): + async def test_out_of_range_root_index_is_rejected(self): make_png(self.output_dir / "r.png") resp = await self.client.request( "GET", "/prompt_manager/images/serve/r.png?root=7" ) - self.assertEqual(resp.status, 200) + self.assertEqual(resp.status, 403) async def test_directory_named_like_media_is_404(self): (self.output_dir / "dir.png").mkdir() @@ -1977,3 +1977,94 @@ class TestThumbnailStartupStatus(ThumbnailTestCase): data = dict(parse_sse(await response.text()))["complete"] self.assertEqual(data["count"], 0) self.assertEqual(data["skipped"], 1) + + +class TestGalleryRootIdentity(ThumbnailTestCase): + async def test_colliding_original_and_thumbnail_urls_serve_their_own_roots(self): + roots = [self.output_dir, self.comfy_dir / "second-output"] + for index, root in enumerate(roots): + make_png(root / "same.png", color=(index * 255, 0, 255 - index * 255)) + self.api._get_all_output_dirs = lambda: roots + await self.client.post("/prompt_manager/images/generate-thumbnails", json={}) + data = await (await self.client.get("/prompt_manager/images/output")).json() + self.assertEqual(len({image["url"] for image in data["images"]}), 2) + self.assertEqual(len({image["thumbnail_url"] for image in data["images"]}), 2) + for image in data["images"]: + root = roots[image["root_index"]] + for key, path in ( + ("url", root / "same.png"), + ("thumbnail_url", root / "thumbnails" / "same_thumb.png"), + ): + response = await self.client.get(image[key]) + self.assertEqual(response.status, 200) + self.assertEqual(await response.read(), path.read_bytes()) + + async def test_root_scoped_prompt_lookup_uses_the_matching_database_image(self): + roots = [self.output_dir, self.comfy_dir / "second-output"] + for index, root in enumerate(roots): + self._link_image(make_png(root / "same.png"), text=f"root {index} prompt") + self.api._get_all_output_dirs = lambda: roots + for index in range(2): + response = await self.client.get( + f"/prompt_manager/images/prompt/same.png?root={index}" + ) + self.assertEqual(response.status, 200) + data = await response.json() + self.assertEqual(data["prompt"]["text"], f"root {index} prompt") + + async def test_special_characters_in_paths_survive_serving_and_prompt_lookup(self): + from urllib.parse import quote + + filenames = ["space #%20 café.png"] + if os.name != "nt": + filenames.append("space #?%20 café.png") + for filename in filenames: + self._link_image(make_png(self.output_dir / filename), text="special path") + for filename in filenames: + with self.subTest(filename=filename): + original = self.output_dir / filename + data = await ( + await self.client.get("/prompt_manager/images/output") + ).json() + entry = next( + image for image in data["images"] if image["filename"] == filename + ) + response = await self.client.get(entry["url"]) + self.assertEqual(response.status, 200) + self.assertEqual(await response.read(), original.read_bytes()) + response = await self.client.get( + f"/prompt_manager/images/prompt/{quote(filename, safe='')}?root=0" + ) + data = await response.json() + self.assertEqual(data["prompt"]["text"], "special path") + + async def test_invalid_explicit_roots_never_fall_back_to_another_image(self): + self._link_image(make_png(self.output_dir / "same.png"), text="first root") + for root in ("bad", "-1", "7", ""): + for route in ("serve", "prompt"): + with self.subTest(root=root, route=route): + response = await self.client.get( + f"/prompt_manager/images/{route}/same.png?root={root}" + ) + self.assertEqual(response.status, 403) + + async def test_explicit_root_prompt_lookup_rejects_paths_outside_that_root(self): + from urllib.parse import quote + + outside = make_png(self.comfy_dir / "outside.png") + self._link_image(outside, text="outside") + response = await self.client.get( + f"/prompt_manager/images/prompt/{quote(str(outside), safe='')}?root=0" + ) + self.assertEqual(response.status, 403) + + async def test_unlinked_image_does_not_inherit_a_prompt_from_another_root(self): + second = self.comfy_dir / "second-output" + self._link_image(make_png(self.output_dir / "same.png"), text="first root") + make_png(second / "same.png") + self.api._get_all_output_dirs = lambda: [self.output_dir, second] + response = await self.client.get( + "/prompt_manager/images/prompt/same.png?root=1" + ) + self.assertEqual(response.status, 200) + self.assertFalse((await response.json())["success"]) diff --git a/tools/vendor/tests/viewer.spec.mjs b/tools/vendor/tests/viewer.spec.mjs index dbc18fd..f597288 100644 --- a/tools/vendor/tests/viewer.spec.mjs +++ b/tools/vendor/tests/viewer.spec.mjs @@ -181,3 +181,28 @@ test('touch pinch zoom and rotation remain usable', async ({ page, isMobile }) = await session.send('Input.dispatchTouchEvent', { type: 'touchEnd', touchPoints: [] }); await expect.poll(() => page.evaluate(() => Math.abs(gallery.viewer.imageData.rotate))).toBe(90); }); + +test('metadata follows the selected image when responses finish out of order', async ({ page }) => { + await loadGallery(page); + await page.evaluate(() => { + delete gallery.loadImageMetadata; + window.metadataRequests = new Map(); + gallery.extractImageMetadata = url => new Promise(resolve => metadataRequests.set(url, resolve)); + gallery.openViewer(0); + }); + await expect.poll(() => page.evaluate(() => metadataRequests.has('http://viewer.test/original/0.svg'))).toBe(true); + await page.evaluate(() => gallery.viewer.next()); + await expect.poll(() => page.evaluate(() => metadataRequests.has('http://viewer.test/original/1.svg'))).toBe(true); + await page.evaluate(() => metadataRequests.get('http://viewer.test/original/1.svg')({ positivePrompt: 'Selected prompt' })); + await expect(page.locator('#metadata-content')).toContainText('Selected prompt'); + await page.evaluate(() => metadataRequests.get('http://viewer.test/original/0.svg')({ positivePrompt: 'Stale prompt' })); + await expect(page.locator('#metadata-content')).toContainText('Selected prompt'); + await expect(page.locator('#metadata-content')).not.toContainText('Stale prompt'); + expect(await page.evaluate(() => gallery.currentMetadata.positivePrompt)).toBe('Selected prompt'); + await page.evaluate(() => gallery.viewer.next()); + await expect.poll(() => page.evaluate(() => metadataRequests.has('http://viewer.test/original/2.svg'))).toBe(true); + await page.keyboard.press('Escape'); + await expect(page.locator('#metadata-sidebar')).toHaveCount(0); + await page.evaluate(() => metadataRequests.get('http://viewer.test/original/2.svg')({ positivePrompt: 'Closed prompt' })); + expect(await page.evaluate(() => gallery.currentMetadata)).toBeNull(); +}); diff --git a/web/js/gallery.js b/web/js/gallery.js index beff22f..78deff6 100644 --- a/web/js/gallery.js +++ b/web/js/gallery.js @@ -583,6 +583,8 @@ } destroyViewer({ keepSlideshow = false } = {}) { + this.metadataRequest = null; + this.currentMetadata = null; if (!keepSlideshow) this.stopViewerSlideshow(); if (this.viewer) { this.viewer.clearEnforceFocus(); @@ -647,6 +649,8 @@ } }, hide: () => { + this.metadataRequest = null; + this.currentMetadata = null; this.viewer?.clearEnforceFocus(); this.stopViewerSlideshow(); const sidebar = document.getElementById('metadata-sidebar'); @@ -1189,15 +1193,18 @@ } async loadImageMetadata(imageUrl, container) { + const request = {}; + this.metadataRequest = request; + this.currentMetadata = null; + container.textContent = 'Loading metadata...'; + const isCurrent = () => this.metadataRequest === request && container.isConnected !== false; try { // First try to get linked prompt from database let databasePrompt = null; try { - // Extract relative path from the URL for the API call - const urlParts = imageUrl.split('/prompt_manager/images/serve/'); - if (urlParts.length > 1) { - const relativePath = urlParts[1]; - const response = await ApiClient.fetchUrl(`/prompt_manager/images/prompt/${encodeURIComponent(relativePath)}`); + const metadataUrl = ImageHelpers.metadataLookupUrl(imageUrl); + if (metadataUrl) { + const response = await ApiClient.fetchUrl(metadataUrl); if (response.ok) { const data = await response.json(); if (data.success && data.prompt) { @@ -1256,8 +1263,9 @@ } } - this.displayMetadata(metadata, container); + if (isCurrent()) this.displayMetadata(metadata, container); } catch (error) { + if (!isCurrent()) return; console.error('Error loading metadata:', error); container.innerHTML = `
diff --git a/web/js/image-helpers.js b/web/js/image-helpers.js index 73ae4da..70f31ee 100644 --- a/web/js/image-helpers.js +++ b/web/js/image-helpers.js @@ -103,7 +103,23 @@ return original; } - const api = { formatImageCaption, workflowDocumentHtml, autotagSingleBody, fallbackImageSource }; + function metadataLookupUrl(imageUrl) { + try { + const url = new URL(imageUrl, 'http://prompt-manager.invalid'); + const marker = '/prompt_manager/images/serve/'; + const index = url.pathname.indexOf(marker); + if (index < 0) return null; + const relativePath = decodeURIComponent(url.pathname.slice(index + marker.length)); + const query = new URLSearchParams(); + if (url.searchParams.has('root')) query.set('root', url.searchParams.get('root')); + const suffix = query.size ? `?${query}` : ''; + return `/prompt_manager/images/prompt/${encodeURIComponent(relativePath)}${suffix}`; + } catch { + return null; + } + } + + const api = { formatImageCaption, workflowDocumentHtml, autotagSingleBody, fallbackImageSource, metadataLookupUrl }; if (typeof module !== "undefined" && module.exports) module.exports = api; else root.ImageHelpers = api; })(typeof window !== "undefined" ? window : globalThis);