Skip to content

Commit

Permalink
fix(gatsby-plugin-sharp): don't serve static assets that are not resu…
Browse files Browse the repository at this point in the history
…lt of currently triggered deferred job (#37796)

* add tests

* fix(gatsby-plugin-sharp): don't serve static assets that are not result of currently triggered deferred job
  • Loading branch information
pieh authored Mar 29, 2023
1 parent 36bffc4 commit 6539860
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 8 deletions.
1 change: 1 addition & 0 deletions e2e-tests/development-runtime/SHOULD_NOT_SERVE
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
this file shouldn't be allowed to be served
5 changes: 3 additions & 2 deletions e2e-tests/development-runtime/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,15 @@
"license": "MIT",
"scripts": {
"build": "gatsby build",
"develop": "cross-env CYPRESS_SUPPORT=y ENABLE_GATSBY_REFRESH_ENDPOINT=y GATSBY_ENABLE_QUERY_ON_DEMAND_IN_CI=y gatsby develop",
"develop": "cross-env CYPRESS_SUPPORT=y ENABLE_GATSBY_REFRESH_ENDPOINT=y GATSBY_ENABLE_QUERY_ON_DEMAND_IN_CI=y GATSBY_ENABLE_LAZY_IMAGES_IN_CI=y gatsby develop",
"serve-static-files": "node ./serve-static-files.mjs",
"serve": "gatsby serve",
"clean": "gatsby clean",
"typecheck": "tsc --noEmit",
"start": "npm run develop",
"format": "prettier --write \"src/**/*.js\"",
"test": "npm run start-server-and-test || (npm run reset && exit 1)",
"test:dir-traversel-access": "! curl -f http://localhost:8000/%2e%2e/SHOULD_NOT_SERVE",
"posttest": "npm run reset",
"reset": "node scripts/reset.js",
"reset:preview": "curl -X POST http://localhost:8000/__refresh",
Expand All @@ -55,7 +56,7 @@
"playwright:debug": "playwright test --project=chromium --debug",
"start-server-and-test:playwright": "start-server-and-test develop http://localhost:8000 serve-static-files http://localhost:8888 playwright",
"start-server-and-test:playwright-debug": "start-server-and-test develop http://localhost:8000 serve-static-files http://localhost:8888 playwright:debug",
"combined": "npm run playwright && npm run cy:run",
"combined": "npm run playwright && npm run cy:run && npm run test:dir-traversel-access",
"postinstall": "playwright install chromium"
},
"devDependencies": {
Expand Down
1 change: 1 addition & 0 deletions e2e-tests/production-runtime/SHOULD_NOT_SERVE
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
this file shouldn't be allowed to be served
3 changes: 2 additions & 1 deletion e2e-tests/production-runtime/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
"start": "npm run develop",
"clean": "gatsby clean",
"test": "npm run build && npm run start-server-and-test && npm run test-env-vars",
"test:dir-traversel-access": "! curl -f http://localhost:9000/%2e%2e/SHOULD_NOT_SERVE",
"test:offline": "npm run build:offline && yarn start-server-and-test:offline && npm run test-env-vars",
"test-env-vars": " node __tests__/env-vars.js",
"start-server-and-test": "start-server-and-test serve http://localhost:9000 serve-static-files http://localhost:8888 combined",
Expand All @@ -51,7 +52,7 @@
"playwright:debug": "playwright test --project=chromium --debug",
"start-server-and-test:playwright": "start-server-and-test serve http://localhost:9000 serve-static-files http://localhost:8888 playwright",
"start-server-and-test:playwright-debug": "start-server-and-test serve http://localhost:9000 serve-static-files http://localhost:8888 playwright:debug",
"combined": "npm run playwright && npm run cy:run",
"combined": "npm run playwright && npm run cy:run && npm run test:dir-traversel-access",
"postinstall": "playwright install chromium"
},
"devDependencies": {
Expand Down
12 changes: 8 additions & 4 deletions packages/gatsby-plugin-sharp/src/gatsby-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,17 @@ exports.onCreateDevServer = async ({ app, cache, reporter }) => {
const decodedURI = decodeURIComponent(req.path)
const pathOnDisk = path.resolve(path.join(`./public/`, decodedURI))

if (await pathExists(pathOnDisk)) {
return res.sendFile(pathOnDisk)
}

const jobContentDigest = await cache.get(decodedURI)
const cacheResult = jobContentDigest
? await cache.get(jobContentDigest)
: null

if (!cacheResult) {
// this handler is meant to handle lazy images only (images that were registered for
// processing, but deffered to be processed only on request in develop server).
// If we don't have cache result - it means that this is not lazy image or that
// image was already handled in which case `express.static` handler (that is earlier
// than this handler) should take care of handling request.
return next()
}

Expand All @@ -64,6 +65,9 @@ exports.onCreateDevServer = async ({ app, cache, reporter }) => {
await removeCachedValue(cache, jobContentDigest)
}

// we reach this point only when this is a lazy image that we just processed
// because `express.static` is earlier handler, we do have to manually serve
// produced file for current request
return res.sendFile(pathOnDisk)
})
}
Expand Down
2 changes: 1 addition & 1 deletion packages/gatsby-plugin-sharp/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ function createJob(job, { reporter }) {
function lazyJobsEnabled() {
return (
process.env.gatsby_executing_command === `develop` &&
!isCI() &&
(!isCI() || process.env.GATSBY_ENABLE_LAZY_IMAGES_IN_CI) &&
!(
process.env.ENABLE_GATSBY_EXTERNAL_JOBS === `true` ||
process.env.ENABLE_GATSBY_EXTERNAL_JOBS === `1`
Expand Down

0 comments on commit 6539860

Please sign in to comment.