Skip to content

Commit

Permalink
fix(gatsby-plugin-sharp): strip non related args when hashing resizin…
Browse files Browse the repository at this point in the history
…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
oorestisime authored and pieh committed Mar 27, 2019
1 parent 85b8749 commit da0b622
Show file tree
Hide file tree
Showing 6 changed files with 189 additions and 49 deletions.
1 change: 0 additions & 1 deletion packages/gatsby-plugin-sharp/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"@babel/runtime": "^7.0.0",
"async": "^2.1.2",
"bluebird": "^3.5.0",
"fs-exists-cached": "^1.0.0",
"fs-extra": "^7.0.0",
"imagemin": "^6.0.0",
"imagemin-mozjpeg": "^8.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ Object {
"aspectRatio": 2.0661764705882355,
"base64": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABQAAAAKCAYAAAC0VX7mAAAACXBIWXMAABYlAAAWJQFJUiTwAAABAklEQVQoz61R7WrCQBDM+7+Cv1qhlIogoUWMoH8KrfgOLVXRKDV4d7kkl6/pbsxpqBIo9mBuZ7N3k51bB7TKsrwZVsexyS2rKerwFhYFVI1DnkMQOHIuCaLB+ayuwXlMaApXgvMoxjTU8MIQE4rMx8SnNffUkU/qM/bbiPAeRfgw5n8tB+QgO1kmUvwROdm0kYHmUNoe+NoU2wZTdVjQH7Isg9YRpFIXor8vpGmKjb/FQQgslit8fi3wvQ/OHXLhxRvDHY7Qd5+x3e2udmNzTUN4fZvhoddH566L+8cn9AZuVcvpLR3e9kEAISWEkDAmbRXkGCcJ1r4PRRNPEkPu9Kn2Azs0CqJYU3JKAAAAAElFTkSuQmCC",
"density": 144,
"originalImg": "/static/1234/c81ba/test.png",
"originalImg": "/static/1234/e681d/test.png",
"originalName": undefined,
"presentationHeight": 68,
"presentationWidth": 141,
"sizes": "(max-width: 141px) 100vw, 141px",
"src": "/static/1234/c81ba/test.png",
"srcSet": "/static/1234/e4832/test.png 200w,
/static/1234/c81ba/test.png 281w",
"src": "/static/1234/e681d/test.png",
"srcSet": "/static/1234/9ec3c/test.png 200w,
/static/1234/e681d/test.png 281w",
"srcSetType": "image/png",
"tracedSVG": undefined,
}
Expand All @@ -33,14 +33,14 @@ Object {
"aspectRatio": 2.0661764705882355,
"base64": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABQAAAAKCAYAAAC0VX7mAAAACXBIWXMAABYlAAAWJQFJUiTwAAABAklEQVQoz61R7WrCQBDM+7+Cv1qhlIogoUWMoH8KrfgOLVXRKDV4d7kkl6/pbsxpqBIo9mBuZ7N3k51bB7TKsrwZVsexyS2rKerwFhYFVI1DnkMQOHIuCaLB+ayuwXlMaApXgvMoxjTU8MIQE4rMx8SnNffUkU/qM/bbiPAeRfgw5n8tB+QgO1kmUvwROdm0kYHmUNoe+NoU2wZTdVjQH7Isg9YRpFIXor8vpGmKjb/FQQgslit8fi3wvQ/OHXLhxRvDHY7Qd5+x3e2udmNzTUN4fZvhoddH566L+8cn9AZuVcvpLR3e9kEAISWEkDAmbRXkGCcJ1r4PRRNPEkPu9Kn2Azs0CqJYU3JKAAAAAElFTkSuQmCC",
"density": 144,
"originalImg": "/static/1234/ec7d1/test.png",
"originalImg": "/static/1234/e681d/test.png",
"originalName": undefined,
"presentationHeight": 136,
"presentationWidth": 281,
"sizes": "(max-width: 281px) 100vw, 281px",
"src": "/static/1234/ec7d1/test.png",
"srcSet": "/static/1234/dc107/test.png 200w,
/static/1234/ec7d1/test.png 281w",
"src": "/static/1234/e681d/test.png",
"srcSet": "/static/1234/9ec3c/test.png 200w,
/static/1234/e681d/test.png 281w",
"srcSetType": "image/png",
"tracedSVG": undefined,
}
Expand All @@ -51,13 +51,13 @@ Object {
"aspectRatio": 1,
"base64": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABQAAAAUCAIAAAAC64paAAAACXBIWXMAAAsSAAALEgHS3X78AAABKklEQVQ4y8WUy07DMBBFrwMpr4aybjepUeqIFd/CAgmJsuIHgWXVL+Gx4C3xCdQJnpm0UDJNAQnVuouMM8dzY4+DcoA/C/8Fe9bv4ABMMpQOZc5yFPqfwEUQY499jHoY9/BsKQyTRTNMyzu8WAwTbBnI2DY438XbPr0qFsFCvlrkMTERYFgRL3HYUvh5OMdpQqkbBtPC9NDiINQPCZM6LGVvUzIpwNch4V5EGxHS/DeYtjfHZbcyXB8yGfaPimcafLEMHqmw2L5OsWkUUuY6Ee5V28WUP25TXjwPS3iWUFnfcFQPFna98rlmSGL4IOZucQvgT76Po53qeOTYTtp4qpFKe/qM29PhJsVVl3SXsluntLfS7rPvn10MP1Aau+lKBuA9I/kV/AyW6gNTIWTuA/r8lQAAAABJRU5ErkJggg==",
"density": 72,
"originalImg": "/static/1234/197d2/test.png",
"originalImg": "/static/1234/c0399/test.png",
"originalName": undefined,
"presentationHeight": 100,
"presentationWidth": 100,
"sizes": "(max-width: 100px) 100vw, 100px",
"src": "/static/1234/197d2/test.png",
"srcSet": "/static/1234/197d2/test.png 100w",
"src": "/static/1234/c0399/test.png",
"srcSet": "/static/1234/c0399/test.png 100w",
"srcSetType": "image/png",
"tracedSVG": undefined,
}
Expand Down Expand Up @@ -144,8 +144,8 @@ Object {
"base64": undefined,
"height": 100,
"originalName": undefined,
"src": "/static/1234/24ae5/test.png",
"srcSet": "/static/1234/24ae5/test.png 1x",
"src": "/static/1234/c0399/test.png",
"srcSet": "/static/1234/c0399/test.png 1x",
"tracedSVG": "data:image/svg+xml,%3csvg xmlns='http://www.w3.org/2000/svg' width='100' height='100'%3e%3cpath d='M41 24c-18 7-24 29-11 43 15 17 44 8 46-15 1-19-17-34-35-28' fill='red' fill-rule='evenodd'/%3e%3c/svg%3e",
"width": 100,
}
Expand All @@ -156,15 +156,15 @@ Object {
"aspectRatio": 1,
"base64": undefined,
"density": 72,
"originalImg": "/static/1234/9dd55/test.png",
"originalImg": "/static/1234/c0399/test.png",
"originalName": undefined,
"presentationHeight": 100,
"presentationWidth": 100,
"sizes": "(max-width: 100px) 100vw, 100px",
"src": "/static/1234/9dd55/test.png",
"srcSet": "/static/1234/a516e/test.png 25w,
/static/1234/208be/test.png 50w,
/static/1234/9dd55/test.png 100w",
"src": "/static/1234/c0399/test.png",
"srcSet": "/static/1234/0f0dc/test.png 25w,
/static/1234/bc08f/test.png 50w,
/static/1234/c0399/test.png 100w",
"srcSetType": "image/png",
"tracedSVG": "data:image/svg+xml,%3csvg xmlns='http://www.w3.org/2000/svg' width='100' height='100'%3e%3cpath d='M41 24c-18 7-24 29-11 43 15 17 44 8 46-15 1-19-17-34-35-28' fill='red' fill-rule='evenodd'/%3e%3c/svg%3e",
}
Expand Down
94 changes: 94 additions & 0 deletions packages/gatsby-plugin-sharp/src/__tests__/process-file.js
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 })
})
})
})
34 changes: 7 additions & 27 deletions packages/gatsby-plugin-sharp/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const _ = require(`lodash`)
const fs = require(`fs-extra`)
const path = require(`path`)
const { scheduleJob } = require(`./scheduler`)
const { createArgsDigest } = require(`./process-file`)

