Skip to content

Commit

Permalink
fix(fastify): Server not handling periods in query params (#9714)
Browse files Browse the repository at this point in the history
Fix for this issue.
#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

- Version: 6.3.3 or higher
- When the application is running with `yarn rw serve` command.

- Version: 6.3.2 or lower
- When the application is running with `yarn rw dev` command.

---------

Co-authored-by: yuichi.nishitani <hawkk9.public@gmail.com>
Co-authored-by: Dominic Saadi <dominiceliassaadi@gmail.com>
  • Loading branch information
3 people authored and Tobbe committed Dec 21, 2023
1 parent 56ed142 commit 3342f99
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 4 deletions.
20 changes: 20 additions & 0 deletions packages/api-server/src/__tests__/withWebServer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
})
7 changes: 6 additions & 1 deletion packages/api-server/src/plugins/withWebServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -27,6 +28,8 @@ const withWebServer = async (
fastify: FastifyInstance,
options: WebServerArgs
) => {
fastify.register(fastifyUrlData)

const prerenderedFiles = findPrerenderedHtml()
const indexPath = getFallbackIndexPath()

Expand Down Expand Up @@ -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') {
Expand Down
6 changes: 5 additions & 1 deletion packages/fastify/src/web.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -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') {
Expand Down
1 change: 1 addition & 0 deletions packages/web-server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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.5.1",
"chalk": "4.1.2",
"dotenv-defaults": "5.0.2",
Expand Down
16 changes: 14 additions & 2 deletions packages/web-server/src/web.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -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()
Expand Down
1 change: 1 addition & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9157,6 +9157,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.5.1"
"@types/yargs-parser": "npm:21.0.3"
chalk: "npm:4.1.2"
Expand Down

0 comments on commit 3342f99

Please sign in to comment.