Fix wallpaper narrowing + reuse shared sanitizeUrl at img sink; add tests

Co-authored-by: mroxso <24775431+mroxso@users.noreply.github.com>

src/apps/settings/WallpaperSection.test.tsx

@@ -0,0 +1,55 @@
+import { beforeEach, describe, expect, it } from 'vitest';
+import { fireEvent, render, screen, waitFor } from '@testing-library/react';
+import { TestApp } from '@/test/TestApp';
+import { WallpaperSection } from './index';
+
+// Radix's Slider (used for the dimming control) measures its track via
+// ResizeObserver. The vi.fn()-based mock in src/test/setup.ts cannot be
+// used with `new` on this environment/vitest combination, so provide a
+// minimal real constructor for just this file.
+class StubResizeObserver {
+  observe() {}
+  unobserve() {}
+  disconnect() {}
+}
+
+describe('WallpaperSection', () => {
+  beforeEach(() => {
+    global.ResizeObserver = StubResizeObserver as unknown as typeof ResizeObserver;
+  });
+
+  it('shows the dot-grid curated wallpaper selected by default', async () => {
+    render(<WallpaperSection />, { wrapper: TestApp });
+
+    const dotGrid = await screen.findByRole('radio', { name: 'Dot grid' });
+    expect(dotGrid).toBeChecked();
+    const aurora = screen.getByRole('radio', { name: 'Aurora bands' });
+    expect(aurora).not.toBeChecked();
+  });
+
+  it('selects a different curated wallpaper when clicked', async () => {
+    render(<WallpaperSection />, { wrapper: TestApp });
+
+    const aurora = await screen.findByRole('radio', { name: 'Aurora bands' });
+    fireEvent.click(aurora);
+
+    await waitFor(() => expect(aurora).toBeChecked());
+    expect(screen.getByRole('radio', { name: 'Dot grid' })).not.toBeChecked();
+  });
+
+  it('does not enable saving a custom URL until it has been previewed', async () => {
+    render(<WallpaperSection />, { wrapper: TestApp });
+
+    const saveButton = await screen.findByRole('button', { name: 'Save wallpaper' });
+    expect(saveButton).toBeDisabled();
+
+    fireEvent.change(screen.getByLabelText('Custom image'), {
+      target: { value: 'http://not-https.example.com/a.png' },
+    });
+    fireEvent.click(screen.getByRole('button', { name: 'Preview' }));
+
+    // An insecure URL must never start a preview/decode.
+    expect(saveButton).toBeDisabled();
+    expect(screen.queryByAltText('Wallpaper preview')).not.toBeInTheDocument();
+  });
+});

src/apps/settings/index.tsx

@@ -165,7 +165,7 @@ const MAX_WALLPAPER_FILE_BYTES = 5 * 1024 * 1024;
 const MAX_WALLPAPER_DIMENSION = 6000;
 const ALLOWED_WALLPAPER_TYPES = new Set(['image/jpeg', 'image/png', 'image/webp', 'image/gif']);
 
-function WallpaperSection() {
+export function WallpaperSection() {
   const { config, updateConfig } = useAppContext();
   const { user } = useCurrentUser();
   const { mutateAsync: uploadFile, isPending: uploading } = useUploadFile();
@@ -195,13 +195,14 @@ function WallpaperSection() {
   };
 
   const applyUrl = () => {
-    if (preview.status !== 'ready' || !preview.url || preview.url !== urlDraft.trim()) {
+    const previewedUrl = preview.url;
+    if (preview.status !== 'ready' || !previewedUrl || previewedUrl !== urlDraft.trim()) {
       toast({ title: 'Preview the image before saving it', variant: 'destructive' });
       return;
     }
     updateConfig((current) => ({
       ...current,
-      wallpaper: { version: 1, selection: { source: 'url', url: preview.url as string, presentation: { fit, dim } } },
+      wallpaper: { version: 1, selection: { source: 'url', url: previewedUrl, presentation: { fit, dim } } },
     }));
     toast({ title: 'Wallpaper saved' });
   };

src/components/os/Desktop.tsx

@@ -21,6 +21,7 @@ import { useIconLayout } from '@/os/useIconLayout';
 import { useAppContext } from '@/hooks/useAppContext';
 import { useDecodedImage } from '@/hooks/useDecodedImage';
 import { CURATED_WALLPAPERS, DEFAULT_CURATED_ID, isSafeWallpaperUrl, resolveCurated } from '@/lib/wallpaper';
+import { sanitizeUrl } from '@/lib/nostrUtils';
 import { cn } from '@/lib/utils';
 
 const CELL_WIDTH = 96;
@@ -57,10 +58,20 @@ export function Desktop() {
   const slots = layout.desktop;
 
   const wallpaper = config.wallpaper.selection;
-  const isCustomWallpaper = wallpaper.source === 'url' && isSafeWallpaperUrl(wallpaper.url);
-  const decoded = useDecodedImage(isCustomWallpaper ? wallpaper.url : undefined);
-  const showWallpaperImage = isCustomWallpaper && decoded.status === 'ready' && decoded.url === wallpaper.url;
-  const wallpaperDataAttr = wallpaper.source === 'curated' ? wallpaper.id : undefined;
+  const customWallpaper = wallpaper.source === 'url' && isSafeWallpaperUrl(wallpaper.url) ? wallpaper : undefined;
+  const decoded = useDecodedImage(customWallpaper?.url);
+  // `decoded.url` holds the last *successfully* decoded image regardless of
+  // whether a newer selection is still loading or has failed, so switching
+  // to a new custom wallpaper (or a failed one) never blanks the desktop.
+  // Re-validated at the render boundary (rather than trusted from state) so
+  // the only place an <img src> is ever set from external data is guarded
+  // right next to the sink, independent of how `decoded.url` got here. Uses
+  // the same protocol-allowlist sanitizer as every other untrusted URL in
+  // the app (see `sanitizeUrl` in `nostrUtils.ts`), tightened to https-only.
+  const sanitizedWallpaperUrl = sanitizeUrl(decoded.url);
+  const safeWallpaperImageUrl =
+    customWallpaper && sanitizedWallpaperUrl && isSafeWallpaperUrl(sanitizedWallpaperUrl) ? sanitizedWallpaperUrl : undefined;
+  const curatedId = wallpaper.source === 'curated' ? wallpaper.id : undefined;
 
   const setCuratedWallpaper = useCallback((id: string) => {
     updateConfig((current) => ({ ...current, wallpaper: { version: 1, selection: { source: 'curated', id } } }));
@@ -155,30 +166,28 @@ export function Desktop() {
         <main
           className={cn(
             'os-desktop-surface absolute inset-x-0 bottom-0 overflow-hidden',
-            wallpaperDataAttr && resolveCurated(wallpaperDataAttr).className,
+            curatedId && resolveCurated(curatedId).className,
           )}
           style={{ top: MENUBAR_HEIGHT }}
           onPointerDown={(event) => {
             if (event.target === event.currentTarget) setSelected(null);
           }}
         >
-          {isCustomWallpaper && (
+          {customWallpaper && safeWallpaperImageUrl && (
             <>
-              {showWallpaperImage && (
-                <img
-                  src={decoded.url}
-                  alt=""
-                  aria-hidden="true"
-                  className={cn(
-                    'pointer-events-none absolute inset-0 h-full w-full',
-                    wallpaper.presentation.fit === 'contain' ? 'object-contain' : 'object-cover',
-                  )}
-                />
-              )}
-              {showWallpaperImage && wallpaper.presentation.dim > 0 && (
+              <img
+                src={safeWallpaperImageUrl}
+                alt=""
+                aria-hidden="true"
+                className={cn(
+                  'pointer-events-none absolute inset-0 h-full w-full',
+                  customWallpaper.presentation.fit === 'contain' ? 'object-contain' : 'object-cover',
+                )}
+              />
+              {customWallpaper.presentation.dim > 0 && (
                 <div
                   className="pointer-events-none absolute inset-0 bg-black"
-                  style={{ opacity: wallpaper.presentation.dim / 100 }}
+                  style={{ opacity: customWallpaper.presentation.dim / 100 }}
                   aria-hidden="true"
                 />
               )}

src/hooks/useDecodedImage.test.ts

@@ -0,0 +1,118 @@
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+import { renderHook, waitFor } from '@testing-library/react';
+import { useDecodedImage } from './useDecodedImage';
+
+/**
+ * A controllable stand-in for the browser's `Image` constructor. Each
+ * instance's `decode()` promise is only resolved/rejected when the test
+ * explicitly does so, so we can assert on the intermediate "loading" state
+ * and on stale-request ordering.
+ */
+class FakeImage {
+  decoding = '';
+  src = '';
+  naturalWidth = 100;
+  naturalHeight = 50;
+  private resolveDecode!: () => void;
+  private rejectDecode!: (error: unknown) => void;
+  readonly decodePromise = new Promise<void>((resolve, reject) => {
+    this.resolveDecode = resolve;
+    this.rejectDecode = reject;
+  });
+
+  constructor() {
+    instances.push(this);
+  }
+
+  decode() {
+    return this.decodePromise;
+  }
+
+  finish() {
+    this.resolveDecode();
+  }
+
+  fail() {
+    this.rejectDecode(new Error('decode failed'));
+  }
+}
+
+let instances: FakeImage[] = [];
+
+describe('useDecodedImage', () => {
+  let originalImage: typeof Image;
+
+  beforeEach(() => {
+    instances = [];
+    originalImage = globalThis.Image;
+    globalThis.Image = FakeImage as unknown as typeof Image;
+  });
+
+  afterEach(() => {
+    globalThis.Image = originalImage;
+  });
+
+  it('starts idle when there is no url', () => {
+    const { result } = renderHook(() => useDecodedImage(undefined));
+    expect(result.current.status).toBe('idle');
+    expect(result.current.url).toBeUndefined();
+  });
+
+  it('transitions loading -> ready once the image decodes', async () => {
+    const { result } = renderHook(() => useDecodedImage('https://example.com/a.png'));
+    expect(result.current.status).toBe('loading');
+
+    instances[0].finish();
+
+    await waitFor(() => expect(result.current.status).toBe('ready'));
+    expect(result.current.url).toBe('https://example.com/a.png');
+    expect(result.current.width).toBe(100);
+  });
+
+  it('keeps the last successfully decoded url when a new decode fails', async () => {
+    const { result, rerender } = renderHook(({ url }) => useDecodedImage(url), {
+      initialProps: { url: 'https://example.com/good.png' },
+    });
+    instances[0].finish();
+    await waitFor(() => expect(result.current.status).toBe('ready'));
+    expect(result.current.url).toBe('https://example.com/good.png');
+
+    rerender({ url: 'https://example.com/bad.png' });
+    expect(result.current.status).toBe('loading');
+
+    instances[1].fail();
+    await waitFor(() => expect(result.current.status).toBe('error'));
+    // The previously-working wallpaper must not be un-applied by a failed swap.
+    expect(result.current.url).toBe('https://example.com/good.png');
+  });
+
+  it('ignores a stale decode that resolves after a newer request replaced it', async () => {
+    const { result, rerender } = renderHook(({ url }) => useDecodedImage(url), {
+      initialProps: { url: 'https://example.com/first.png' },
+    });
+    rerender({ url: 'https://example.com/second.png' });
+    expect(instances).toHaveLength(2);
+
+    instances[1].finish();
+    await waitFor(() => expect(result.current.status).toBe('ready'));
+    expect(result.current.url).toBe('https://example.com/second.png');
+
+    // The stale first request resolving afterwards must not overwrite the
+    // newer, already-applied selection.
+    instances[0].finish();
+    await new Promise((resolve) => setTimeout(resolve, 0));
+    expect(result.current.url).toBe('https://example.com/second.png');
+  });
+
+  it('resets to idle when the url is cleared', async () => {
+    const { result, rerender } = renderHook(({ url }) => useDecodedImage(url), {
+      initialProps: { url: 'https://example.com/a.png' as string | undefined },
+    });
+    instances[0].finish();
+    await waitFor(() => expect(result.current.status).toBe('ready'));
+
+    rerender({ url: undefined });
+    expect(result.current.status).toBe('idle');
+    expect(result.current.url).toBeUndefined();
+  });
+});

src/hooks/useDecodedImage.ts

@@ -3,7 +3,12 @@ import { useEffect, useState } from 'react';
 export type DecodedImageStatus = 'idle' | 'loading' | 'ready' | 'error';
 
 export interface DecodedImageState {
-  /** The URL that finished decoding successfully, or undefined if none has yet. */
+  /**
+   * The URL that most recently finished decoding successfully, or undefined
+   * if none has yet. This intentionally lags behind `status` while a newer
+   * request is loading or has failed, so a consumer can keep rendering it
+   * as "the current working wallpaper" until a *new* url succeeds.
+   */
   url: string | undefined;
   status: DecodedImageStatus;
   width: number | undefined;
@@ -27,10 +32,13 @@ export function useDecodedImage(url: string | undefined): DecodedImageState {
 
   // Mirrors the useLocalStorage "key changed" pattern: reset synchronously
   // during render rather than in an effect body, so there is no flash of
-  // stale state before the new decode starts.
+  // stale state before the new decode starts. `url` (the last *successful*
+  // decode) is deliberately preserved here — only `status` moves to
+  // 'loading'/'idle' — so a caller keeps showing the last working image
+  // while a new one loads or if it fails.
   if (trackedUrl !== url) {
     setTrackedUrl(url);
-    setState(url ? { url: undefined, status: 'loading', width: undefined, height: undefined } : { url: undefined, status: 'idle', width: undefined, height: undefined });
+    setState((prev) => (url ? { ...prev, status: 'loading' } : { url: undefined, status: 'idle', width: undefined, height: undefined }));
   }
 
   useEffect(() => {
@@ -47,7 +55,7 @@ export function useDecodedImage(url: string | undefined): DecodedImageState {
       })
       .catch(() => {
         if (cancelled) return;
-        setState((prev) => ({ url: prev.status === 'ready' ? prev.url : undefined, status: 'error', width: undefined, height: undefined }));
+        setState((prev) => ({ ...prev, status: 'error' }));
       });
 
     return () => {