# Edge Case Hunter — Story 4.2 Code Review You are an **Edge Case Hunter**. You receive the diff below AND read access to the project. Walk every branching path and boundary condition. Report ONLY unhandled edge cases. ## Instructions - Review the diff against the existing project codebase. - Walk all: conditional branches, type transitions, null/undefined checks, boundary values, error paths, async flows, DOM states, mock/fake surfaces, API integration points. - Report ONLY issues where an edge case is unhandled or handled incorrectly — not general critique. - Each finding: **one-line title**, file + line, the edge condition, and why it matters. - If every edge case is properly handled, say "No unhandled edge cases found." ## Project Access You have read access to the full project at: `/home/morr/work/foundryvtt/video-view-manager/` Key files to cross-reference: - `src/contracts/privacy-settings.js` — existing contract (read the full file for context around the diff) - `src/core/PlayerPrivacyManager.js` — existing manager (read the full file for context around the diff) - `src/core/PortraitFallbackHandler.js` — new file (shown in full below) - `src/foundry/FoundryAdapter.js` — adapter interface - `src/ui/RoleRenderer.js` — eventual integration target - `src/ui/player/PlayerPrivacyPanel.js` — eventual UI target - `tests/helpers/foundryAdapterMock.js` — existing mock patterns - `tests/helpers/playerPrivacyManagerMock.js` — new mock helper ## Diff === MODIFIED FILES === ```diff diff --git a/src/contracts/privacy-settings.js b/src/contracts/privacy-settings.js index b838445..f618241 100644 --- a/src/contracts/privacy-settings.js +++ b/src/contracts/privacy-settings.js @@ -5,15 +5,34 @@ * their on-screen presence. Settings are stored as user flags on the user document. * * Storage key: game.user.setFlag('video-view-manager', key, value) - * Shape: { reactionCamEnabled: boolean, hpReactiveCamStylingEnabled: boolean } + * Shape: { reactionCamEnabled: boolean, hpReactiveCamStylingEnabled: boolean, customPortraitFallback: string | null } * * @module contracts/privacy-settings */ +export const MAX_PORTRAIT_SIZE = 5 * 1024 * 1024; + +export const VALID_PORTRAIT_FORMATS = Object.freeze([ + "image/png", + "image/jpeg", + "image/webp", + "image/gif", +]); + /** * @typedef {Object} PrivacySettings * @property {boolean} reactionCamEnabled * @property {boolean} hpReactiveCamStylingEnabled + * @property {string|null} customPortraitFallback */ export const PRIVACY_SETTINGS_VERSION = 1; @@ -25,6 +44,7 @@ export const PRIVACY_SETTINGS_VERSION = 1; export const PRIVACY_SETTINGS_DEFAULT = { reactionCamEnabled: false, hpReactiveCamStylingEnabled: false, + customPortraitFallback: null, }; @@ -34,6 +54,7 @@ export const PRIVACY_SETTING_KEYS = Object.freeze([ "reactionCamEnabled", "hpReactiveCamStylingEnabled", + "customPortraitFallback", ]); @@ -45,6 +66,49 @@ export const FEATURE_NAME_MAP = Object.freeze({ hpReactiveCamStyling: "hpReactiveCamStylingEnabled", }); +export function validatePortraitDataURL(dataURL) { + if (dataURL === null || dataURL === undefined) { + return dataURL; + } + if (typeof dataURL !== "string") { + throw new TypeError(`Invalid DataURL: expected string, got ${typeof dataURL}`); + } + if (dataURL === "") { + return dataURL; + } + + if (!dataURL.startsWith("data:")) { + throw new TypeError("Invalid DataURL format: must start with 'data:'"); + } + + const mimeMatch = dataURL.match(/^data:(image\/[a-zA-Z0-9+\-.]+|video\/[a-zA-Z0-9+\-.]+)/); + if (!mimeMatch) { + throw new TypeError("Invalid DataURL format: missing or invalid MIME type"); + } + + const mimeType = mimeMatch[1].toLowerCase(); + + if (!VALID_PORTRAIT_FORMATS.includes(mimeType)) { + throw new TypeError( + `Unsupported portrait format: ${mimeType}. Supported: ${VALID_PORTRAIT_FORMATS.join(", ")}` + ); + } + + return dataURL; +} + /** * Creates a new PrivacySettings object with defaults. */ @@ -63,6 +127,7 @@ export function createPrivacySettings(overrides = {}) { /** * Validates a PrivacySettings DTO. Throws TypeError on any violation. + * Backward compatible: accepts settings without customPortraitFallback key. */ @@ -75,23 +140,50 @@ export function isValidPrivacySettings(data) { throw new TypeError("PrivacySettings: must be an object"); } const obj = /** @type {Record} */ (data); - const { reactionCamEnabled, hpReactiveCamStylingEnabled, ...rest } = obj; - if (Object.keys(rest).length > 0) { + + const knownKeys = new Set(PRIVACY_SETTING_KEYS); + const unknownKeys = Object.keys(obj).filter((k) => !knownKeys.has(k)); + if (unknownKeys.length > 0) { throw new TypeError( - `PrivacySettings: unknown keys: ${Object.keys(rest).join(", ")}` + `PrivacySettings: unknown keys: ${unknownKeys.join(", ")}` ); } - if (typeof reactionCamEnabled !== "boolean") { - throw new TypeError( - `PrivacySettings: reactionCamEnabled must be a boolean, got ${typeof reactionCamEnabled}` - ); + + if ("reactionCamEnabled" in obj) { + if (typeof obj.reactionCamEnabled !== "boolean") { + throw new TypeError( + `PrivacySettings: reactionCamEnabled must be a boolean, got ${typeof obj.reactionCamEnabled}` + ); + } } - if (typeof hpReactiveCamStylingEnabled !== "boolean") { - throw new TypeError( - `PrivacySettings: hpReactiveCamStylingEnabled must be a boolean, got ${typeof hpReactiveCamStylingEnabled}` - ); + if ("hpReactiveCamStylingEnabled" in obj) { + if (typeof obj.hpReactiveCamStylingEnabled !== "boolean") { + throw new TypeError( + `PrivacySettings: hpReactiveCamStylingEnabled must be a boolean, got ${typeof obj.hpReactiveCamStylingEnabled}` + ); + } } - return /** @type {PrivacySettings} */ (data); + if ("customPortraitFallback" in obj) { + if (obj.customPortraitFallback !== null && typeof obj.customPortraitFallback !== "string") { + throw new TypeError( + `PrivacySettings: customPortraitFallback must be a string or null, got ${typeof obj.customPortraitFallback}` + ); + } + if (typeof obj.customPortraitFallback === "string" && obj.customPortraitFallback !== "") { + try { + validatePortraitDataURL(obj.customPortraitFallback); + } catch (e) { + throw new TypeError( + `PrivacySettings: customPortraitFallback ${e.message}` + ); + } + } + } + + return /** @type {PrivacySettings} */ (obj); } diff --git a/src/core/PlayerPrivacyManager.js b/src/core/PlayerPrivacyManager.js index 117afa2..9347edd 100644 --- a/src/core/PlayerPrivacyManager.js +++ b/src/core/PlayerPrivacyManager.js @@ -15,9 +15,11 @@ import { PRIVACY_SETTINGS_DEFAULT, PRIVACY_SETTING_KEYS, FEATURE_NAME_MAP, + MAX_PORTRAIT_SIZE, validateSettingKey, validateSettingValue, validateFeatureName, + validatePortraitDataURL, } from "../contracts/privacy-settings.js"; @@ -118,12 +120,19 @@ export class PlayerPrivacyManager { * @param {string} key * @param {boolean} value */ async setSetting(userId, key, value) { + if (key === "customPortraitFallback") { + throw new TypeError( + "PlayerPrivacyManager: customPortraitFallback must use setPortraitFallback() method" + ); + } + // Validate key validateSettingKey(key); @@ -245,6 +254,149 @@ export class PlayerPrivacyManager { } } + async setPortraitFallback(userId, dataURL) { + validatePortraitDataURL(dataURL); + + const user = this._adapter.users.get(userId); + if (!user) { + throw new TypeError(`PlayerPrivacyManager: User '${userId}' not found`); + } + + if (typeof user.setFlag !== "function") { + throw new TypeError(`PlayerPrivacyManager: User '${userId}' does not support setFlag`); + } + + const previousValue = this.getPortraitFallback(userId); + + await user.setFlag("video-view-manager", "customPortraitFallback", dataURL); + + this._notifyPortraitChange(userId, dataURL, previousValue); + } + + getPortraitFallback(userId) { + const user = this._adapter.users.get(userId); + + if (!user || typeof user.getFlag !== "function") { + return null; + } + + const dataURL = user.getFlag("video-view-manager", "customPortraitFallback"); + + if (dataURL !== null && dataURL !== undefined) { + try { + if (typeof dataURL === "string" && dataURL !== "") { + validatePortraitDataURL(dataURL); + } + return dataURL ?? null; + } catch (e) { + console.warn(`[ScryingPool] PlayerPrivacyManager: Invalid stored portrait DataURL for user '${userId}': ${e.message}`); + return null; + } + } + + return null; + } + + getPortraitFallbackDataURL(userId) { + return this.getPortraitFallback(userId); + } + + async removePortraitFallback(userId) { + const user = this._adapter.users.get(userId); + if (!user) { + throw new TypeError(`PlayerPrivacyManager: User '${userId}' not found`); + } + + if (typeof user.unsetFlag !== "function") { + throw new TypeError(`PlayerPrivacyManager: User '${userId}' does not support unsetFlag`); + } + + const previousValue = this.getPortraitFallback(userId); + + await user.unsetFlag("video-view-manager", "customPortraitFallback"); + + this._notifyPortraitChange(userId, null, previousValue); + } + + _notifyPortraitChange(userId, newValue, previousValue) { + for (const callback of this._subscribers) { + try { + callback(userId, "customPortraitFallback", newValue, previousValue); + } catch (err) { + console.error(`[ScryingPool] PlayerPrivacyManager portrait subscriber error:`, err); + } + } + } + /** * Cleans up internal state. teardown() { ... } ``` === NEW FILES === **src/core/PortraitFallbackHandler.js** (249 lines — full file available at that path) **tests/helpers/playerPrivacyManagerMock.js** (50 lines — full file available at that path)