const imageSizeCache = new Map()
const getImageSize = file => {
Expand Down Expand Up @@ -148,33 +149,12 @@ const healOptions = (

function queueImageResizing({ file, args = {}, reporter }) {
const options = healOptions(pluginOptions, args, file.extension)
// Filter out false args, and args not for this extension and put width at
// end (for the file path)
const pairedArgs = _.toPairs(args)
let filteredArgs
// Remove non-true arguments
filteredArgs = _.filter(pairedArgs, arg => arg[1])
// Remove pathPrefix
filteredArgs = _.filter(filteredArgs, arg => arg[0] !== `pathPrefix`)
filteredArgs = _.filter(filteredArgs, arg => {
if (file.extension.match(/^jp*/)) {
return !_.includes(arg[0], `png`)
} else if (file.extension.match(/^png/)) {
return !arg[0].match(/^jp*/)
}
return true
})
const sortedArgs = _.sortBy(filteredArgs, arg => arg[0] === `width`)
const fileExtension = options.toFormat ? options.toFormat : file.extension

const argsDigest = crypto
.createHash(`md5`)
.update(JSON.stringify(sortedArgs))
.digest(`hex`)

const argsDigestShort = argsDigest.substr(argsDigest.length - 5)
if (!options.toFormat) {
options.toFormat = file.extension
}

const imgSrc = `/${file.name}.${fileExtension}`
const argsDigestShort = createArgsDigest(options)
const imgSrc = `/${file.name}.${options.toFormat}`
const dirPath = path.join(
process.cwd(),
`public`,
Expand Down Expand Up @@ -212,7 +192,7 @@ function queueImageResizing({ file, args = {}, reporter }) {
}

// encode the file name for URL
const encodedImgSrc = `/${encodeURIComponent(file.name)}.${fileExtension}`
const encodedImgSrc = `/${encodeURIComponent(file.name)}.${options.toFormat}`

// Prefix the image src.
const digestDirPrefix = `${file.internal.contentDigest}/${argsDigestShort}`
Expand Down
69 changes: 68 additions & 1 deletion packages/gatsby-plugin-sharp/src/process-file.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ const imagemin = require(`imagemin`)
const imageminMozjpeg = require(`imagemin-mozjpeg`)
const imageminPngquant = require(`imagemin-pngquant`)
const imageminWebp = require(`imagemin-webp`)
const _ = require(`lodash`)
const crypto = require(`crypto`)

// Try to enable the use of SIMD instructions. Seems to provide a smallish
// speedup on resizing heavy loads (~10%). Sharp disables this feature by
Expand All @@ -24,7 +26,49 @@ try {
// doesn't support cpu-core-count utility.
}

module.exports = (file, transforms, options = {}) => {
/**
* List of arguments used by `processFile` function.
* This is used to generate args hash using only
* arguments that affect output of that function.
*/
const argsWhitelist = [
`height`,
`width`,
`cropFocus`,
`toFormat`,
`pngCompressionLevel`,
`quality`,
`jpegProgressive`,
`grayscale`,
`rotate`,
`duotone`,
]

/**
* @typedef {Object} TransformArgs
* @property {number} height
* @property {number} width
* @property {number} cropFocus
* @property {string} toFormat
* @property {number} pngCompressionLevel
* @property {number} quality
* @property {boolean} jpegProgressive
* @property {boolean} grayscale
* @property {number} rotate
* @property {object} duotone
*/

/**+
* @typedef {Object} Transform
* @property {string} outputPath
* @property {TransformArgs} args
*/

/**
* @param {String} file
* @param {Transform[]} transforms
*/
exports.processFile = (file, transforms, options = {}) => {
let pipeline
try {
pipeline = sharp(file)
Expand Down Expand Up @@ -169,3 +213,26 @@ const compressWebP = (pipeline, outputPath, options) =>
})
.then(imageminBuffer => fs.writeFile(outputPath, imageminBuffer))
)

exports.createArgsDigest = args => {
const filtered = _.pickBy(args, (value, key) => {
// remove falsy
if (!value) return false
if (args.toFormat.match(/^jp*/) && _.includes(key, `png`)) {
return false
} else if (args.toFormat.match(/^png/) && key.match(/^jp*/)) {
return false
}
// after initial processing - get rid of unknown/unneeded fields
return argsWhitelist.includes(key)
})

const argsDigest = crypto
.createHash(`md5`)
.update(JSON.stringify(filtered, Object.keys(filtered).sort()))
.digest(`hex`)

const argsDigestShort = argsDigest.substr(argsDigest.length - 5)

return argsDigestShort
}
4 changes: 2 additions & 2 deletions packages/gatsby-plugin-sharp/src/scheduler.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
const _ = require(`lodash`)
const ProgressBar = require(`progress`)
const existsSync = require(`fs-exists-cached`).sync
const { existsSync } = require(`fs`)
const queue = require(`async/queue`)
const processFile = require(`./process-file`)
const { processFile } = require(`./process-file`)

const toProcess = {}
let totalJobs = 0
Expand Down

0 comments on commit da0b622

Please sign in to comment.