Skip to content

Commit eb6c134

Browse files
eduardoboucasmrstork
eduardoboucas
and
mrstork
authored
fix: improve static file handler (#248)
* fix: improve static file handler * refactor: only apply headers to static files * refactor: simplify * chore: add comment * chore: add test * chore: fix eslint errors * refactor: append headers in pre middleware --------- Co-authored-by: Mateusz Bocian <[email protected]>
1 parent 3af0fdf commit eb6c134

File tree

5 files changed

+158
-46
lines changed

5 files changed

+158
-46
lines changed

‎packages/dev/src/main.test.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -671,5 +671,50 @@ describe('Handling requests', () => {
671671

672672
await fixture.destroy()
673673
})
674+
675+
test('Invoking a function that shadows a static file and introspecting the result', async () => {
676+
const fixture = new Fixture()
677+
.withFile(
678+
'netlify.toml',
679+
`[build]
680+
publish = "public"
681+
`,
682+
)
683+
.withFile(
684+
'netlify/functions/greeting.mjs',
685+
`export default async (req, context) => new Response(context.params.greeting + ", friend!");
686+
687+
export const config = { path: "/:greeting", preferStatic: true };`,
688+
)
689+
.withFile('public/hello.html', '<html>Hello</html>')
690+
.withStateFile({ siteId: 'site_id' })
691+
const directory = await fixture.create()
692+
693+
await withMockApi(routes, async (context) => {
694+
const dev = new NetlifyDev({
695+
apiURL: context.apiUrl,
696+
apiToken: 'token',
697+
projectRoot: directory,
698+
})
699+
700+
await dev.start()
701+
702+
const req1 = new Request('https://site.netlify/hi')
703+
const res1 = await dev.handleAndIntrospect(req1)
704+
705+
expect(await res1?.response.text()).toBe('hi, friend!')
706+
expect(res1?.type).toBe('function')
707+
708+
const req2 = new Request('https://site.netlify/hello')
709+
const res2 = await dev.handleAndIntrospect(req2)
710+
711+
expect(await res2?.response.text()).toBe('<html>Hello</html>')
712+
expect(res2?.type).toBe('static')
713+
714+
await dev.stop()
715+
})
716+
717+
await fixture.destroy()
718+
})
674719
})
675720
})

‎packages/dev/src/main.ts‎

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { resolveConfig } from '@netlify/config'
66
import { ensureNetlifyIgnore, getAPIToken, mockLocation, LocalState, type Logger, HTTPServer } from '@netlify/dev-utils'
77
import { EdgeFunctionsHandler } from '@netlify/edge-functions/dev'
88
import { FunctionsHandler } from '@netlify/functions/dev'
9-
import { HeadersHandler } from '@netlify/headers'
9+
import { HeadersHandler, type HeadersCollector } from '@netlify/headers'
1010
import { RedirectsHandler } from '@netlify/redirects'
1111
import { StaticHandler } from '@netlify/static'
1212

@@ -95,6 +95,18 @@ const notFoundHandler = async () => new Response('Not found', { status: 404 })
9595

9696
type Config = Awaited<ReturnType<typeof resolveConfig>>
9797

98+
interface HandleOptions {
99+
/**
100+
* An optional callback that will be called with every header (key and value)
101+
* coming from header rules.
102+
*
103+
* {@link} https://docs.netlify.com/routing/headers/
104+
*/
105+
headersCollector?: HeadersCollector
106+
}
107+
108+
export type ResponseType = 'edge-function' | 'function' | 'redirect' | 'static'
109+
98110
export class NetlifyDev {
99111
#apiHost?: string
100112
#apiScheme?: string
@@ -113,7 +125,7 @@ export class NetlifyDev {
113125
redirects: boolean
114126
static: boolean
115127
}
116-
#headersHandler: { handle: HeadersHandler['handle'] }
128+
#headersHandler?: HeadersHandler
117129
#logger: Logger
118130
#projectRoot: string
119131
#redirectsHandler?: RedirectsHandler
@@ -143,13 +155,16 @@ export class NetlifyDev {
143155
static: options.staticFiles?.enabled !== false,
144156
}
145157
this.#functionsServePath = path.join(projectRoot, '.netlify', 'functions-serve')
146-
this.#headersHandler = { handle: async (_request: Request, response: Response) => response }
147158
this.#logger = options.logger ?? globalThis.console
148159
this.#server = options.serverAddress
149160
this.#projectRoot = projectRoot
150161
}
151162

