-
Notifications
You must be signed in to change notification settings - Fork 10.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(gatsby-plugin-utils): path pieces too long and url safe base64 encoding #35160
Conversation
packages/gatsby-plugin-utils/src/polyfill-remote-file/__tests__/gatsby-image-resolver.ts
Outdated
Show resolved
Hide resolved
packages/gatsby-plugin-utils/src/polyfill-remote-file/utils/url-generator.ts
Outdated
Show resolved
Hide resolved
Sharing a bit on why I changed the code here. Image Url & args can indeed be any value like @veryspry did. Instead of creating our own md5 hash, we use createContentDigest (which is also an md5 hash but native to Gatsby). For url and args we will use query string parameters as they can be any length and keep code between image-cdn and local images the same. |
@wardpeet for files on disk (with image CDN turned off) wont the query args not work? EDIT: I see you have an md5 in the file path with the args - that makes sense 👍 I can see how it works now |
0675146
to
52ab2ca
Compare
packages/gatsby-plugin-utils/src/polyfill-remote-file/utils/url-generator.ts
Show resolved
Hide resolved
packages/gatsby-plugin-utils/src/polyfill-remote-file/jobs/__tests__/gatsby-worker.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Judging by the tests this looks good
packages/gatsby-plugin-utils/src/polyfill-remote-file/graphql/gatsby-image-resolver.ts
Outdated
Show resolved
Hide resolved
packages/gatsby-plugin-utils/src/polyfill-remote-file/jobs/__tests__/gatsby-worker.ts
Show resolved
Hide resolved
packages/gatsby-plugin-utils/src/polyfill-remote-file/utils/__tests__/url-generator.ts
Show resolved
Hide resolved
packages/gatsby-plugin-utils/src/polyfill-remote-file/utils/url-generator.ts
Show resolved
Hide resolved
9c96d24
to
3d98ed4
Compare
…wnloading images locally for a build
…mage" image files in the dev server
…only url safe chars are used
3d98ed4
to
a4a39f8
Compare
Just bumping this because we are running into this issue as well 👍 |
If I want to test this without the |
@davidpaulsson You can use the |
Hi, I run into this issue during the building of my site. I retrieve datas from WordPress and the process fails during gatsby.IMAGE_CDN job which return this error
I have "gatsby": "^4.14.0-next.0" version but the problem still persist :( Can anyone help me? |
Hey @thekage91 👋 For some extra context, this PR made a change to md5 hash some of the image path segments via the gatsby Taking a look at the path in your error:
I extract the last piece:
This will either be a base64 encoded version of the source image URL OR an md5 hash of the image source URL depending on the Gatsby version you are on. If I base64 decode the string:
The operation is successful and I get this back which tells me that the version of gatsby you have installed does not include this fix.
Can you try using the explicit Let me know if this helps and if not, feel free to ping me back here and hopefully we can get this sorted out 🙂 |
…coding (gatsbyjs#35160) Co-authored-by: Ward Peeters <ward@coding-tech.com>
Description
The PR solves two bugs:
gatsbyImage
resolver locally, sometimes base64 encoded pieces would exceed system limits. The fix is to move to query parameters and use md5 hashes for the base64 pieces when an image is downloaded and saved to disk during build time.base64
spec includes forward slashes making it unsuitable for use in URL's, with the query param approach we can ditch base64Related Issues
Fixes #35126