Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions src/server.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Fastify from 'fastify';
import rateLimit from '@fastify/rate-limit';
import { readFile } from 'fs/promises';
import { STATUS_CODES } from 'node:http';
import { join, dirname } from 'path';
import { fileURLToPath } from 'url';
import { handleGet, handleHead, handlePut, handleDelete, handleOptions, handlePatch } from './handlers/resource.js';
Expand Down Expand Up @@ -169,6 +170,32 @@ export function createServer(options = {}) {
}
// Default Fastify behavior for other client errors
socket.destroy(err);
},
// Catch Fastify-internal errors that fire BEFORE any user hook
// runs — notably FST_ERR_BAD_URL on malformed percent-encoding
// (`%g1`, truncated `%E0%`, invalid UTF-8). Without this, Fastify
// writes the 400 response directly via `res.writeHead` and the
// browser sees a CORS error (no Access-Control-Allow-*) instead
// of the real status. #376.
frameworkErrors: (err, request, reply) => {
// ALWAYS apply CORS headers — matches the rest of the server's
// behavior (every successful response sets CORS via the global
// onRequest hook). getCorsHeaders defaults Allow-Origin to `*`
// when the request didn't send an Origin header.
const cors = getCorsHeaders(request.headers?.origin);
for (const [k, v] of Object.entries(cors)) reply.header(k, v);
const statusCode = err.statusCode ?? 400;
reply.code(statusCode).type('application/json').send({
// Use the HTTP status text (e.g. "Bad Request" for 400)
// rather than err.name (which for FastifyError is the
// unhelpful string "FastifyError"). Matches Fastify's
// default error-body shape that pre-fix clients were
// parsing.
error: STATUS_CODES[statusCode] || 'Error',
code: err.code,
Comment on lines +188 to +195
message: err.message,
statusCode,
});
Comment on lines +187 to +198
}
};

Expand Down
82 changes: 82 additions & 0 deletions test/error-handler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,85 @@ describe('registerErrorHandler (#312)', () => {
assert.strictEqual(unhandled.length, 0, '4xx must not produce a 5xx stack log');
});
});

// #376: Fastify-internal errors that fire BEFORE any user hook
// runs (notably FST_ERR_BAD_URL on malformed percent-encoding)
// previously bypassed every CORS-injecting handler in JSS, leaving
// the 400 response with no Access-Control-Allow-* headers — browsers
// surfaced the response as a CORS error instead of the real status.
// The fix uses Fastify's `frameworkErrors` option in createServer
// to attach the full CORS header set on EVERY framework-error
// response (matching the rest of the server, where the global
// onRequest hook sets CORS unconditionally). Allow-Origin mirrors
// the request's `Origin` if present, otherwise defaults to `*`.
describe('frameworkErrors injects CORS headers on FST_ERR_BAD_URL (#376)', () => {
let server;
let baseUrl;

before(async () => {
const { createServer } = await import('../src/server.js');
server = createServer({ logger: false, forceCloseConnections: true });
await server.listen({ port: 0, host: '127.0.0.1' });
const addr = server.server.address();
baseUrl = `http://127.0.0.1:${addr.port}`;
});

after(async () => {
await server.close();
});

it('returns 400 with full CORS headers for a malformed-percent URL + Origin', async () => {
// `%g1` is the canonical FST_ERR_BAD_URL trigger — `g` isn't a
// valid hex digit, so the percent-decode bails before any route
// handler is even resolved.
const r = await fetch(`${baseUrl}/foo%g1`, {
headers: { Origin: 'https://example.com' },
});
assert.strictEqual(r.status, 400);
// CORS headers are what was missing pre-fix. ACAO should mirror
// the request Origin (not `*`), since the request explicitly
// sent one — that's what getCorsHeaders does.
assert.strictEqual(r.headers.get('access-control-allow-origin'), 'https://example.com');
assert.match(r.headers.get('access-control-allow-methods') || '', /GET/);
assert.ok(r.headers.get('access-control-allow-headers'), 'ACAH must be set');
assert.ok(r.headers.get('access-control-expose-headers'), 'ACEH must be set');
// Body uses HTTP status text ("Bad Request"), not err.name
// ("FastifyError") — matches Fastify's default body shape so
// any pre-fix client parsing `error` keeps working.
const body = await r.json();
assert.strictEqual(body.error, 'Bad Request');
assert.strictEqual(body.code, 'FST_ERR_BAD_URL');
assert.strictEqual(body.statusCode, 400);
});

it('returns CORS headers (ACAO=*) and well-shaped JSON for the same bad URL without an Origin', async () => {
// Non-browser clients without an Origin still receive the full
// CORS header set — consistent with the rest of the server,
// where the global onRequest hook always sets CORS. ACAO
// defaults to `*` when no Origin was sent (per getCorsHeaders).
const r = await fetch(`${baseUrl}/foo%g1`);
assert.strictEqual(r.status, 400);
assert.strictEqual(r.headers.get('access-control-allow-origin'), '*');
assert.match(r.headers.get('access-control-allow-methods') || '', /GET/);
assert.ok(r.headers.get('access-control-allow-headers'), 'ACAH must be set');
const body = await r.json();
assert.strictEqual(body.error, 'Bad Request');
assert.strictEqual(body.code, 'FST_ERR_BAD_URL');
assert.strictEqual(body.statusCode, 400);
});

it('does NOT regress: a normal 404 still carries CORS via the wildcard handler', async () => {
// Belt-and-suspenders: the frameworkErrors hook shouldn't have
// displaced any existing CORS behavior on responses that go
// through the wildcard handler. Ask for a path that hits the
// LDP wildcard and 404s (no such resource), confirm CORS.
const r = await fetch(`${baseUrl}/nonexistent/deep/path`, {
headers: { Origin: 'https://example.com' },
});
// Could be 401 or 404 depending on auth defaults; in either case
// CORS must be present.
assert.ok([401, 404].includes(r.status), `expected 401 or 404, got ${r.status}`);
assert.ok(r.headers.get('access-control-allow-origin'),
'ACAO must be set on the normal-handler error path too');
});
});