From 2983cf6a3f9590590b064f103c01526c0ba92908 Mon Sep 17 00:00:00 2001 From: Yangtse Su Date: Thu, 15 Oct 2026 12:59:31 +0800 Subject: [PATCH] fix(render): budget decoder intermediates on top of the decoded image Both image decode sites handed the image crate a single budget derived from the *finished* RGBA8 raster, while `Limits::max_alloc` accounts for every allocation the decoder makes - its own sample buffer, palette and row buffers included. A picture whose raster fits the limits therefore still failed with "Memory limit exceeded" before those limits were consulted: - `ImageDecoder::decode` rejected every image whose decode needs more than one raster's worth of memory, which is why `crates/rofd-render/tests/images.rs::decoded_limits_have_exact_and_one_over_boundaries` is red on main (`decoded_limits_have_exact_and_one_over_boundaries`). - `decode_seal_raster` (stamp annotations, i.e. electronic seals) reported a decode error instead of `RasterBudgetExceeded`, and silently dropped seals that are inside the limits. Measured with the real decoder on 16-bit RGBA PNGs: a 6000x6000 seal (137 MiB raster, 274 MiB decoder buffers, both well inside `ResourceLimits::max_decoded_image_bytes` = 400 MiB) failed to decode at the old budget and decodes now; a 14336x14336 one is still rejected. Budget the decoder with three times the finished raster and keep the raster checks as the authority (the pre-decode dimension limit and the post-decode `RasterBudgetExceeded` comparison are unchanged). A decoder holds its own buffer (up to 8 bytes per pixel for 16-bit channels) and the RGBA8 conversion at the same time, so the exact boundary of the caller's budget decodes and the wall stays proportional to it. Tests: `cargo test -p rofd-render --test images` 14/14 (1/14 at 140b936), the new `cairo_stamp.rs::a_large_seal_raster_within_the_document_limits_is_not_called_over_budget` (6000x6000 16-bit seal, generated fixture, 420 KiB, ~11 s) fails before and passes after, and `cargo test -p rofd-render` is green. Verified with `cargo fmt --all -- --check` and `cargo clippy -p rofd-render --all-targets -- -D warnings`. --- crates/rofd-render/src/cairo_renderer.rs | 10 +++++- crates/rofd-render/src/images.rs | 9 +++++- crates/rofd-render/tests/cairo_stamp.rs | 30 ++++++++++++++++++ .../tests/fixtures/images/PROVENANCE.md | 30 ++++++++++++++++++ .../fixtures/images/seal-16bit-boundary.png | Bin 0 -> 430205 bytes 5 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 crates/rofd-render/tests/fixtures/images/seal-16bit-boundary.png diff --git a/crates/rofd-render/src/cairo_renderer.rs b/crates/rofd-render/src/cairo_renderer.rs index 74949987..4f514e10 100644 --- a/crates/rofd-render/src/cairo_renderer.rs +++ b/crates/rofd-render/src/cairo_renderer.rs @@ -1614,7 +1614,15 @@ fn decode_seal_raster( )) })?; let mut decoder_limits = Limits::default(); - decoder_limits.max_alloc = Some(limits.max_decoded_image_bytes.min(max_raster_bytes)); + // Two different budgets meet here. `max_decoded_image_bytes` and + // `max_raster_bytes` both bound the finished RGBA8 raster, which is only known + // after decoding, so the image crate's allocation budget (which also covers the + // decoder's own intermediate buffers) must be wider than the tighter of the two + // — the RGBA8 check below stays the authority. Three times the tighter budget: + // a decoder holds its own buffer (up to 8 bytes per pixel for 16-bit channels) + // and the RGBA8 conversion at the same time. + let raster_budget = limits.max_decoded_image_bytes.min(max_raster_bytes); + decoder_limits.max_alloc = Some(raster_budget.saturating_mul(3)); reader.limits(decoder_limits); let decoded = reader.decode().map_err(|error| { invalid_display_list(format!("seal picture could not be decoded: {error}")) diff --git a/crates/rofd-render/src/images.rs b/crates/rofd-render/src/images.rs index a1f9a9a9..fd0bf7d4 100644 --- a/crates/rofd-render/src/images.rs +++ b/crates/rofd-render/src/images.rs @@ -666,7 +666,14 @@ fn decode_pixels( let mut decoder_limits = Limits::default(); decoder_limits.max_image_width = Some(width); decoder_limits.max_image_height = Some(height); - decoder_limits.max_alloc = Some(limits.max_decoded_image_bytes); + // `Limits::max_alloc` is a running budget for every allocation the decoder + // makes, its own intermediate buffers included, while + // `max_decoded_image_bytes` bounds the finished RGBA8 buffer, which + // `validate_dimensions` has already checked. Allow three times that budget: a + // decoder holds its own buffer (up to 8 bytes per pixel for 16-bit channels) + // and the RGBA8 conversion (4 bytes per pixel) at the same time, so an image + // that exactly fills the byte budget must still decode. + decoder_limits.max_alloc = Some(limits.max_decoded_image_bytes.saturating_mul(3)); reader.limits(decoder_limits); let decoded = reader .decode() diff --git a/crates/rofd-render/tests/cairo_stamp.rs b/crates/rofd-render/tests/cairo_stamp.rs index b774ab20..ccad520d 100644 --- a/crates/rofd-render/tests/cairo_stamp.rs +++ b/crates/rofd-render/tests/cairo_stamp.rs @@ -10,6 +10,13 @@ use zip::{write::SimpleFileOptions, ZipWriter}; /// A 3x2 RGBA PNG with six distinct pixels. const PNG: &[u8] = include_bytes!("fixtures/images/asymmetric-rgba.png"); +/// A 6000x6000 16-bit RGBA PNG. Its finished RGBA8 raster is 137 MiB, well inside +/// the default document limits (400 MiB decoded bytes, 100 M pixels), but the +/// decoder holds the 274 MiB source buffer and the RGBA8 conversion at the same +/// time, which the old single-budget accounting rejected ("Memory limit exceeded") +/// before those limits were ever consulted - so the seal was silently dropped. +const LARGE_PNG: &[u8] = include_bytes!("fixtures/images/seal-16bit-boundary.png"); + fn der(tag: u8, content: &[u8]) -> Vec { let mut out = vec![tag]; if content.len() < 0x80 { @@ -193,6 +200,29 @@ fn ofd_stamp_renders_the_mini_document_page() { assert_eq!(pixel(&mut surface, 14, 8), [255, 255, 255, 255]); } +#[test] +fn a_large_seal_raster_within_the_document_limits_is_not_called_over_budget() { + // The stamp raster decode must not report a budget error for a picture that + // fits the document's decoded-image limits: the image crate's allocation + // budget covers its own intermediate buffers on top of the finished RGBA8 + // raster, so budgeting it with the raster size alone rejects pictures that + // are well inside the limits. A broken picture still leaves the page blank + // (see the next test); this one distinguishes a decode failure from a budget + // failure by pixel colour. + let mut surface = render( + r#""#, + "png", + LARGE_PNG, + ); + let inside = pixel(&mut surface, 4, 4); + assert_eq!(inside[3], 255, "stamp pixel stayed transparent: {inside:?}"); + assert_eq!( + (&inside[0], &inside[1], &inside[2]), + (&0x20, &0x40, &0x60), + "stamp pixel is not the fixture's colour: {inside:?}" + ); +} + #[test] fn broken_or_unsupported_stamps_are_skipped_silently() { let mut surface = render( diff --git a/crates/rofd-render/tests/fixtures/images/PROVENANCE.md b/crates/rofd-render/tests/fixtures/images/PROVENANCE.md index 8b610176..59cd3dfc 100644 --- a/crates/rofd-render/tests/fixtures/images/PROVENANCE.md +++ b/crates/rofd-render/tests/fixtures/images/PROVENANCE.md @@ -32,3 +32,33 @@ b98b78b351b1511a0b9c5a953334bf2e801f3bf8c6922173d4590613bdb9f9a8 asymmetric-rgb 86367e4786fe0911e54a3c7567e1cbd1f68557de199b1f72ffcf43b184c27982 asymmetric-rgb.gif e6b956ad3682993147602bda9ff31c4a6755ebf7cf5b4e0f9015069f240e93ec asymmetric-rgb.tiff ``` + +`seal-16bit-boundary.png` is a programmatically generated 6000 by 6000 16-bit +RGBA test image, filled with the single opaque colour `#204060` at 16-bit +precision (every 16-bit channel is the 8-bit value repeated). It contains no +third-party artwork or other licensed material. It was generated with CPython +3.14 in a few lines (a zero filter byte per row, `zlib.compress` at level 9, and +`zlib.crc32` for the PNG chunks), with no image library involved: + +```text +python3 - <<'EOF' +import struct, zlib +w = h = 6000 +raw = bytearray() +for _ in range(h): + raw.append(0) + raw.extend(b'\x20\x20\x40\x40\x60\x60\xff\xff' * w) +chunk = lambda t, d: struct.pack('>I', len(d)) + t + d + struct.pack('>I', zlib.crc32(t + d) & 0xffffffff) +open('seal-16bit-boundary.png', 'wb').write( + b'\x89PNG\r\n\x1a\n' + + chunk(b'IHDR', struct.pack('>IIBBBBB', w, h, 16, 6, 0, 0, 0)) + + chunk(b'IDAT', zlib.compress(bytes(raw), 9)) + + chunk(b'IEND', b'')) +EOF +``` + +SHA-256 checksums: + +```text +264bfb51e016bb80c612db7dbab8d6070b91e5b4b4b0f8f7f8a2012842496c55 seal-16bit-boundary.png +``` diff --git a/crates/rofd-render/tests/fixtures/images/seal-16bit-boundary.png b/crates/rofd-render/tests/fixtures/images/seal-16bit-boundary.png new file mode 100644 index 0000000000000000000000000000000000000000..66dbd20bfd558ba86249a4d0c7fb3555dbdd9ef1 GIT binary patch literal 430205 zcmeI*&#PzmS-|m&3?Yp+HM%ee#WoCaTUW)3B{d_RK~qcHjS5oGh0sM^Sdb#r4wRUM z5C+^B@&_nb5QVO+r3&K8g2jbhghDnJS8*XCwtkzNIrn*W!?e9qh-W9|zQc+Pij zW=`Jc{rouh!#wwuPk!R#?|tg0pE@}?dGEcucRqD;@>9R}!+(GLiNhy<{SUu+a`MEh zckbQ!*k}IW-~QuUKljWZy>N2tum1F{PyNF-+KLzKlAVZ^7H@o(SQD*2eQ3hiI_T`pChl;DJ6iv3k}Rok}FC zgbq5Rowt(WP9-9s4+T~2zNRn=%CYeSai_Y zBw#q*M03a+bkK?IbtdKxcV>KvZJImWxdwN-V}_f!L~8sIpcBhijnSz@qFLym)3rNv z&`AVrI88W8exQR+Y_Bshcepd-ODsLy;SP5q0WUVOKGYbUN`yNd5p>Y$IxITqY!Y~^ zhSTF?2HWB`pWGp>(LpD6|CyLO+?nwuP7&PU4tEj(KNKb5DEWa7Id-o65&oq1RZp`4vP*tn*$}xWk=Dz?;Nzl{otZoii5En$s8^bh>_m z4myc|@hjmd`Ej%9%pNn?rPsNH4m!sJ+t?@E;m(*TrGXtTxx*dqBm$O%Cmba|&_U<8 zjf{Q59qx>oQi{&uk~`etP9$J-KdusIpP+NbB3g4Aqk~S@PtZXp5xBPTYwVce61O%b z3|(G?D(iMSYUrR7l4UTbPQQ#h+~H0lV25YIQSt*Fbf!8?_+svGC+JWrOMl57?roQi{&uk~`etP9)&P=D13nee&4SIX-7NyXp9N$70*! zy0VB4I%9csoP*rq4tElPYuhxK?qbo>?W}V-1%wVd$6amg6Yg+l%#@`VF9CPB!<|UL zo5WHaRswWZpe*{JgHG2^&_O2=Fn%Q*B|mNsor^eTu=ucZ2_1BnIc+lF4tFM%Svnba zxWk=9z)I1CqvQuV=q%l1GT;t(CY4#L4|ll3ok+mwz7&U*0G$;mi$3U})AbW{&`AWY zWc=EH%wW@CdRirY(LrZv{gVNAxHGBDQhm6?9quFoc6cTnB|p$XXXzf30e84FsmxM+ zxWgUpL;^U82uhbJ{1k!yWEK0-D4!L9PVotUy`xK?j|#pP++IB4GSV zI7)t?^RA{dcFb^dmPjpQ#6*Bj2MZl^N>3TSm;ALyV{I_&Tzcepbwi@MXvg3cAu>5dt$WNXv-|EJsOiVHgE zl&U#=$sO(t%Tfw9ceulyNWga7QX*V7ImkS1)ZBt=lGc6inlfu1&=P>PFL&DLFXuC25TC3xWk=Dz=qSVbfAMyS258+ zClN4UBpf9_&_O2-A2Ts`xHIEREIr)e4tF8}FE+70)EJ#gggYIPbvnoA45PnC{QVxr zR%~@W3>|bHX3A`qafdtHi3DuN?P>@*=yVkm9dr@_14hD8@&g@o;_xvObB8-KzQoeQ z9qw=^67XUZ>qCvvsYJNb5xFWl7jevBTU>lvHAbfriIW35=#19EN{Ty`h|KDUpo7ld zTEiXgaA%8vAKeh^Lygg?M50INpflQADk<(%A~LHZf(|-+YYlg}!<|h6{Ws(GA2V29 z7Yk2~(WykDpy;48+O#Su?o=W&t0RIAI(uskceulyEdqX$M63@rMyC>q9-)KIXm6>c zxKoM9td0me=Ym zTWh$(9qw!qu&g)MhZ>_(iA0akL1(nLR8rijL}XS+1RZqt)*9|`hdY}D=D!&?bj)Dm zSC@6@ptG5g9hyTk<4y+%9du+?B4Al>!cp=A9dzO=;!MmP?#%cSOAmLr!<|UL=pO4s zjnSz@xYH3q2c53NqJz#RfyZGuZH^f%KHPkAhqOiqo!I?nV(xHf#+NunaECkGNd)?} zkm*agGmIk^9rI2n4m#-UO)KtjhdY&kj$VnxEI|jIGMx=ya)&#^vc#fuh`Ga^o$eeR zGgy4sxr7cnJ2UMbceulyL|}BaUnkCC{*hT7Aau}4S=ZqxZx-C?V4;JK%t{1w^n|12 z2Ri7K4m*6w9qtUvqV9CEpo7j!w~7V(E>-1Dr87E5=L~yG+1rn4erwa})3t}OdwA%e zvo}k*!yWD<0(w@$QSt*FbXI4yfe*REodHcs#W}s?4tKZ{2^ig13&lGhqJvIXG0{OM z5inpR93?-{K_?C$Gx3@`7jVpA(_qQbbcs&FQG7)QovxsxgH9swIBXgWHO%d#r&H1u z9dtsn6g&D$?r?`Yk$_pbs~G5@(^X7#&`AUg7zszo4|LFp!^cd_9qtrgb{{iX65hFl z4mu^%hA+9poncwZbjTgM4QZ(Tx`GF2PrH2e(a)&#^vZyo(LpCx@R^u9+?nwuR)6krhdYVD*;>d>jhIBM zvCQi5p@UA!x(-LlFYa`(&_PFLB?3Bn!cp=A9dt^E9lqoacZOwAcRE?nS*J5}%wX~1 zI=oY<&_QQ!mF5n2xRVI1t@bP3d*o7P$*km6hocC^oemZ{=*X-@K+j4zN`9b&PU*12 zm)zmbuq^6MCks00Jb%AfukOh)#3NV8zxEtERA{xDyF@v5EDe#^_Wc-06rs#&nL3 z87!}hPpii0R3dS5KnI=CI#@|@rxKA_9T9ZU*;{M4!yWEy5wMOn)`uFSQ;9^6&_QRk zw^UNxsYGN}M+6;o_SPEiaECja1TOxYaYyG2qyEKAeHdFQy7}Y|SkXb}VW!Mx8F#qD zokYM&(S)Pq2Ri7?HkpTC&mHbObfJ`$b1%8W9qvQ|M)$60po30VG0{OM5inpR93?-{ zK_?C$Gx24*a{#*pcvq`{kx{2nHH|U@f+v`ls9q!Ed65BL) zxO3_5Odc~_{MM$u|A%j9ireY%p@U9rC^Ip4xHIERY}MT14tF8}d#7W4s4+T~2zNRn z=%CYeSai_YBw#q*M03a+bkK?IbtdKxcV>KvZJImpZg+-`87^~cQ-?Q-7-F~6WivYH zgk+h@FYa)MJCT6h!Ce7C2c52BqJvH%V8BQ?N`9b&P8>dFV(xHf#+O)nxWgUpL;_xH zVtuGFI+X}_IwB8lH^&TDv$d(ez3{7UC%Rl?bSjZJIiQ11C?!#ib7SsshdYsg&B|TT zKnIApPvr)_NGS68LcL8nchCI`909qvQ|7R)#0;x;qCvvsYJNb5kUu?uEU~(&L#oF=_Z;(-k^g{Y_Bshcepd-OKj8J z;SP5qflGZUzJOx}tNr5Bsxdm1NUU$@pcA|QOw1kb%=n`2biSa2&faX|4tKb-MZhZK zSRZPPP9+jOLI<7E-cm_%rxKA_9T9ZU*;{M4!=28Z-Ny`;*TvMTF*=n=W*AXUr6Jr!xf| zbPfdk1e>mypo7kFp&a{!JKPyF<+u@XhdbPf1ibbgSBbMv&^coftvQX+L8t2{=v+6Q zdd%>j&)?sBODX+_w>EVOJh*KSWA`-BK__KhhodAKcRE<;pd+&q0UbTzDEWa7I;F!7 zUvh^#!?LJ5oh;~}<2x8lG4WzkN<_LuC*dd_p@Ysudr7dklW^R)%&O-MFYXoTK+dMY z&Lwov*_&M4;SP5afot3}ScZhRFWpXhIwf7vL1!&%iYBX<+~E#)A_3FjYN2@NLv+yT zDkeJUBmxGEgrnpKI_SjVVL1(5i$0pAm?rIYU#`!Ggz_JJsot=iT!OR<_>pee2IOMJKW(;B4FQr!cp=A z9du#?nTff>of%(Z>ERA{xDyE&-D7>IF*=n9cRC{Ipwo3&bgqle$T5QzTiw$^2c6j8 zW@7GeXU3Px2mV{|GJ?sP=ZL8t4m=%BMnz;L>W=8!k&pcC8cOw1kb z%=i-9GW3}5`gx4-tIU;gIVkAVKyS3a=!mZI9f^ZJwBvyETc#%`y38t9mJWzpwy={))|!`ClnyV9GlUCeeRx3ieD5}>mJW$D)FpwpFNbkK2ppl8@Zs4tF8}J*$+6bcs&F5yd&g z=%CZ}6Lio?1dLw^N68O#&?y~u_>w!^8J0!e>106%9pA`piph-1DC3Miyvm$}({FcGMF~j9`HCSKKS#fD$hYmV%ke!LS!<`vlmZEbAxWgUpBm#B^Cmba|&_O3QkeQe} z+?nwumLBeKhdYsg(LL6O8lzK*aHk`J4mw?jy>dE#^8Sz9J~=u0?6ZIK{XhTw&-}<& zK7Q*WZtu3mhcVM@j7}vIr9%gu(K=X3aiuB5LKqpqu z8lzK*#4JMxowm3{SxU?u?rP%@{Ob%I-s0lJOZ(8eM}Q7G z4>M&pxwyj}?nDCDxoL1Ed)>|ol;wG$gHBh#(LpB>Fz+NBB|p$XXX&kz0e84FsmxM+ zxWgUpL;_xHmg2AyptAyH(dS0ddE{dTYau(A&_So0P0&Fn5wO}X;VAim4m!)6HW_e- zJCn*R)rULW;Z7vr#bzlED*-wyP!@gAL8t2{=%AAb7{3yZk{{@xv-H-<;Huo&dCXv2 zT&FfV=uGDO(mJ@q9quFoRv9N8B|p$XXXzf30e84FsmxM+xWgUpL;^p!0aqnL1{$+OK4mw>wK?j{g!1$GLl>9*F2GV)o fr~l$tKKc5ue)09g_cTxL{qiU7eD#+;d;k9cf`Hl| literal 0 HcmV?d00001