-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(gatsby-plugin-sharp): strip non related args when hashing resizin…
…g args (#12129) So continuing on the work to make gatsby-plugin-sharp friendly on third-party packages (oorestisime/gatsby-remark-rehype-images#2) we now face this issue #12080 TL;DR images generated from transformer-sharp or and other plugins are the same (same sha256) but with different hash (depends on the available options). The problem is that all the plugins leak their options into the hash generation and even ones that are unrelated to the resizing processing (base64, generateTraceSVG, traceSVG etc) I've been discussing this with @pieh for the last 2-3 hours :D Proposed solution was to sanitize the args based on a whitelist, which is implemented on the PR. Which leads to the first question: - is it ok if after this release _all_ image hashes are changing? By doing so it can also potentially help lazy image processing @wardpeet by getting rid of unrelated options when persisting jobs. Anyway current state of the PR is that it kind of works, i ve tested this in websites using my plugin and other that not. The issue here is that there are some differences in the number of images created when this modification is on or not. I also verified that public folder doesn't contain duplicate images: `find public -regex ".*\.\(jpg\|webp\|png\|jpeg\)" -type f -exec sha256sum {} \; | cut -d ' ' -f 1 |sort| uniq -d | wc -l`
- Loading branch information
1 parent
85b8749
commit da0b622
Showing
6 changed files
with
189 additions
and
49 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
94 changes: 94 additions & 0 deletions
94
packages/gatsby-plugin-sharp/src/__tests__/process-file.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
const { createArgsDigest } = require(`../process-file`) | ||
|
||
describe(`createArgsDigest`, () => { | ||
const defaultArgsBaseline = { | ||
toFormat: `jpg`, | ||
width: 500, | ||
height: 500, | ||
cropFocus: 17, | ||
quality: 50, | ||
pngCompressionLevel: 4, | ||
jpegProgressive: false, | ||
grayscale: false, | ||
rotate: 0, | ||
duotone: null, | ||
} | ||
|
||
describe(`changes hash if used args are different`, () => { | ||
const testHashDifferent = (label, change, extraBaselineOptions = {}) => { | ||
it(label, () => { | ||
const argsBaseline = { | ||
...defaultArgsBaseline, | ||
...extraBaselineOptions, | ||
} | ||
const baselineHash = createArgsDigest(argsBaseline) | ||
const outputHash = createArgsDigest({ | ||
...defaultArgsBaseline, | ||
...change, | ||
}) | ||
expect(baselineHash).not.toBe(outputHash) | ||
}) | ||
} | ||
|
||
testHashDifferent(`width change`, { width: defaultArgsBaseline.width + 1 }) | ||
testHashDifferent(`height change`, { | ||
height: defaultArgsBaseline.height + 1, | ||
}) | ||
testHashDifferent(`cropFocus change`, { | ||
cropFocus: defaultArgsBaseline.cropFocus + 1, | ||
}) | ||
testHashDifferent(`format change`, { toFormat: `png` }) | ||
testHashDifferent(`jpegProgressive change`, { | ||
jpegProgressive: !defaultArgsBaseline.jpegProgressive, | ||
}) | ||
testHashDifferent(`grayscale change`, { | ||
grayscale: !defaultArgsBaseline.grayscale, | ||
}) | ||
testHashDifferent(`rotate change`, { | ||
rotate: defaultArgsBaseline.rotate + 1, | ||
}) | ||
testHashDifferent(`dutone change`, { | ||
duotone: { | ||
highlight: `#ff0000`, | ||
shadow: `#000000`, | ||
}, | ||
}) | ||
}) | ||
|
||
describe(`doesn't change hash if not used options are different`, () => { | ||
const testHashEqual = (label, change, extraBaselineOptions = {}) => { | ||
it(label, () => { | ||
const argsBaseline = { | ||
...defaultArgsBaseline, | ||
...extraBaselineOptions, | ||
} | ||
const baselineHash = createArgsDigest(argsBaseline) | ||
const outputHash = createArgsDigest({ | ||
...defaultArgsBaseline, | ||
...change, | ||
}) | ||
expect(baselineHash).toBe(outputHash) | ||
}) | ||
} | ||
|
||
testHashEqual(`unrelated argument`, { foo: `bar` }) | ||
|
||
testHashEqual(`png option change when using jpg`, { | ||
pngCompressionLevel: defaultArgsBaseline.pngCompressionLevel + 1, | ||
}) | ||
testHashEqual( | ||
`jpg option change when using png`, | ||
{ | ||
toFormat: `png`, | ||
jpegProgressive: !defaultArgsBaseline.jpegProgressive, | ||
}, | ||
{ | ||
toFormat: `png`, | ||
} | ||
) | ||
describe(`not used arguments`, () => { | ||
testHashEqual(`maxWidth`, { maxWidth: 500 }) | ||
testHashEqual(`base64`, { base64: true }) | ||
}) | ||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters