fix(gatsby-plugin-sharp): don't serve static assets that are not resu…

…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
gatsbyjs · Mar 29, 2023 · 6539860 · 6539860
1 parent 36bffc4
commit 6539860
Show file tree

Hide file tree

Showing 6 changed files with 16 additions and 8 deletions.
diff --git a/e2e-tests/development-runtime/SHOULD_NOT_SERVE b/e2e-tests/development-runtime/SHOULD_NOT_SERVE
@@ -0,0 +1 @@
+this file shouldn't be allowed to be served
diff --git a/e2e-tests/development-runtime/package.json b/e2e-tests/development-runtime/package.json
@@ -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",
@@ -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": {

diff --git a/e2e-tests/production-runtime/SHOULD_NOT_SERVE b/e2e-tests/production-runtime/SHOULD_NOT_SERVE
@@ -0,0 +1 @@
+this file shouldn't be allowed to be served
diff --git a/e2e-tests/production-runtime/package.json b/e2e-tests/production-runtime/package.json
@@ -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",
@@ -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": {

diff --git a/packages/gatsby-plugin-sharp/src/gatsby-node.js b/packages/gatsby-plugin-sharp/src/gatsby-node.js
@@ -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()
     }
 
@@ -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)
   })
 }

diff --git a/packages/gatsby-plugin-sharp/src/index.js b/packages/gatsby-plugin-sharp/src/index.js
@@ -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`