Skip to content

feat: allow manually registering the attachFieldsToBody preValidation hook - #634

Open
Truta446 wants to merge 2 commits into
fastify:mainfrom
Truta446:feat/599-attach-fields-to-body-hook
Open

feat: allow manually registering the attachFieldsToBody preValidation hook#634
Truta446 wants to merge 2 commits into
fastify:mainfrom
Truta446:feat/599-attach-fields-to-body-hook

Conversation

@Truta446

Copy link
Copy Markdown

Summary

Fixes #599.

When attachFieldsToBody is enabled, the plugin registers its own
preValidation hook at the instance level. Because instance-level hooks
always run before route-level ones (and in registration order relative to
other instance-level hooks in the same scope), users had no way to control
this hook's position relative to their own preValidation hooks — e.g. a
hook that verifies a signature on the raw body before any field parsing
happens.

This PR extracts the hook body into a method decorated on the request,
request.attachFieldsToBody(), and adds a new attachFieldsToBodyHook
option (default true, matching current behavior). When set to false,
the plugin skips automatic registration and the user calls
request.attachFieldsToBody() themselves, from whichever hook and in
whatever order they choose:

fastify.register(require('@fastify/multipart'), { attachFieldsToBody: true, attachFieldsToBodyHook: false })

fastify.addHook('preValidation', async function (req) {
  await verifySignature(req) // runs first, now controllable

  if (req.isMultipart()) {
    await req.attachFieldsToBody()
  }
})

No behavior change when attachFieldsToBodyHook is left unset.

Test plan

  • Added a test in test/multipart-attach-body.test.js reproducing the
    issue's scenario: a custom preValidation hook that must run before
    field parsing, using attachFieldsToBodyHook: false to control ordering.
  • npm run test:unit — 91/91 passing, 100% coverage.
  • npm run test:typescript — passing.
  • npm run lint — clean.

@climba03003 climba03003 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verify signature means it needs to read the payload.
When the payload is consumed by the user, attachFieldsToBody can't do anything later.

@Truta446

Copy link
Copy Markdown
Author

You're absolutely right, verifying a signature means reading the payload, and once it's read attachFieldsToBody() has nothing left to parse. That use case isn't what this option solves; it belongs in a preParsing hook that tees the stream (which is what #620 enables). I've dropped the signature-verification framing from this PR: the README example now shows a correct use case (rejecting early, before the plugin buffers the upload, using headers/token rather than the body), plus an explicit note that attachFieldsToBody() consumes the stream and that payload-reading concerns like signature verification should tee the stream in preParsing instead. The test was updated to match.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support manual hook injection

3 participants