fix: slide images keep their shape, and only safe parsers measure them
Some checks failed
Forgejo Android APK / Root app tests (push) Successful in 50s
Forgejo Docker Build / Root app tests (push) Successful in 47s
Forgejo Android APK / Build signed APK (push) Successful in 2m13s
Forgejo Docker Build / Build Docker image (push) Successful in 23s
Forgejo Docker Build / Deploy to the host (push) Failing after 0s
Some checks failed
Forgejo Android APK / Root app tests (push) Successful in 50s
Forgejo Docker Build / Root app tests (push) Successful in 47s
Forgejo Android APK / Build signed APK (push) Successful in 2m13s
Forgejo Docker Build / Build Docker image (push) Successful in 23s
Forgejo Docker Build / Deploy to the host (push) Failing after 0s
Every generated image in an exported deck was distorted. PowerPoint scales an image to whatever extent it is given, and the export handed it the content box verbatim. pptxgenjs has a `sizing: contain` option that looks like it solves this; reading the emitted slide XML shows it does not — a 200x800 image in an 11.8x3.9 box came out as cx=10789920 cy=3566160 with <a:stretch/>, stretched from 1:4 to 3:1. It cannot do better: it never measures the image, and its own getSizeFromImage is commented out and marked "currently unused". So the export measures the image itself and hands PowerPoint an extent that already has the right shape, centred in the space available. Verified: a 200x800 image now places 0.97x3.90 and a 4x3 places 5.20x3.90, both matching their source aspect exactly, neither overflowing. An image that cannot be measured keeps the old behaviour rather than failing the export. image-size becomes a real dependency rather than one borrowed transitively, and an override collapses it to a single copy — pptxgenjs declares it but the string appears in none of its four shipped bundles, so npm was placing a second copy in the production image that nothing could load. Its ICNS, JXL and HEIF parsers have open denial-of-service advisories against every published version (GHSA-w3rx-r6r6-pgpr, GHSA-5p2g-fcmc-qvqq, both ranged <=2.0.2), so there is no release to upgrade to. They are disabled instead: this application measures PNG, JPEG, WebP and GIF and nothing else. An ICNS buffer is now refused and falls back to the box rather than entering the parser. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU
This commit is contained in:
parent
b3d66caaca
commit
61da9e6bf2
4 changed files with 111 additions and 17 deletions
19
package-lock.json
generated
19
package-lock.json
generated
|
|
@ -26,6 +26,7 @@
|
|||
"express": "^4.21.0",
|
||||
"express-rate-limit": "^7.4.0",
|
||||
"helmet": "^8.0.0",
|
||||
"image-size": "^2.0.2",
|
||||
"jsonwebtoken": "^9.0.2",
|
||||
"katex": "^0.18.7",
|
||||
"mammoth": "^1.8.0",
|
||||
|
|
@ -5122,13 +5123,10 @@
|
|||
}
|
||||
},
|
||||
"node_modules/image-size": {
|
||||
"version": "1.2.1",
|
||||
"resolved": "https://registry.npmjs.org/image-size/-/image-size-1.2.1.tgz",
|
||||
"integrity": "sha512-rH+46sQJ2dlwfjfhCyNx5thzrv+dtmBIhPHk0zgRUukHzZ/kRueTJXoYYsclBaKcSMBWuGbOFXtioLpzTb5euw==",
|
||||
"version": "2.0.2",
|
||||
"resolved": "https://registry.npmjs.org/image-size/-/image-size-2.0.2.tgz",
|
||||
"integrity": "sha512-IRqXKlaXwgSMAMtpNzZa1ZAe8m+Sa1770Dhk8VkSsP9LS+iHD62Zd8FQKs8fbPiagBE7BzoFX23cxFnwshpV6w==",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"queue": "6.0.2"
|
||||
},
|
||||
"bin": {
|
||||
"image-size": "bin/image-size.js"
|
||||
},
|
||||
|
|
@ -6741,15 +6739,6 @@
|
|||
"node": ">=10.13.0"
|
||||
}
|
||||
},
|
||||
"node_modules/queue": {
|
||||
"version": "6.0.2",
|
||||
"resolved": "https://registry.npmjs.org/queue/-/queue-6.0.2.tgz",
|
||||
"integrity": "sha512-iHZWu+q3IdFZFX36ro/lKBkSvfkztY5Y7HMiPlOUjhupPcG2JMfst2KKEpu5XndviX/3UhFbRngUPNKtgvtZiA==",
|
||||
"license": "MIT",
|
||||
"dependencies": {
|
||||
"inherits": "~2.0.3"
|
||||
}
|
||||
},
|
||||
"node_modules/range-parser": {
|
||||
"version": "1.2.1",
|
||||
"resolved": "https://registry.npmjs.org/range-parser/-/range-parser-1.2.1.tgz",
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@
|
|||
"express": "^4.21.0",
|
||||
"express-rate-limit": "^7.4.0",
|
||||
"helmet": "^8.0.0",
|
||||
"image-size": "^2.0.2",
|
||||
"jsonwebtoken": "^9.0.2",
|
||||
"katex": "^0.18.7",
|
||||
"mammoth": "^1.8.0",
|
||||
|
|
@ -76,7 +77,8 @@
|
|||
},
|
||||
"speech-rule-engine": {
|
||||
"@xmldom/xmldom": "0.9.12"
|
||||
}
|
||||
},
|
||||
"image-size": "^2.0.2"
|
||||
},
|
||||
"engines": {
|
||||
"node": "24.x"
|
||||
|
|
|
|||
|
|
@ -551,6 +551,54 @@ router.post('/webdav-path', require('../utils/policy').requireFeature('nextcloud
|
|||
});
|
||||
|
||||
// ── POST /api/admin/learning/generate-pptx ──────────────────
|
||||
// Fit an image inside a box without distorting it.
|
||||
//
|
||||
// PowerPoint scales an image to whatever extent it is given, so passing the box
|
||||
// straight through stretches anything whose aspect ratio differs — which is
|
||||
// every generated image, since they are square or portrait and the content area
|
||||
// is wide. pptxgenjs offers a `sizing` option that looks like it solves this,
|
||||
// but it emits the box dimensions unchanged with <a:stretch/>; it cannot do
|
||||
// better, because it never measures the image.
|
||||
//
|
||||
// So measure it here and hand PowerPoint an extent that already has the right
|
||||
// shape, centred in the space available. An image that cannot be measured keeps
|
||||
// the old behaviour rather than failing an export.
|
||||
// The parsers this application will run, and only these.
|
||||
//
|
||||
// image-size ships twenty formats. Three of them — ICNS, JXL and HEIF — have
|
||||
// open denial-of-service advisories against every published version
|
||||
// (GHSA-w3rx-r6r6-pgpr, GHSA-5p2g-fcmc-qvqq): a crafted file spins forever in
|
||||
// the parser. There is no fixed release to upgrade to, so the answer is not to
|
||||
// run them. Slide images come from the generator as PNG, JPEG or WebP.
|
||||
//
|
||||
// Done once, at load, rather than per call: disableTypes replaces the enabled
|
||||
// set, so calling it repeatedly would be wasted work and easy to get wrong.
|
||||
var imageSizeReady = false;
|
||||
function measurer() {
|
||||
var lib = require('image-size');
|
||||
if (!imageSizeReady) {
|
||||
var keep = { png: 1, jpg: 1, webp: 1, gif: 1 };
|
||||
lib.disableTypes((lib.types || []).filter(function (t) { return !keep[t]; }));
|
||||
imageSizeReady = true;
|
||||
}
|
||||
return lib.imageSize;
|
||||
}
|
||||
|
||||
function fitImage(bytes, x, y, maxW, maxH) {
|
||||
var fallback = { x: x, y: y, w: maxW, h: maxH };
|
||||
if (!bytes || !bytes.length) return fallback;
|
||||
try {
|
||||
var dims = measurer()(bytes);
|
||||
if (!dims || !dims.width || !dims.height) return fallback;
|
||||
var scale = Math.min(maxW / dims.width, maxH / dims.height);
|
||||
var w = dims.width * scale;
|
||||
var h = dims.height * scale;
|
||||
return { x: x + (maxW - w) / 2, y: y + (maxH - h) / 2, w: w, h: h };
|
||||
} catch (e) {
|
||||
return fallback;
|
||||
}
|
||||
}
|
||||
|
||||
// Convert Marp markdown to PPTX using pptxgenjs (no browser needed)
|
||||
|
||||
router.post('/generate-pptx', async function(req, res) {
|
||||
|
|
@ -559,9 +607,15 @@ router.post('/generate-pptx', async function(req, res) {
|
|||
if (!markdown) return res.status(400).json({ error: 'markdown required' });
|
||||
|
||||
var assetImages = {};
|
||||
// Kept alongside the data URI so the image can be measured. pptxgenjs cannot
|
||||
// do it: its sizing option emits the box dimensions verbatim with
|
||||
// <a:stretch/>, so a portrait image handed to a landscape box comes out
|
||||
// stretched. Its own getSizeFromImage is commented out and marked unused.
|
||||
var assetImageBytes = {};
|
||||
for (const id of require('../utils/generatedImageLinks').references(markdown)) {
|
||||
const image = await require('../utils/generatedImages').service().asset(id, req.user);
|
||||
assetImages['/api/generated-images/' + id] = 'data:' + image.mime + ';base64,' + image.bytes.toString('base64');
|
||||
assetImageBytes['/api/generated-images/' + id] = image.bytes;
|
||||
}
|
||||
var PptxGenJS = require('pptxgenjs');
|
||||
var pptx = new PptxGenJS();
|
||||
|
|
@ -655,7 +709,8 @@ router.post('/generate-pptx', async function(req, res) {
|
|||
if (type === 'image') {
|
||||
var src = line.trim().match(/\(([^)]+)\)$/)[1];
|
||||
if (!assetImages[src]) throw new Error('Generated image unavailable');
|
||||
slide.addImage({ data: assetImages[src], x: 0.5, y: contentY, w: 11.8, h: Math.max(0.5, 5.2 - contentY), sizing: { type: 'contain', w: 11.8, h: Math.max(0.5, 5.2 - contentY) } });
|
||||
var box = fitImage(assetImageBytes[src], 0.5, contentY, 11.8, Math.max(0.5, 5.2 - contentY));
|
||||
slide.addImage({ data: assetImages[src], x: box.x, y: box.y, w: box.w, h: box.h });
|
||||
contentY = 5.2; i++; continue;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -285,3 +285,51 @@ test('citation quality is measured on the server, where answer and sources both
|
|||
assert.match(route, /if \(!tracker\) return;/);
|
||||
assert.match(route, /tracker\.store\(req\.user\.id, question, result, sources\);/);
|
||||
});
|
||||
|
||||
test('PPTX images keep their shape', () => {
|
||||
// PowerPoint scales an image to whatever extent it is given. pptxgenjs offers
|
||||
// a `sizing` option that looks like it handles this, but it emits the box
|
||||
// dimensions unchanged with <a:stretch/> — verified by reading the slide XML:
|
||||
// a 200x800 image in an 11.8x3.9 box came out as cx=10789920 cy=3566160,
|
||||
// i.e. stretched from 1:4 to 3:1. It cannot do better, because it never
|
||||
// measures the image; its own getSizeFromImage is commented out as unused.
|
||||
const src = read('src/routes/learningAI.js');
|
||||
assert.match(src, /function fitImage\(bytes, x, y, maxW, maxH\)/);
|
||||
assert.match(src, /var dims = measurer\(\)\(bytes\);/, 'measured here instead');
|
||||
assert.match(src, /var scale = Math\.min\(maxW \/ dims\.width, maxH \/ dims\.height\);/,
|
||||
'scaled to fit inside the box on whichever axis binds first');
|
||||
assert.match(src, /x: x \+ \(maxW - w\) \/ 2, y: y \+ \(maxH - h\) \/ 2/, 'and centred in it');
|
||||
// An image that cannot be measured must not fail the export.
|
||||
assert.match(src, /var fallback = \{ x: x, y: y, w: maxW, h: maxH \};/);
|
||||
assert.match(src, /catch \(e\) \{\s*\n\s*return fallback;/);
|
||||
// The sizing option is gone: leaving it would only re-stretch what fitImage
|
||||
// just measured.
|
||||
const placement = src.slice(src.indexOf("if (type === 'image')"), src.indexOf("// ── Code block ──"));
|
||||
assert.doesNotMatch(placement, /sizing:/, 'no sizing option fighting the measured box');
|
||||
assert.match(placement, /slide\.addImage\(\{ data: assetImages\[src\], x: box\.x, y: box\.y, w: box\.w, h: box\.h \}\)/);
|
||||
|
||||
// image-size is a real dependency now, and only one copy of it exists.
|
||||
const pkg = JSON.parse(read('package.json'));
|
||||
assert.ok(pkg.dependencies['image-size'], 'declared directly rather than borrowed transitively');
|
||||
assert.equal(pkg.overrides['image-size'], '^2.0.2',
|
||||
'pptxgenjs declares it but never requires it; without this npm ships a second copy nothing can load');
|
||||
});
|
||||
|
||||
test('only the image parsers this app needs are allowed to run', () => {
|
||||
// ICNS, JXL and HEIF have open denial-of-service advisories against EVERY
|
||||
// published image-size — GHSA-w3rx-r6r6-pgpr and GHSA-5p2g-fcmc-qvqq, both
|
||||
// ranged <=2.0.2. There is no fixed release, so the fix is not to run them.
|
||||
const src = read('src/routes/learningAI.js');
|
||||
assert.match(src, /var keep = \{ png: 1, jpg: 1, webp: 1, gif: 1 \};/);
|
||||
assert.match(src, /lib\.disableTypes\(\(lib\.types \|\| \[\]\)\.filter\(function \(t\) \{ return !keep\[t\]; \}\)\)/);
|
||||
// disableTypes replaces the enabled set, so calling it per measurement would
|
||||
// be wasted work and easy to get wrong.
|
||||
assert.match(src, /if \(!imageSizeReady\) \{/, 'done once, at first use');
|
||||
|
||||
// And the library actually honours it — this is the contract that could move.
|
||||
const lib = require('image-size');
|
||||
const keep = { png: 1, jpg: 1, webp: 1, gif: 1 };
|
||||
lib.disableTypes((lib.types || []).filter(t => !keep[t]));
|
||||
assert.throws(() => lib.imageSize(Buffer.concat([Buffer.from('icns'), Buffer.alloc(60)])),
|
||||
'a disabled parser must refuse rather than run');
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue