diff --git a/backend/__tests__/integration/imageSecurityDefaults.test.js b/backend/__tests__/integration/imageSecurityDefaults.test.js index 1e8bea4a..74ca0c11 100644 --- a/backend/__tests__/integration/imageSecurityDefaults.test.js +++ b/backend/__tests__/integration/imageSecurityDefaults.test.js @@ -134,6 +134,15 @@ describe('image-security creation defaults', () => { expect(await getImageSecurityDefaults()).toEqual({ image_quality: 72 }); }); + it('reads a value buried under many saves, not just one', async () => { + // Each visit to the settings tab used to add a layer, so the depth is + // however many times someone opened it — not a number to cap. + let raw = JSON.stringify('maximum'); + for (let i = 0; i < 8; i += 1) raw = JSON.stringify(raw); + await setRaw('default_protection_level', raw); + expect(await getImageSecurityDefaults()).toEqual({ protection_level: 'maximum' }); + }); + it('still rejects a malformed value however many times it was encoded', async () => { await setRaw('default_image_quality', JSON.stringify(JSON.stringify('72oops'))); expect(await getImageSecurityDefaults()).toEqual({}); diff --git a/backend/src/routes/adminEvents/helpers.js b/backend/src/routes/adminEvents/helpers.js index e031c604..f6fef319 100644 --- a/backend/src/routes/adminEvents/helpers.js +++ b/backend/src/routes/adminEvents/helpers.js @@ -135,10 +135,16 @@ const toInteger = (value) => { return undefined; }; -const getImageSecurityDefaults = async () => { +const getImageSecurityDefaults = async (trx = null) => { const defaults = {}; try { - const rows = await db('app_settings') + // Accepts a transaction the way getAppSetting does. It matters on + // sqlite3, whose pool holds a single connection: a caller already inside + // db.transaction() that read through the global `db` would block on the + // connection its own transaction holds until the acquire timeout, and + // the catch below would then quietly swallow it and drop the defaults. + const query = trx || db; + const rows = await query('app_settings') .whereIn('setting_key', [ 'default_protection_level', 'default_image_quality', @@ -156,12 +162,16 @@ const getImageSecurityDefaults = async () => { // `true` is stored as "\"true\"" and a single parse yields the string // 'true', which the type checks below reject — the settings would go // quietly dead again, which is the bug this whole change exists to fix. - // Unwrap until it stops being a JSON string, bounded so nothing spins. + // The GET handler now decodes, so this stops accumulating — but installs + // that already stacked N layers have to keep working, and N is however + // many times someone opened that tab. So unwrap until it stops being a + // JSON string rather than to a fixed depth; this terminates because each + // parse of a string is strictly shorter than its input. const read = (key) => { const row = rows.find((r) => r.setting_key === key); if (!row) return undefined; let value = row.setting_value; - for (let i = 0; i < 4 && typeof value === 'string'; i += 1) { + while (typeof value === 'string') { let parsed; try { parsed = JSON.parse(value); } catch { break; } if (parsed === value) break; diff --git a/backend/src/routes/adminImageSecurity.js b/backend/src/routes/adminImageSecurity.js index 877a5421..e83df3b3 100644 --- a/backend/src/routes/adminImageSecurity.js +++ b/backend/src/routes/adminImageSecurity.js @@ -32,9 +32,22 @@ router.get('/settings', adminAuth, requirePermission(['settings.view', 'image_se const config = {}; settings.forEach(setting => { - // PostgreSQL JSON columns are already parsed by the driver - // Just use the value directly - no need to JSON.parse - config[setting.setting_key] = setting.setting_value; + // setting_value is JSON text on SQLite, and already decoded by the + // driver on a PG json column — so returning it raw shipped strings + // like "true" to a tab that types the field as boolean. Worse, the + // tab PUTs this whole object straight back through JSON.stringify, + // so every save wrapped another layer of quoting around values nobody + // edited, until consumers could no longer read them (#1296). Decode + // here so a round trip is idempotent. This terminates: each parse of + // a string is strictly shorter than its input. + let value = setting.setting_value; + while (typeof value === 'string') { + let parsed; + try { parsed = JSON.parse(value); } catch { break; } + if (parsed === value) break; + value = parsed; + } + config[setting.setting_key] = value; }); res.json(config); diff --git a/backend/src/services/quoteService.js b/backend/src/services/quoteService.js index e92245e1..e6943973 100644 --- a/backend/src/services/quoteService.js +++ b/backend/src/services/quoteService.js @@ -1684,7 +1684,7 @@ async function convertToEvent(quoteId, adminId, options = {}) { // — bullet-proof against schema drift in either direction. const eventCols = await trx('events').columnInfo(); const { getImageSecurityDefaults, resolveImageSecurityColumns } = require('../routes/adminEvents/helpers'); - const imageSecurityColumns = resolveImageSecurityColumns({}, await getImageSecurityDefaults()); + const imageSecurityColumns = resolveImageSecurityColumns({}, await getImageSecurityDefaults(trx)); const candidate = { slug: `quote-${quote.quote_number.toLowerCase()}-${crypto.randomBytes(3).toString('hex')}`, event_name: quote.event_name || `Event ${quote.quote_number}`,