152-
private async handleInEphemeralDirectory(request: Request, destPath: string) {
163+
private async handleInEphemeralDirectory(
164+
request: Request,
165+
destPath: string,
166+
options: HandleOptions = {},
167+
): Promise<{ response: Response; type: ResponseType } | undefined> {
153168
// Try to match the request against the different steps in our request chain.
154169
//
155170
// https://docs.netlify.com/platform/request-chain/
@@ -158,7 +173,7 @@ export class NetlifyDev {
158173
// with both modes of cache (manual and off) by running them serially.
159174
const edgeFunctionResponse = await this.#edgeFunctionsHandler?.handle(request.clone())
160175
if (edgeFunctionResponse) {
161-
return edgeFunctionResponse
176+
return { response: edgeFunctionResponse, type: 'edge-function' }
162177
}
163178

164179
// 2. Check if the request matches a function.
@@ -171,12 +186,15 @@ export class NetlifyDev {
171186

172187
if (staticMatch) {
173188
const response = await staticMatch.handle()
174-
return this.#headersHandler.handle(request, response)
189+
190+
await this.#headersHandler?.apply(request, response, options.headersCollector)
191+
192+
return { response, type: 'static' }
175193
}
176194
}
177195

178196
// Let the function handle the request.
179-
return functionMatch.handle(request)
197+
return { response: await functionMatch.handle(request), type: 'function' }
180198
}
181199

182200
// 3. Check if the request matches a redirect rule.
@@ -187,33 +205,40 @@ export class NetlifyDev {
187205
// we'll follow the redirect rule.
188206
const functionMatch = await this.#functionsHandler?.match(new Request(redirectMatch.target), destPath)
189207
if (functionMatch && !functionMatch.preferStatic) {
190-
return functionMatch.handle(request)
208+
return { response: await functionMatch.handle(request), type: 'function' }
191209
}
192210

193211
const response = await this.#redirectsHandler?.handle(
194212
request,
195213
redirectMatch,
196214
async (maybeStaticFile: Request) => {
197215
const staticMatch = await this.#staticHandler?.match(maybeStaticFile)
198-
199-
if (!staticMatch) return
216+
if (!staticMatch) {
217+
return
218+
}
200219

201220
return async () => {
202221
const response = await staticMatch.handle()
203-
return this.#headersHandler.handle(new Request(redirectMatch.target), response)
222+
223+
await this.#headersHandler?.apply(new Request(redirectMatch.target), response, options.headersCollector)
224+
225+
return response
204226
}
205227
},
206228
)
207229
if (response) {
208-
return response
230+
return { response, type: 'redirect' }
209231
}
210232
}
211233

212234
// 4. Check if the request matches a static file.
213235
const staticMatch = await this.#staticHandler?.match(request)
214236
if (staticMatch) {
215237
const response = await staticMatch.handle()
216-
return this.#headersHandler.handle(request, response)
238+
239+
await this.#headersHandler?.apply(request, response, options.headersCollector)
240+
241+
return { response, type: 'static' }
217242
}
218243
}
219244

@@ -236,7 +261,13 @@ export class NetlifyDev {
236261
return config
237262
}
238263

