Skip to content

chore(deps): move from image-size to probe-image-size - #207

Closed
elefevre wants to merge 1 commit into
TurboDocx:mainfrom
elefevre:fix/replace-image-size-with-probe-image-size
Closed

elefevre wants to merge 1 commit into
TurboDocx:mainfrom
elefevre:fix/replace-image-size-with-probe-image-size

Conversation

@elefevre

Copy link
Copy Markdown

image-size is being flagged by vulnerability scanners such as Snyk:

Vulnerabilities are assessed as "High". Practical risk might be lower, as HEIF, JP2, JXL, ICNS are not standard web image formats.

Still, image-size is now archived, so it might be appropriate to switch to another library anyway.

Notes:

  • made heavy use of Claude Code here; I think the result is good, though
  • Claude suggested a few alternative libraries before settling on probe-image-size:
    • image-size@2.x (the existing one); includes the vulnerabilities and is archived
    • image-size@1.x (the previous version of the same library); this also includes the vulnerability on ICNS
    • fast-image-size; still in version 0.1.3 and not maintained since 2016
    • image-meta; still in version 0.2.2; last maintained in Oct 2025; might be vulnerable to ICNS images with zero length too (it's hard to tell)

It seems that probe-image-size is a good choice, despite the several dependencies that it is pulling.

Happy to adjust this PR upon your feedback!

@github-actions

github-actions Bot commented Jun 15, 2026 •

Copy link
Copy Markdown

TurboDocx DOCX Diff Report

Automated HTML to DOCX regression testing | Powered by TurboDocx

Summary

  • ✅ Identical files: 71
  • 🔄 Changed files: 0
  • ➕ New files: 0
  • ➖ Deleted files: 0

🚀 Powered by TurboDocx | html-to-docx

Automated DOCX regression testing • Catch document generation bugs before they ship • 100% open source

Generated by TurboDocx DOCX Diff workflow • Learn more

@elefevre
elefevre force-pushed the fix/replace-image-size-with-probe-image-size branch from 58d8c2d to f7f97b8 Compare June 15, 2026 20:26
@nicolasiscoding

Copy link
Copy Markdown
Member

@elefevre please add yourself to the contributors in package.json and can get a maintainer to look at th is

@elefevre
elefevre force-pushed the fix/replace-image-size-with-probe-image-size branch from f7f97b8 to 88f86c1 Compare June 16, 2026 12:42
@elefevre

Copy link
Copy Markdown
Author

Added myself to the contributors.
It seems that the most active maintainers around these bits are yourself, @nicolasiscoding and @K-Kumar-01 (I assume the original author of the forked library is not active anymore). Please have a look whenever possible. 🙏

@elefevre

Copy link
Copy Markdown
Author

Additional note: in the first approach, Claude actually implemented the necessary code directly, without using an external library. It actually felt reasonable (~50 lines of not-too-complex lines). But I wasn't sure how to test this and I didn't feel that I could stand by it.
Still, I liked it, as it reduced dependencies.

@K-Kumar-01

Copy link
Copy Markdown
Collaborator

@elefevre
Thank you for your contributions. I will take a look here.

Comment thread src/helpers/xml-builder.js Outdated
let imageProperties;
try {
imageProperties = sizeOf(imageBuffer);
imageProperties = await sizeOf(Readable.from(imageBuffer));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there a reason to use the async streaming API (await sizeOf(Readable.from(imageBuffer))) over the synchronous buffer one (sizeOf.sync(imageBuffer))?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really, I think I simply rushed through the documentation and didn't realize there was a sync function available. I don't see the benefit of using the async alternative here. I'll switch to using sync.

@K-Kumar-01 K-Kumar-01 left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@elefevre
Requested one clarification, before we merge this.

`image-size` is being flagged by vulnerability scanners such as Snyk:
- Infinite loop on image type ICNS
  https://security.snyk.io/vuln/SNYK-JS-IMAGESIZE-17295814
- Infinite loop on image types HEIF, JP2, and JXL
  https://security.snyk.io/vuln/SNYK-JS-IMAGESIZE-17295814

Vulnerabilities are assessed as "High". Practical risks might be lower,
as HEIF, JP2, JXL, ICNS are not standard web image formats.

Still, `image-size` has been marked as archived (see
https://github.com/image-size/image-size), so it is appropriate to
switch to another library.
@elefevre
elefevre force-pushed the fix/replace-image-size-with-probe-image-size branch from 88f86c1 to a352ef3 Compare June 16, 2026 20:09
@elefevre

elefevre commented Jun 16, 2026 •

Copy link
Copy Markdown
Author

Now uses the sync function provided by probe-image-size. This makes the diff with the existing code even smaller.

@nicolasiscoding

Copy link
Copy Markdown
Member

@elefevre please point this to develop and I can merge it

@nicolasiscoding

Copy link
Copy Markdown
Member

@elefevre, @amitsharma-turbodocx ported it, rebased it, and then kept the package.json contributors

@amitsharma-turbodocx

Copy link
Copy Markdown
Contributor

Thanks @elefevre — good catch, and the library comparison in your description was genuinely helpful.

We've ported this to #227, branched off develop (our integration branch — dependency changes land there before main). Your commit carries over via Co-authored-by:, and you've been added to contributors in package.json.

One deviation: we import from probe-image-size/sync rather than the package root, since the root entry pulls in needle (and with it http/https/zlib/iconv-lite), which would land in our bundled browser builds. Otherwise it's your change as-is.

#227 is what we'll take through testing and merge from. Thanks again!

@elefevre
elefevre deleted the fix/replace-image-size-with-probe-image-size branch September 1, 2026 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants