-
-
Notifications
You must be signed in to change notification settings - Fork 727
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP Add way to reject body with the wrong content type #274
base: master
Are you sure you want to change the base?
Conversation
Neat! Obviously we would want all the types to have this behavior, not just the json. There was a plan written out for how we wanted to add this (vety different from what you implemented here), I need to find that and link you to it, if you want to work on it. The method you did here was mentioned, but was quickly rejected because you cannot stack the parsers properly and we did come up with a good solution that no one has yet worked on if you would like to. |
If you're wondering, I'm searching for the conversation. From what I recall it was two main APIs, a bodyParser.only() and a bodyParser.none() or something. Basically some way to either (1) wrap a series of parsers that will 415 if none of them caused a parse and / or a middleware that will 415 when hit (i.e. an "I have declared all my parsers now, any unparsed body is rejected"). The thread had a lot more background and information, which is why I'm trying to find it. |
I was wondering about that too. Express lets you add multiple middlewares, but I didn't see any reference to "chain" or "stack" in the README so went ahead with this approach. |
Because being able to stack things is implied with how middleware works; it is not expected that a middleware will respond to something it cannot handle which is why a non matching type just "falls through" so you can stack them. Almost all the example in the README here shows this behavior as well. A common example is the statix file serving, where requesting a non existant file doesn't cause the static middleware to respond with a 404, instead it just continyes down your middleware staxk to handle it some other way. |
Something that would help inform the design is:
Middlewares are independent and generally don't know about each other, but a middleware generating a message like that would require outside knowledge. |
0ad1d88
to
2a2f471
Compare
I was messing around with an Express route I had
bodyParser.json()
on, and kind of assumed that if I gave it a random POST, it would fail. But after reading the README again, the way Body Parser works makes sense. I started to think about writing this logic apart, but it seemed really closely coupled to the logic of Body Parser, so I looked adding this.I started off writing this as an issue but I figure a diff would help illustrate the idea a lot better.
This adds a new
strictType
option. It's false by default, but when enabled, it fails requests with a HTTP 415 Unsupported Media Type errorIf there's interest, I'll finish out this PR: