fix(feedback): bound image preview resources

This commit is contained in:
Brennan Benson
2026-07-29 18:42:21 -07:00
parent a2d217273f
commit e2d60e84e5
9 changed files with 138 additions and 16 deletions
@@ -166,6 +166,32 @@ describe('SidebarFeedbackDialog image submission', () => {
expect(onOpenChange).toHaveBeenCalledWith(false)
})
it('releases image previews when the sidebar unmounts the dialog', async () => {
mocks.readFeedbackImageFiles.mockResolvedValue({
images: [
{
id: 'shot',
name: 'shot.png',
contentType: 'image/png',
bytes: 1,
data: new Uint8Array([1]),
previewUrl: 'blob:shot'
}
],
errors: []
})
const { container, unmount } = render(<SidebarFeedbackDialog open onOpenChange={vi.fn()} />)
const input = container.querySelector<HTMLInputElement>('input[type="file"]')
fireEvent.change(input!, {
target: { files: [new File(['x'], 'shot.png', { type: 'image/png' })] }
})
await screen.findByRole('button', { name: 'Remove shot.png' })
unmount()
expect(URL.revokeObjectURL).toHaveBeenCalledWith('blob:shot')
})
it('does not consume text when the pasted image cannot be attached', () => {
mocks.readFeedbackImageFiles.mockResolvedValue({
images: [],
@@ -70,17 +70,23 @@ export function SidebarFeedbackDialog({
const [pendingImageReadCount, setPendingImageReadCount] = useState(0)
const mountedRef = useMountedRef()
const feedbackTextareaRef = useRef<HTMLTextAreaElement>(null)
const liveImageDraftsRef = useRef<FeedbackImageDraft[]>([])
const clearImages = React.useCallback(() => {
setImages((current) => {
current.forEach(releaseFeedbackImageDraft)
return []
})
liveImageDraftsRef.current.forEach(releaseFeedbackImageDraft)
liveImageDraftsRef.current = []
setImages([])
}, [])
// Why: object URLs for the thumbnails leak until revoked, so drop them when
// the dialog unmounts as well as when an attachment is removed.
React.useEffect(() => clearImages, [clearImages])
React.useEffect(
() => () => {
liveImageDraftsRef.current.forEach(releaseFeedbackImageDraft)
liveImageDraftsRef.current = []
},
[]
)
const imageCount = images.length
@@ -117,6 +123,7 @@ export function SidebarFeedbackDialog({
}
setPendingImageReadCount((current) => Math.max(0, current - files.length))
if (added.length > 0) {
liveImageDraftsRef.current = [...liveImageDraftsRef.current, ...added]
setImages((existing) => [...existing, ...added])
}
// Why: never drop an attachment without telling the user — that
@@ -142,13 +149,12 @@ export function SidebarFeedbackDialog({
)
const handleRemoveImage = React.useCallback((id: string) => {
setImages((current) => {
const removed = current.find((image) => image.id === id)
if (removed) {
releaseFeedbackImageDraft(removed)
}
return current.filter((image) => image.id !== id)
})
const removed = liveImageDraftsRef.current.find((image) => image.id === id)
if (removed) {
releaseFeedbackImageDraft(removed)
liveImageDraftsRef.current = liveImageDraftsRef.current.filter((image) => image.id !== id)
}
setImages((current) => current.filter((image) => image.id !== id))
}, [])
const { isDragActive, contentRef, dragHandlers } = useFeedbackImageDrop(open, handleAddFiles)
+2
View File
@@ -667,6 +667,8 @@
"empty": "{{fileName}} is empty.",
"tooLarge": "{{fileName}} is larger than {{maxSize}}.",
"tooMany": "You can attach up to {{maxCount}} images.",
"dimensionsTooLarge": "{{fileName}} has dimensions that are too large to preview safely.",
"invalidImage": "{{fileName}} is not a valid supported image.",
"additionalErrors": "{{count}} additional images could not be attached."
}
}
+2
View File
@@ -644,6 +644,8 @@
"empty": "{{fileName}} is empty.",
"tooLarge": "{{fileName}} is larger than {{maxSize}}.",
"tooMany": "You can attach up to {{maxCount}} images.",
"dimensionsTooLarge": "{{fileName}} has dimensions that are too large to preview safely.",
"invalidImage": "{{fileName}} is not a valid supported image.",
"additionalErrors": "{{count}} additional images could not be attached."
}
}
+2
View File
@@ -644,6 +644,8 @@
"empty": "{{fileName}} is empty.",
"tooLarge": "{{fileName}} is larger than {{maxSize}}.",
"tooMany": "You can attach up to {{maxCount}} images.",
"dimensionsTooLarge": "{{fileName}} has dimensions that are too large to preview safely.",
"invalidImage": "{{fileName}} is not a valid supported image.",
"additionalErrors": "{{count}} additional images could not be attached."
}
}
+2
View File
@@ -644,6 +644,8 @@
"empty": "{{fileName}} is empty.",
"tooLarge": "{{fileName}} is larger than {{maxSize}}.",
"tooMany": "You can attach up to {{maxCount}} images.",
"dimensionsTooLarge": "{{fileName}} has dimensions that are too large to preview safely.",
"invalidImage": "{{fileName}} is not a valid supported image.",
"additionalErrors": "{{count}} additional images could not be attached."
}
}
+2
View File
@@ -644,6 +644,8 @@
"empty": "{{fileName}} is empty.",
"tooLarge": "{{fileName}} is larger than {{maxSize}}.",
"tooMany": "You can attach up to {{maxCount}} images.",
"dimensionsTooLarge": "{{fileName}} has dimensions that are too large to preview safely.",
"invalidImage": "{{fileName}} is not a valid supported image.",
"additionalErrors": "{{count}} additional images could not be attached."
}
}
@@ -14,8 +14,21 @@ beforeEach(() => {
URL.revokeObjectURL = vi.fn()
})
function pngFile(name: string, size = 8): File {
const file = new File(['x'], name, { type: 'image/png' })
function pngHeader(width = 1, height = 1): Uint8Array<ArrayBuffer> {
const bytes = new Uint8Array(new ArrayBuffer(24))
bytes.set([137, 80, 78, 71, 13, 10, 26, 10])
const view = new DataView(bytes.buffer)
view.setUint32(8, 13)
bytes.set([73, 72, 68, 82], 12)
view.setUint32(16, width)
view.setUint32(20, height)
return bytes
}
function pngFile(name: string, size = 24, dimensions = { width: 1, height: 1 }): File {
const file = new File([pngHeader(dimensions.width, dimensions.height)], name, {
type: 'image/png'
})
Object.defineProperty(file, 'size', { value: size })
return file
}
@@ -55,7 +68,7 @@ describe('readFeedbackImageFiles', () => {
expect(errors).toEqual([])
expect(images).toHaveLength(2)
expect(new Set(images.map((image) => image.id)).size).toBe(2)
expect(images[0].data.byteLength).toBe(1)
expect(images[0].data.byteLength).toBe(24)
})
it('reports an unsupported type instead of skipping it', async () => {
@@ -98,6 +111,42 @@ describe('readFeedbackImageFiles', () => {
expect(errors).toEqual(['empty.png is empty.'])
})
it('rejects a raster that would exceed the decoded preview budget', async () => {
const file = pngFile('huge-dimensions.png', 24, { width: 8192, height: 8192 })
const { images, errors } = await readFeedbackImageFiles([file], 0)
expect(images).toEqual([])
expect(errors).toEqual([
'huge-dimensions.png has dimensions that are too large to preview safely.'
])
expect(URL.createObjectURL).not.toHaveBeenCalled()
})
it('rejects invalid raster bytes instead of mounting a broken preview', async () => {
const file = new File(['not an image'], 'broken.png', { type: 'image/png' })
const { images, errors } = await readFeedbackImageFiles([file], 0)
expect(images).toEqual([])
expect(errors).toEqual(['broken.png is not a valid supported image.'])
expect(URL.createObjectURL).not.toHaveBeenCalled()
})
it('does not count a rejected preview against the attachment limit', async () => {
const files = [
new File(['not an image'], 'broken.png', { type: 'image/png' }),
...Array.from({ length: MAX_FEEDBACK_IMAGE_COUNT }, (_, index) =>
pngFile(`valid-${index}.png`)
)
]
const { images, errors } = await readFeedbackImageFiles(files, 0)
expect(images).toHaveLength(MAX_FEEDBACK_IMAGE_COUNT)
expect(errors).toEqual(['broken.png is not a valid supported image.'])
})
it('reports the overflow once the running count is already at capacity', async () => {
const { images, errors } = await readFeedbackImageFiles(
[pngFile('a.png')],
@@ -1,5 +1,10 @@
import { translate } from '@/i18n/i18n'
import { createBrowserUuid } from './browser-uuid'
import {
INVALID_RASTER_IMAGE_PREVIEW_ERROR,
RASTER_IMAGE_PREVIEW_TOO_LARGE_ERROR,
assertRasterImagePreviewWithinLimits
} from '../../../shared/raster-image-preview-limits'
export const MAX_FEEDBACK_IMAGE_COUNT = 4
export const MAX_FEEDBACK_IMAGE_BYTES = 8 * 1024 * 1024
@@ -123,6 +128,32 @@ export async function readFeedbackImageFiles(
)
break
}
const data = new Uint8Array(await file.arrayBuffer())
try {
assertRasterImagePreviewWithinLimits(data, file.type)
} catch (error) {
if (error instanceof Error && error.message === RASTER_IMAGE_PREVIEW_TOO_LARGE_ERROR) {
addError(() =>
translate(
'auto.lib.feedback.image.attachments.dimensionsTooLarge',
'{{fileName}} has dimensions that are too large to preview safely.',
{ fileName }
)
)
continue
}
if (error instanceof Error && error.message === INVALID_RASTER_IMAGE_PREVIEW_ERROR) {
addError(() =>
translate(
'auto.lib.feedback.image.attachments.invalidImage',
'{{fileName}} is not a valid supported image.',
{ fileName }
)
)
continue
}
throw error
}
remaining -= 1
images.push({
// Why: crypto.randomUUID is undefined in non-secure browser contexts (LAN
@@ -131,7 +162,7 @@ export async function readFeedbackImageFiles(
name: fileName,
contentType: file.type,
bytes: file.size,
data: new Uint8Array(await file.arrayBuffer()),
data,
previewUrl: URL.createObjectURL(file)
})
}