From beb7a0a8f89d007d27d511a11fe6baf5bc032847 Mon Sep 17 00:00:00 2001 From: Yuichi Nishitani <56058655+hawkk9@users.noreply.github.com> Date: Tue, 19 Dec 2023 10:19:02 +0900 Subject: [PATCH] fix(fastify): Server not handling periods in query params (#9714) ## What does this PR do? Fix for this issue. https://github.com/redwoodjs/redwood/pull/9663 If the query parameter contains`. `will result in a 404 error when routing to the defined route. Example.) http://localhost:8910/posts?redirect_url=https%3A%2F%2Fwww.google.com ## The problem occurs - Version: 6.3.3 or higher - When the application is running with `yarn rw serve` command. ## The problem not occurs - Version: 6.3.2 or lower - When the application is running with `yarn rw dev` command. --------- Co-authored-by: yuichi.nishitani Co-authored-by: Dominic Saadi --- .../src/__tests__/withWebServer.test.ts | 20 +++++++++++++++++++ .../api-server/src/plugins/withWebServer.ts | 7 ++++++- packages/fastify/src/web.ts | 6 +++++- packages/web-server/package.json | 1 + packages/web-server/src/web.ts | 16 +++++++++++++-- yarn.lock | 1 + 6 files changed, 47 insertions(+), 4 deletions(-) diff --git a/packages/api-server/src/__tests__/withWebServer.test.ts b/packages/api-server/src/__tests__/withWebServer.test.ts index ecd7c0f6508d..07a5f7eabe80 100644 --- a/packages/api-server/src/__tests__/withWebServer.test.ts +++ b/packages/api-server/src/__tests__/withWebServer.test.ts @@ -280,4 +280,24 @@ describe('withWebServer', () => { ) }) }) + + describe("returns a 404 for assets that can't be found", () => { + it("returns a 404 for non-html assets that can't be found", async () => { + const res = await fastifyInstance.inject({ + method: 'GET', + url: '/kittens.png', + }) + + expect(res.statusCode).toBe(404) + }) + + it('handles "."s in routes', async () => { + const res = await fastifyInstance.inject({ + method: 'GET', + url: '/my-page?loading=spinner.blue', + }) + + expect(res.statusCode).toBe(200) + }) + }) }) diff --git a/packages/api-server/src/plugins/withWebServer.ts b/packages/api-server/src/plugins/withWebServer.ts index fa34797ce134..98c5b9d8ce6c 100644 --- a/packages/api-server/src/plugins/withWebServer.ts +++ b/packages/api-server/src/plugins/withWebServer.ts @@ -2,6 +2,7 @@ import fs from 'fs' import path from 'path' import fastifyStatic from '@fastify/static' +import fastifyUrlData from '@fastify/url-data' import type { FastifyInstance, FastifyReply, FastifyRequest } from 'fastify' import { getPaths } from '@redwoodjs/project-config' @@ -27,6 +28,8 @@ const withWebServer = async ( fastify: FastifyInstance, options: WebServerArgs ) => { + fastify.register(fastifyUrlData) + const prerenderedFiles = findPrerenderedHtml() const indexPath = getFallbackIndexPath() @@ -57,7 +60,9 @@ const withWebServer = async ( fastify.setNotFoundHandler( {}, function (req: FastifyRequest, reply: FastifyReply) { - const requestedExtension = path.extname(req.url) + const urlData = req.urlData() + const requestedExtension = path.extname(urlData.path ?? '') + // If it's requesting some sort of asset, e.g. .js or .jpg files // Html files should fallback to the index.html if (requestedExtension !== '' && requestedExtension !== '.html') { diff --git a/packages/fastify/src/web.ts b/packages/fastify/src/web.ts index 8814ee604fff..93bd0cc89e7e 100644 --- a/packages/fastify/src/web.ts +++ b/packages/fastify/src/web.ts @@ -2,6 +2,7 @@ import fs from 'node:fs' import path from 'node:path' import fastifyStatic from '@fastify/static' +import fastifyUrlData from '@fastify/url-data' import fg from 'fast-glob' import type { FastifyInstance, @@ -20,6 +21,7 @@ export async function redwoodFastifyWeb( opts: RedwoodFastifyWebOptions, done: HookHandlerDoneFunction ) { + fastify.register(fastifyUrlData) const prerenderedFiles = findPrerenderedHtml() // Serve prerendered HTML directly, instead of the index. @@ -51,7 +53,9 @@ export async function redwoodFastifyWeb( fastify.setNotFoundHandler( {}, function (req: FastifyRequest, reply: FastifyReply) { - const requestedExtension = path.extname(req.url) + const urlData = req.urlData() + const requestedExtension = path.extname(urlData.path ?? '') + // If it's requesting some sort of asset, e.g. .js or .jpg files // Html files should fallback to the index.html if (requestedExtension !== '' && requestedExtension !== '.html') { diff --git a/packages/web-server/package.json b/packages/web-server/package.json index d260a0b86792..94cee769bf84 100644 --- a/packages/web-server/package.json +++ b/packages/web-server/package.json @@ -27,6 +27,7 @@ "dependencies": { "@fastify/http-proxy": "9.3.0", "@fastify/static": "6.12.0", + "@fastify/url-data": "5.4.0", "@redwoodjs/project-config": "6.0.7", "chalk": "4.1.2", "dotenv-defaults": "5.0.2", diff --git a/packages/web-server/src/web.ts b/packages/web-server/src/web.ts index ab23bb69f7cc..8b115880335a 100644 --- a/packages/web-server/src/web.ts +++ b/packages/web-server/src/web.ts @@ -6,6 +6,7 @@ import fs from 'node:fs' import path from 'node:path' import fastifyStatic from '@fastify/static' +import fastifyUrlData from '@fastify/url-data' import fg from 'fast-glob' import type { FastifyInstance, @@ -26,6 +27,7 @@ export async function redwoodFastifyWeb( opts: RedwoodFastifyWebOptions, done: HookHandlerDoneFunction ) { + fastify.register(fastifyUrlData) const prerenderedFiles = findPrerenderedHtml() // Serve prerendered HTML directly, instead of the index. @@ -53,9 +55,19 @@ export async function redwoodFastifyWeb( const indexPath = getFallbackIndexPath() // For SPA routing, fallback on unmatched routes and let client-side routing take over. - fastify.setNotFoundHandler({}, function (_, reply: FastifyReply) { + fastify.setNotFoundHandler({}, function (req, reply) { + const urlData = req.urlData() + const requestedExtension = path.extname(urlData.path ?? '') + + // If it's requesting some sort of asset, e.g. .js or .jpg files + // Html files should fallback to the index.html + if (requestedExtension !== '' && requestedExtension !== '.html') { + reply.code(404) + return reply.send('Not Found') + } + reply.header('Content-Type', 'text/html; charset=UTF-8') - reply.sendFile(indexPath) + return reply.sendFile(indexPath) }) done() diff --git a/yarn.lock b/yarn.lock index 873789142760..6ff3c94b6a56 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9514,6 +9514,7 @@ __metadata: dependencies: "@fastify/http-proxy": "npm:9.3.0" "@fastify/static": "npm:6.12.0" + "@fastify/url-data": "npm:5.4.0" "@redwoodjs/project-config": "npm:6.0.7" "@types/yargs-parser": "npm:21.0.3" chalk: "npm:4.1.2"