withastro / astro

The web framework for content-driven websites. ⭐️ Star to support our work!
https://astro.build
Other
46.84k stars 2.49k forks source link

Generated images are larger when a certain package is imported #12076

Closed Hanziness closed 1 month ago

Hanziness commented 1 month ago

Astro Info

Astro                    v4.15.9
Node                     v20.15.1
System                   Linux (x64)
Package Manager          pnpm
Output                   static
Adapter                  none
Integrations             none

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

Based on the title, this might seem like an issue with https://github.com/ACP-CODE/astro-favicons, but I'm unsure as I couldn't drill deep enough to find the root cause. So, the issue is the following: when simply importing the package astro-favicons@2.0.2 (latest), the size of the generated WebP images (using sharp) increase. This seems to be a very elusive issue, but I'm hoping someone can help me find what's causing it. Only now had I been able to somewhat reliably reproduce it.

Repro (only seems to happen locally, couldn't yet reproduce on Stackblitz that runs on Node 18):

  1. Open the attached repo (https://github.com/Hanziness/astro-image-bug), set it up (I used pnpm, so pnpm install)
  2. Run pnpm benchmark (alias node ./generate.js) and observe that sharp outputs a roughly ~58.01 KB WebP file for a width of 2000 (same as input) and a quality of 80 (equivalent of high when using the <Image> component). These files can be used for reference as they perform the same transforms that Astro does.
  3. Open astro.config.mjs and observe the following:
  4. A) Run pnpm dev with line 2 still in the config. On the Network tab in the inspector, notice that the image of the same width generated by Astro is 87.12 KB (~150% increase compared to benchmark)
  5. B) Remove the second line (import favicons from "astro-favicons") from astro.config.mjs and restart the dev server (kill it and run pnpm dev again)
  6. Observe that the generated image is now 58.01 KB, same as the benchmark image.
  7. Also observe that when running astro build the generated image is 58.01 KB even with the import present, but on my personal site (where I have dynamic imports and such), I have started experiencing increased file sizes in these cases, too - that's why the bug was so elusive for me.

If this is 100% not an Astro issue, please forgive me, but if one import can lead to such issues, I figured it might even lead to a security issue where additional bytes could be added to any generated image by simply importing an unused dependency. Though the bytes at the end of the file seem to be metadata from the original file added as text.

Additional info that might help debugging

Very interestingly this does not seem to happen in the linked Stackblitz, so try https://github.com/Hanziness/astro-image-bug

~I was only able to try on NodeJS 20 and 21, both seem affected. Stackblitz runs the project on Node 18, that could also be a starting point. I'll try Node 18 if I have some more time.~ (Update: it happens on Node 18, too, but I was using Alpine Linux in all three cases).

The image in question has a bunch of EXIF data. It was edited using darktable, which also adds lots of data in XML format, including edit history and such, that it originally stores in XMP sidecar files. When I inspected the "bad" (big) image, it seemed to have that whole XML data pasted at the end of the file as plaintext:

View of the HEX data of the bigger file

The issue happens on the latest stable 4.15.9 and on 5.0.0-beta.2, too.

The "wrong" image (top one on the following screenshot) seems less saturated than the "good" (bottom) one, possibly due to the ICC profile breaking, but that might have nothing to do with the issue, it's just an observation (and it's why I started looking around).

Saturation differences between generated (top) and reference (bottom)

I also tried to insert some debug code into the Vite transform callback of the astro-favicons plugin, but it did not seem to interact with the _image endpoint. I suppose the _image endpoint (https://github.com/withastro/astro/tree/main/packages/astro/src/assets/endpoint) might be the culprit, but nothing seemed out of order for me at first glance there, either.

Image sizes screenshot from the browser:

Image sizes

Let me know if I can help with anything else! I'll also report progress if I find anything else on my own.

What's the expected result?

The generated images should be of the same size as the benchmark ones even if astro-favicons is imported as, from my investigations, it did not seem to interact with them.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-tknb4g

Participation

Hanziness commented 1 month ago

Quick update: I tried to run the project in Docker with Node 18 (node:18.20.4-alpine3.20), but the Astro generated image in dev mode was still ~29.11 KB bigger. So far I've only used Alpine Linux (on my dev machine and now in Docker), I'll test with some more Docker images to see if the OS has to do anything with the issue. Very interesting that it does not seem to happen on Stackblitz (with the same container).

ascorbic commented 1 month ago

What a fun one! My immediate thought was that it was about conflicting versions of sharp, and it seems that's correct. The favicons lib has a dependency on an old version of sharp, and when you import it, it's causing astro to use that version. It seems that that version has different settings for webp, causing the different filesize. If you add a resolutions block to your package.json it fixes it:

    "resolutions": {
        "sharp": "0.33.5"
    }

Now, I don't know if that will break the favicons library, but it fixes the immediate issue.

Hanziness commented 1 month ago

Nice find! Indeed! I also started suspecting the conflicting sharp dependency, even gone as far to uninstall astro-favicons, but I was still seeing the bigger images in the generated files - turns out, those were likely reused cache entries as now I don't get the issue in dev nor in production!

Thank you for the help @ascorbic :)