Repository navigation
Conversation
|
Sounds legit to me. I wonder if we could instead add an instance method? For example: fastify.setContentTypeHeaderParser((value, defaultParser) => {
// ...
})So you can encapsulate this behavior. |
|
Oh yeah ok good call. I will try that approach. Thank you for the pointer. |
557ac97 to
224d4cf
Compare
|
@jean-michelet ok this option now encapsulates, if I managed to do that correctly. Ready for review. I kind of prefer this option over #6926 |
I prefer too, if others accept more then a doc PR. |
de03845 to
2088e79
Compare
|
Rebased and pushed up this PR |
Signed-off-by: Bret Comnes <bret@comnes.org>
Signed-off-by: Bret Comnes <bret@comnes.org>
Signed-off-by: Bret Comnes <bret@comnes.org>
Signed-off-by: Bret Comnes <bret@comnes.org>
Signed-off-by: Bret Comnes <bret@comnes.org>
Signed-off-by: Bret Comnes <bret@comnes.org>
Signed-off-by: Bret Comnes <bret@comnes.org>
Co-authored-by: Jean Michelet <110341611+jean-michelet@users.noreply.github.com> Signed-off-by: Bret Comnes <166301+bcomnes@users.noreply.github.com>
2088e79 to
8cf34b8
Compare
|
I rebased again. I think this is probably the best approach |
| if (!request[kRequestContentType]) { | ||
| request[kRequestContentType] = ContentType.from(ctHeader) | ||
| request[kRequestContentType] = request[kRouteContext].contentTypeParser.parse(ctHeader) | ||
| } |
There was a problem hiding this comment.
This calls user code on an untrusted header with no guard
So if the parser throws or returns undefined, we get uncaughtException
We should wrap it in try/catch; send the error through reply.send like sync throws in body parsers; and treat a non-object result as invalid
There was a problem hiding this comment.
The request.mediaType getter needs the same.
gurgunday
left a comment
There was a problem hiding this comment.
I'm not really in favor of this, but I will review it in case others are supportive
FYI, I currently see some blockers
|
I will review more next week |
|
Which way do you lean? My preference isn't too strong. The other approach was a boolean to turn off strict validation. This way give you a hook to apply corrective action. |
Signed-off-by: Bret Comnes <bret@comnes.org>
This is a complementary or alternative escape hatch than #6926
Add a
contentTypeHeaderParserFactory(defaultParser)server option for replacing Fastify’sContent-Typeheader parsing and canonicalization policy without replacing theaddContentTypeParserbody-parser registry.The custom parser returns a structural parsed-content-type result containing
isValid,isEmpty,mediaType,type,subtype,parameters, andtoString(). TypeScript implementations can use the exportedFastifyParsedContentTypeinterface.#6926 disables strict validation and lets raw invalid headers reach RegExp/catch-all body parsers. This proposal keeps the validation lifecycle intact but makes the header interpretation policy replaceable. Either or both could land depending on what people think.
Checklist
npm run test && npm run benchmark --if-presentand the Code of conduct