feat: make root optional when serve is false #460 #536

Dipali127 · 2025-10-07T12:12:08Z

Summary

This PR makes the root option optional when serve: false in fastifyStatic asynchronous function.
Previously, a dummy directory was required just to satisfy validation.

Changes

Updated index.js to skip root validation if serve: false.
Added a new test serve-option.test.js to verify behavior with and without serving.

Tests

serve: false → no automatic routes are created, sendFile still works.
serve: true → normal static file serving works as expected.

Fixes #460

Fdawgs · 2025-10-07T18:16:31Z

@Dipali127 The index.js change is missing.

Fdawgs

Can you revert the stylistic changes please as they're not needed and are adding noise when reviewing.

Dipali127 · 2025-10-10T06:37:54Z

@Fdawgs I reverted the stylistic changes as requested.
The PR now only contains the functional change for skipping root validation when serve is false.

Copilot

Pull Request Overview

This PR adds support for making the root option optional when serve: false is specified in fastifyStatic, allowing users to skip providing a dummy directory when they only want to use sendFile functionality.

Conditionally skip root validation when serve: false
Add comprehensive tests to verify both serving and non-serving behavior

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
index.js	Added conditional logic to skip root validation when serve is false
test/serve-option.test.js	New test file to verify serve option behavior with and without static file serving
test/root/example.html	Test fixture HTML file for serving tests

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

test/serve-option.test.js

gurgunday

lgtm

Dipali127 · 2025-10-14T07:11:20Z

Hi @Fdawgs, I’ve resolved all the Copilot comments and fixed the lint issues. The PR is now ready for final review and merge. 😊

is2ei · 2025-10-14T08:27:34Z

@Dipali127

The type should also be updated if the root option can be undefined.
For example:
https://github.com/Dipali127/fastify-static/blob/serve-false-root-optional/types/index.d.ts#L88

Also, could you please confirm that the issues mentioned in the comment below are resolved by your changes?
#467 (review)

It may fail to serve files because no root is provided. You can see that the root path is passed everywhere in the codebase. You must provide a test to ensure it works correctly.
It could lead to a security issue because, by default, serving files could escape to anywhere. You should at least default the root to the current working directory (CWD) and update the README.

is2ei · 2025-10-14T08:28:54Z

index.js