239-
async handle(request: Request) {
264+
async handle(request: Request, options: HandleOptions = {}) {
265+
const result = await this.handleAndIntrospect(request, options)
266+
267+
return result?.response
268+
}
269+
270+
async handleAndIntrospect(request: Request, options: HandleOptions = {}) {
240271
const requestID = generateRequestID()
241272

242273
request.headers.set('x-nf-request-id', requestID)
@@ -246,7 +277,7 @@ export class NetlifyDev {
246277
const destPath = await fs.mkdtemp(path.join(this.#functionsServePath, `${requestID}_`))
247278

248279
try {
249-
return await this.handleInEphemeralDirectory(request, destPath)
280+
return await this.handleInEphemeralDirectory(request, destPath, options)
250281
} finally {
251282
try {
252283
await fs.rm(destPath, { force: true, recursive: true })

‎packages/headers/src/main.test.ts‎

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ describe('constructor', () => {
1313
const projectDir = await fixture.create()
1414

1515
const handler = new HeadersHandler({
16+
logger: console,
1617
projectDir,
1718
publishDir: '.', // resolves to the same path as `projectDir`
1819
configHeaders: [{ for: '/test-*', values: { 'X-Config-Header': 'config-value' } }],
@@ -24,12 +25,13 @@ describe('constructor', () => {
2425
})
2526
})
2627

27-
describe('handle', () => {
28+
describe('apply', () => {
2829
test('sets response headers matching rules in provided `configHeaders`', async () => {
2930
const fixture = new Fixture()
3031
const projectDir = await fixture.create()
3132

3233
const handler = new HeadersHandler({
34+
logger: console,
3335
projectDir,
3436
configHeaders: [{ for: '/test-*', values: { 'X-Config-Header': 'config-value' } }],
3537
})
@@ -39,10 +41,11 @@ describe('handle', () => {
3941
'Existing-Header': 'existing-value',
4042
},
4143
})
42-
const result = await handler.handle(request, response)
44+
const result = await handler.apply(request, response)
4345

44-
expect(result.headers.get('X-Config-Header')).toBe('config-value')
45-
expect(result.headers.get('Existing-Header')).toBe('existing-value')
46+
expect(result).toStrictEqual({ 'X-Config-Header': 'config-value' })
47+
expect(response.headers.get('X-Config-Header')).toBe('config-value')
48+
expect(response.headers.get('Existing-Header')).toBe('existing-value')
4649

4750
await fixture.destroy()
4851
})
@@ -53,6 +56,7 @@ describe('handle', () => {
5356
})
5457
const projectDir = await fixture.create()
5558
const handler = new HeadersHandler({
59+
logger: console,
5660
projectDir,
5761
configHeaders: undefined,
5862
})
@@ -63,10 +67,11 @@ describe('handle', () => {
6367
'Existing-Header': 'existing-value',
6468
},
6569
})
66-
const result = await handler.handle(request, response)
70+
const result = await handler.apply(request, response)
6771

68-
expect(result.headers.get('X-Project-Dir-Header')).toBe('project-dir-value')
69-
expect(result.headers.get('Existing-Header')).toBe('existing-value')
72+
expect(result).toStrictEqual({ 'X-Project-Dir-Header': 'project-dir-value' })
73+
expect(response.headers.get('X-Project-Dir-Header')).toBe('project-dir-value')
74+
expect(response.headers.get('Existing-Header')).toBe('existing-value')
7075

7176
await fixture.destroy()
7277
})
@@ -79,6 +84,7 @@ describe('handle', () => {
7984
const projectDir = await fixture.create()
8085

8186
const handler = new HeadersHandler({
87+
logger: console,
8288
projectDir,
8389
publishDir: 'my-dist',
8490
configHeaders: undefined,
@@ -89,10 +95,11 @@ describe('handle', () => {
8995
'Existing-Header': 'existing-value',
9096
},
9197
})
92-
const result = await handler.handle(request, response)
98+
const result = await handler.apply(request, response)
9399

94-
expect(result.headers.get('X-Publish-Dir-Header')).toBe('publish-dir-value')
95-
expect(result.headers.get('Existing-Header')).toBe('existing-value')
100+
expect(result).toStrictEqual({ 'X-Publish-Dir-Header': 'publish-dir-value' })
101+
expect(response.headers.get('X-Publish-Dir-Header')).toBe('publish-dir-value')
102+
expect(response.headers.get('Existing-Header')).toBe('existing-value')
96103

97104
await fixture.destroy()
98105
})
@@ -108,6 +115,7 @@ describe('handle', () => {
108115
})
109116
const projectDir = await fixture.create()
110117
const handler = new HeadersHandler({
118+
logger: console,
111119
projectDir,
112120
publishDir: 'my-dist',
113121
configHeaders: [{ for: '/test-pa*', values: { 'X-Config-Header': 'config-value' } }],
@@ -119,12 +127,22 @@ describe('handle', () => {
119127
'Existing-Header': 'existing-value',
120128
},
121129
})
122-
const result = await handler.handle(request, response)
130+
const expected = {
131+
'X-Project-Dir-Header': 'project-dir-value',
132+
'X-Publish-Dir-Header': 'publish-dir-value',
133+
'X-Config-Header': 'config-value',
134+
}
135+
const collected: Record<string, string> = {}
136+
const result = await handler.apply(request, response, (key, value) => {
137+
collected[key] = value
138+
})
123139

124-
expect(result.headers.get('X-Project-Dir-Header')).toBe('project-dir-value')
125-
expect(result.headers.get('X-Publish-Dir-Header')).toBe('publish-dir-value')
126-
expect(result.headers.get('X-Config-Header')).toBe('config-value')
127-
expect(result.headers.get('Existing-Header')).toBe('existing-value')
140+
expect(result).toStrictEqual(expected)
141+
expect(collected).toStrictEqual(expected)
142+
expect(response.headers.get('X-Project-Dir-Header')).toBe('project-dir-value')
143+
expect(response.headers.get('X-Publish-Dir-Header')).toBe('publish-dir-value')
144+
expect(response.headers.get('X-Config-Header')).toBe('config-value')
145+
expect(response.headers.get('Existing-Header')).toBe('existing-value')
128146

129147
await fixture.destroy()
130148
})
@@ -135,6 +153,7 @@ describe('handle', () => {
135153
})
136154
const projectDir = await fixture.create()
137155
const handler = new HeadersHandler({
156+
logger: console,
138157
projectDir,
139158
configHeaders: [{ for: '/no-match', values: { 'X-Config-Header': 'no-match-value' } }],
140159
})
@@ -146,9 +165,10 @@ describe('handle', () => {
146165
},
147166
})
148167
const originalHeaders = new Headers(response.headers)
149-
const result = await handler.handle(request, response)
168+
const result = await handler.apply(request, response)
150169

151-
expect(result.headers).toEqual(originalHeaders)
170+
expect(result).toStrictEqual({})
171+
expect(response.headers).toEqual(originalHeaders)
152172

153173
await fixture.destroy()
154174
})

‎packages/headers/src/main.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import { headersForPath } from './lib/headersForPath.js'
77

88
const HEADERS_FILE_NAME = '_headers'
99

10+
export type HeadersCollector = (key: string, value: string) => void
11+
1012
interface HeadersHandlerOptions {
1113
configPath?: string | undefined
1214
configHeaders?: ConfigHeader[] | undefined
@@ -45,7 +47,7 @@ export class HeadersHandler {
4547
return this.#headersFiles
4648
}
4749

48-
async handle(request: Request, response: Response) {
50+
async apply(request: Request, response?: Response, collector?: HeadersCollector) {
4951
const headerRules = await parseHeaders({
5052
headersFiles: this.#headersFiles,
5153
configPath: this.#configPath,
@@ -55,9 +57,11 @@ export class HeadersHandler {
5557
const matchingHeaderRules = headersForPath(headerRules, new URL(request.url).pathname)
5658

5759
for (const [key, value] of Object.entries(matchingHeaderRules)) {
58-
response.headers.set(key, value)
60+
response?.headers.set(key, value)
61+
62+
collector?.(key, value)
5963
}
6064

61-
return response
65+
return matchingHeaderRules
6266
}
6367
}

0 commit comments

Comments
 (0)