From a476170fca86fffba283cf88ce12101b5aa81be3 Mon Sep 17 00:00:00 2001 From: timmy Date: Fri, 14 Aug 2026 16:35:21 +0000 Subject: [PATCH] feat: bound decoded screenshot pixels (Closes #837) --- frontend/issue-attachment.js | 9 ++-- frontend/issue-evidence-review.js | 24 ++++++--- tests/test_issue_attachment_ui.py | 85 ++++++++++++++++++++++++++++--- 3 files changed, 102 insertions(+), 16 deletions(-) diff --git a/frontend/issue-attachment.js b/frontend/issue-attachment.js index af00571..9600596 100644 --- a/frontend/issue-attachment.js +++ b/frontend/issue-attachment.js @@ -8,6 +8,7 @@ 'use strict'; const MAX_BYTES = 2 * 1024 * 1024; + const MAX_PIXELS = issueEvidenceReview.MAX_PIXELS; const MAX_FILES = 5; const MAX_FILES_MESSAGE = 'Up to 5 screenshots. Remove one before adding another.'; const IMAGE_TYPES = new Set(['image/png', 'image/jpeg', 'image/webp']); @@ -38,6 +39,7 @@ function create(options) { const upload = options.upload; const maxFiles = options.maxFiles === MAX_FILES ? MAX_FILES : 1; + const inspectPixels = options.inspectPixels === true; const optimizeSelectedImage = options.optimizeImage || optimizeImage; const createOperationId = options.createOperationId || (() => { if (typeof globalThis !== 'undefined' && globalThis.crypto?.randomUUID) { @@ -72,7 +74,7 @@ if (!Number.isFinite(file.size) || file.size <= 0) { throw new Error('Choose a screenshot that is 2 MB or smaller.'); } - if (file.size > MAX_BYTES) { + if (file.size > MAX_BYTES || inspectPixels) { return Promise.resolve(optimizeSelectedImage(file)).then(optimized => { if (generation !== selectionGeneration) return state(); return commitSelection(optimized); @@ -219,7 +221,8 @@ function mount(options) { const controller = create({ - ...options, maxFiles:options.maxFiles || (options.input?.multiple ? MAX_FILES : 1), + ...options, inspectPixels:true, + maxFiles:options.maxFiles || (options.input?.multiple ? MAX_FILES : 1), }); const clearSelection = controller.clear; const restoreSelection = controller.restore; @@ -376,5 +379,5 @@ return Object.assign(controller, { clear: clearPreview, restore: restorePreview, setBusy }); } - return { create, mount, multipart, optimizeImage, MAX_BYTES }; + return { create, mount, multipart, optimizeImage, MAX_BYTES, MAX_PIXELS }; }); diff --git a/frontend/issue-evidence-review.js b/frontend/issue-evidence-review.js index 71c3f25..57b4827 100644 --- a/frontend/issue-evidence-review.js +++ b/frontend/issue-evidence-review.js @@ -6,6 +6,8 @@ 'use strict'; const MAX_BYTES = 2 * 1024 * 1024; + const MAX_PIXELS = 12 * 1024 * 1024; + const MAX_DIMENSION = 8192; function namedBlob(blob, name) { if (typeof File === 'function') return new File([blob], name, { type: blob.type }); @@ -14,20 +16,28 @@ } async function optimizeImage(file, environment = {}) { - if (file.size <= MAX_BYTES) return file; const decode = environment.createImageBitmap || globalThis.createImageBitmap; const makeCanvas = environment.createCanvas || (() => document.createElement('canvas')); - if (typeof decode !== 'function') throw new Error('This browser cannot optimize the screenshot. Try cropping it and choose it again.'); + if (typeof decode !== 'function') throw new Error('This browser cannot safely inspect the screenshot. Try cropping it and choose it again.'); let bitmap; try { bitmap = await decode(file); + const width = Number(bitmap.width); + const height = Number(bitmap.height); + if (!Number.isSafeInteger(width) || !Number.isSafeInteger(height) || width <= 0 || height <= 0) { + throw new Error('decode'); + } + const pixelScale = Math.min(1, Math.sqrt(MAX_PIXELS / (width * height))); + const dimensionScale = Math.min(1, MAX_DIMENSION / Math.max(width, height)); + const byteScale = file.size > MAX_BYTES ? Math.sqrt(MAX_BYTES / file.size) * 0.92 : 1; + let scale = Math.min(pixelScale, dimensionScale, byteScale); + if (scale === 1 && file.size <= MAX_BYTES) return file; const canvas = makeCanvas(); const context = canvas && canvas.getContext && canvas.getContext('2d'); - if (!context || !bitmap.width || !bitmap.height) throw new Error('decode'); - let scale = Math.min(1, Math.sqrt(MAX_BYTES / file.size) * 0.92); + if (!context) throw new Error('decode'); for (let attempt = 0; attempt < 10; attempt += 1) { - canvas.width = Math.max(1, Math.round(bitmap.width * scale)); - canvas.height = Math.max(1, Math.round(bitmap.height * scale)); + canvas.width = Math.max(1, Math.floor(width * scale)); + canvas.height = Math.max(1, Math.floor(height * scale)); context.drawImage(bitmap, 0, 0, canvas.width, canvas.height); const blob = await new Promise(resolve => canvas.toBlob(resolve, file.type, file.type === 'image/png' ? undefined : 0.86)); if (!blob) throw new Error('encode'); @@ -172,5 +182,5 @@ return { activeIndex: () => activeIndex, afterRemoval, clear, render, setBusy }; } - return { create, optimizeImage }; + return { create, optimizeImage, MAX_PIXELS }; }); diff --git a/tests/test_issue_attachment_ui.py b/tests/test_issue_attachment_ui.py index 8348f35..74befbf 100644 --- a/tests/test_issue_attachment_ui.py +++ b/tests/test_issue_attachment_ui.py @@ -280,6 +280,74 @@ const canvas = {{ assert output["name"] == "camera.jpg" +def test_browser_optimizer_downscales_small_file_when_decoded_pixels_exceed_budget(): + script = f""" +const attachment = require({json.dumps(str(ATTACHMENT))}); +const original = new Blob(['compressed'], {{type:'image/jpeg'}}); +original.name = 'panorama.jpg'; +let closed = false; +const attempts = []; +const canvas = {{ + width: 0, height: 0, + getContext: () => ({{drawImage: () => {{}}}}), + toBlob: callback => {{ + attempts.push([canvas.width, canvas.height]); + callback(new Blob(['bounded'], {{type:'image/jpeg'}})); + }}, +}}; +(async()=>{{ + const result = await attachment.optimizeImage(original, {{ + createImageBitmap: async () => ({{width:6000,height:4000,close:()=>{{closed=true;}}}}), + createCanvas: () => canvas, + }}); + process.stdout.write(JSON.stringify({{ + attempts,closed,size:result.size,type:result.type,name:result.name, + maxPixels:attachment.MAX_PIXELS, + }})); +}})().catch(error=>{{console.error(error);process.exit(1);}}); +""" + output = json.loads(run_node(script)) + + assert len(output["attempts"]) == 1 + width, height = output["attempts"][0] + assert width * height <= output["maxPixels"] + assert width < 6000 + assert height < 4000 + assert output["closed"] is True + assert output["size"] == 7 + assert output["type"] == "image/jpeg" + assert output["name"] == "panorama.jpg" + + +def test_mobile_preview_inspects_small_screenshot_before_rendering_it(): + script = f""" +const attachment = require({json.dumps(str(ATTACHMENT))}); +class Element{{constructor(){{this.listeners={{}};this.hidden=true;this.value='';this.files=[];this.textContent='';this.src='';this.disabled=false;}}addEventListener(t,f){{this.listeners[t]=f;}}dispatch(t){{return this.listeners[t]({{target:this}});}}}} +const input=new Element(),preview=new Element(),image=new Element(),meta=new Element(),remove=new Element(),status=new Element(); +const original=new Blob(['compressed'],{{type:'image/jpeg'}}); original.name='panorama.jpg'; +const bounded=new Blob(['bounded'],{{type:'image/jpeg'}}); bounded.name='panorama.jpg'; +const inspected=[]; +attachment.mount({{ + input,preview,image,meta,remove,status, + optimizeImage:async file=>{{inspected.push(file.name);return bounded;}}, + createObjectURL:blob=>blob===bounded?'blob:bounded':'blob:unsafe',revokeObjectURL:()=>{{}},upload:async()=>{{}}, +}}); +input.files=[original]; +(async()=>{{ + await input.dispatch('change'); + process.stdout.write(JSON.stringify({{inspected,src:image.src,hidden:preview.hidden,status:status.textContent}})); +}})().catch(error=>{{console.error(error);process.exit(1);}}); +""" + output = json.loads(run_node(script)) + + assert output == { + "inspected": ["panorama.jpg"], + "src": "blob:bounded", + "hidden": False, + "status": "Screenshot optimized and ready to upload.", + } + + def test_binary_screenshot_builds_multipart_body_with_original_bytes(): script = f""" const attachment=require({json.dumps(str(ATTACHMENT))}); @@ -348,6 +416,7 @@ input.multiple=true; const urls=[]; const controller=attachment.mount({{input,preview,image,meta,remove,status,tray,earlier,later,document, removedMessage:'Screenshot removed. Your issue draft is unchanged.', + optimizeImage:async file=>file, createObjectURL:blob=>{{const url='blob:'+blob.name;urls.push(url);return url;}},revokeObjectURL:()=>{{}},upload:async()=>{{}}}}); const file=name=>{{const blob=new Blob([name],{{type:'image/png'}});blob.name=name;return blob;}}; input.files=['one.png','two.png','three.png'].map(file); @@ -393,6 +462,7 @@ const document={{createElement:tag=>new Element(tag)}}; const input=new Element('input'),preview=new Element(),image=new Element('img'),meta=new Element(),remove=new Element('button'),status=new Element(),tray=new Element(),earlier=new Element('button'),later=new Element('button'),note=new Element('textarea'),noteLabel=new Element('label'); input.multiple=true; const controller=attachment.mount({{input,preview,image,meta,remove,status,tray,earlier,later,note,noteLabel,document, + optimizeImage:async file=>file, createObjectURL:blob=>'blob:'+blob.name,revokeObjectURL:()=>{{}},upload:async()=>{{}}}}); const file=name=>{{const blob=new Blob([name],{{type:'image/png'}});blob.name=name;return blob;}}; input.files=['one.png','two.png','three.png'].map(file); @@ -529,15 +599,18 @@ const input=new Element(), preview=new Element(), image=new Element(), meta=new const revoked=[]; const controller=attachment.mount({{ input,preview,image,meta,remove,status, + optimizeImage:async file=>file, createObjectURL:()=> 'blob:preview', revokeObjectURL:url=>revoked.push(url), readDataUrl:async()=>'', upload:async()=>{{}}, }}); -input.files=[{{name:'payload.svg',type:'image/svg+xml',size:20}}]; input.dispatch('change'); -const invalid={{message:status.textContent,hidden:preview.hidden}}; -input.files=[{{name:'screen.png',type:'image/png',size:2048}}]; input.dispatch('change'); -const selected={{src:image.src,meta:meta.textContent,hidden:preview.hidden,state:controller.state()}}; -remove.dispatch('click'); -process.stdout.write(JSON.stringify({{invalid,selected,removed:{{hidden:preview.hidden,src:image.src,revoked,state:controller.state()}}}})); +;(async()=>{{ + input.files=[{{name:'payload.svg',type:'image/svg+xml',size:20}}]; await input.dispatch('change'); + const invalid={{message:status.textContent,hidden:preview.hidden}}; + input.files=[{{name:'screen.png',type:'image/png',size:2048}}]; await input.dispatch('change'); + const selected={{src:image.src,meta:meta.textContent,hidden:preview.hidden,state:controller.state()}}; + await remove.dispatch('click'); + process.stdout.write(JSON.stringify({{invalid,selected,removed:{{hidden:preview.hidden,src:image.src,revoked,state:controller.state()}}}})); +}})().catch(error=>{{console.error(error);process.exit(1);}}); """ output = json.loads(run_node(script)) assert output["invalid"] == {