Skip to content
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-sharp): don't serve static assets that are not result of currently triggered deferred job #37796

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions e2e-tests/development-runtime/SHOULD_NOT_SERVE
@@ -0,0 +1 @@
this file shouldn't be allowed to be served
5 changes: 3 additions & 2 deletions e2e-tests/development-runtime/package.json
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
@@ -0,0 +1 @@
this file shouldn't be allowed to be served
3 changes: 2 additions & 1 deletion e2e-tests/production-runtime/package.json
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
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
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