-  opts.root = normalizeRoot(opts.root)
-  checkRootPathForErrors(fastify, opts.root)
+  if (opts.serve !== false) {
+    opts.root = normalizeRoot(opts.root)


@Dipali127
Why should normalizeRoot also be skipped?

In this scenario we are not using root if I'm not wrong

Dipali127 · 2025-10-14T10:58:37Z

Hi @is2ei 👋, thank you for the feedback!

I’ve updated the PR with the following changes and clarifications:

Type update:
I’ve made the root option optional in types/index.d.ts since it’s no longer required when serve: false.
This aligns the type definition with the current logic, where developers can use reply.sendFile() without specifying a root at registration time.

Regarding skipping normalizeRoot():
When serve: false, the plugin does not automatically serve static files.
In this mode, developers are expected to manually call reply.sendFile() — where they can explicitly pass a root if needed.
Therefore, normalizing or validating root at registration time isn’t required.
However, if you prefer that normalizeRoot() should always be called regardless of serve, I can adjust the code to normalize root before the if (opts.serve !== false) condition.

About issue #467:
My PR does not address issue #467 — it specifically focuses on making root optional when serve: false.
The potential serving or security concerns mentioned in #467 aren’t part of the scope of this change.

Let me know your thoughts — I can update the normalizeRoot handling based on your preferred behavior. 😊

Fdawgs · 2025-10-15T08:01:28Z

Thanks @Dipali127, could you update the types tests and then i'm happy.

Thanks for taking a look @is2ei.

Uzlopak · 2025-10-15T08:55:37Z

@Dipali127

Could you please disclose any use of AI usage for this PR?

Uzlopak

Suspected AI use.

Fdawgs · 2025-10-15T09:01:56Z

Suspected AI use.

I though so over here in #537 (comment) as the em-dash is suspect but wanted to give benefit of the doubt.

Eomm · 2025-10-15T11:59:36Z

types/index.d.ts


  export interface FastifyStaticOptions extends SendOptions {
-    root: string | string[] | URL | URL[];
+    root?: string | string[] | URL | URL[];


Boring to write, but we need something like:

export interface FastifyStaticOptions extends SendOptions { ...stuff } & ({ serve: false, } | { serve: true, root: string | string[] | URL | URL[]; })

Eomm · 2025-10-15T12:00:03Z

index.js

-  opts.root = normalizeRoot(opts.root)
-  checkRootPathForErrors(fastify, opts.root)
+  if (opts.serve !== false) {
+    opts.root = normalizeRoot(opts.root)


In this scenario we are not using root if I'm not wrong

Eomm · 2025-10-15T12:01:42Z

test/serve-option.test.js

+
+  t.after(() => fastify.close())
+  await fastify.listen({ port: 0 })
+  fastify.server.unref()


Suggested change

fastify.server.unref()

t.after(() => fastify.close())

Eomm · 2025-10-15T12:01:51Z

test/serve-option.test.js

+
+  t.after(() => fastify.close())
+  await fastify.listen({ port: 0 })
+  fastify.server.unref()


Suggested change

fastify.server.unref()

t.after(() => fastify.close())

Uzlopak · 2025-10-15T15:29:59Z

I close this in favor of #540

feat: make root optional when serve is false fastify#460

1000621

fix: skip root validation when serve is false fastify#460

1e4db33

Fdawgs requested changes Oct 8, 2025

View reviewed changes

Dipali127 added 2 commits October 9, 2025 13:48

style: add space before parentheses in function declarations

bcbeb56

style: add space before parentheses in function declarations

0f6bb37

Fdawgs requested a review from Copilot October 10, 2025 08:41

Copilot AI reviewed Oct 10, 2025

View reviewed changes

test/serve-option.test.js Outdated Show resolved Hide resolved

test/serve-option.test.js Outdated Show resolved Hide resolved

test/serve-option.test.js Outdated Show resolved Hide resolved

fix: lint issues

47a3de4

gurgunday approved these changes Oct 11, 2025

View reviewed changes

test: add tests for serve: false option

7616ffc

Fdawgs requested a review from a team October 14, 2025 07:29

is2ei reviewed Oct 14, 2025

View reviewed changes

fix(types): make root option optional when serve is false

b67309e

Merge branch 'main' into serve-false-root-optional

8c39b1b

Uzlopak requested changes Oct 15, 2025

View reviewed changes

Eomm reviewed Oct 15, 2025

View reviewed changes

Uzlopak closed this Oct 15, 2025

gurgunday reopened this Oct 16, 2025

gurgunday closed this Oct 16, 2025

Uh oh!

feat: make root optional when serve is false #460 #536

feat: make root optional when serve is false #460 #536

Uh oh!

Conversation

Dipali127 commented Oct 7, 2025

Summary

Changes

Tests

Uh oh!

Fdawgs commented Oct 7, 2025

Uh oh!

Fdawgs left a comment

Choose a reason for hiding this comment

Uh oh!

Dipali127 commented Oct 10, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gurgunday left a comment

Choose a reason for hiding this comment

Uh oh!

Dipali127 commented Oct 14, 2025

Uh oh!

is2ei commented Oct 14, 2025

Uh oh!

is2ei Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

Eomm Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

Dipali127 commented Oct 14, 2025

Uh oh!

Fdawgs commented Oct 15, 2025

Uh oh!

Uzlopak commented Oct 15, 2025

Uh oh!

Uzlopak left a comment

Choose a reason for hiding this comment

Uh oh!

Fdawgs commented Oct 15, 2025

Uh oh!

Eomm Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

Eomm Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

Eomm Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

Eomm Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

Uzlopak commented Oct 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants