-
-
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
json middleware does not work on content types with a +
#519
Comments
express.json() just for Content-Type: application/json try this middleware ? like this demo app.use((req, res, next) => { app.use(express.json()); |
Thank you for the sample code. I will leave this issue open, as the main reason I opened this issue is in the interest following the robustness principle / postels law - in particular I would expect that any server which can handle |
Moved this to the correct repo. How would you expect to see the |
Folks can do this today but have to configure it like this // docs https://expressjs.com/en/api.html#express.json
app.use(express.json({type: ['application/*+json', 'application/json']})) Unfortunately If you receive input with I am open to this being the default behavior. Im sure 99% of people are just using application/json, so it's a small gain which wouldn't be worth it for correctness alone if it had some downside. Im not aware of any real ones though except additional load/risk parsing payloads you weren't expecting. They should still be JSON though and you'd have to go out of your way to enable parsing a valid json doc if you did want to currently. lil repro: const express = require('express')
const bodyParser = require('body-parser')
const app = express()
// app.use(express.json({type: "*/*+json"}))
app.use(express.json({type: ['application/*+json', 'application/json']}))
app.post('/', async (req, res) => {
console.log("Data posted", await req.body);
});
app.listen(3000, () => {
console.log('Listening on port 3000');
fetch('http://localhost:3000', {
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify({
message: 'application/json',
}),
});
fetch('http://localhost:3000', {
method: 'POST',
headers: {
'Content-Type': 'application/ld+json',
},
body: JSON.stringify({
message: 'application/ld+json',
}),
});
}); |
I don't think this should be a default behavior for |
I agree Id be miffed if valid json crashed my server and that it is very unlikely for someone to use one of these for input. We can shelve the idea of it being a default for now, but... ldjson isnt necessarily an array. But an array is still valid JSON so thats an API implementation detail you can run into regardless of your specific json sub mimetype. Lets make sure we are talking about the same thing. application/ld+json is Linked Data JSON, its still a valid JSON blob, or potentially a top level array of ldjson objects. This is valid ldjson, but the fact it is in an array is not inherently part of what makes it ldjson. It can also be a single top level object, same as any JSON blob [
{
"@context": "https://schema.org",
"@type": "Person",
"name": "Jane Doe",
"jobTitle": "Software Engineer",
"affiliation": "Example Corporation",
"email": "[email protected]"
},
{
"@context": "https://schema.org",
"@type": "Person",
"name": "John Smith",
"jobTitle": "Graphic Designer",
"affiliation": "Creative Design Inc.",
"email": "[email protected]"
}
]
But the toplevel array possibility is universal to all valid JSON so far as I know, that possibility doesnt relate to ldjson. I agree it could be confusing, but I am still looking for an actual downside. It is equally confusing that we dont parse valid JSON objects OData spec looks a lot like ld+json, OData just doesnt use anything but application/json as mime when the payload is json. |
I think I just accidentally wrote ld and not nd. Anyway, can't keep up with this convo right now I just wanted to drop a note that doing this by default is something we should avoid if we can imo. |
Heard. Ndjson has its own mimetype and would not be impacted by the suggested change.
Mime has the + subclasses to deal with OP's topic specifically. Any
|
+1 on @jonchurch's points
Just the result of calling
I'm currently working on a project which involves conversational Web agents that are posting RDF to one another, granted I'm parsing the body as a dataset most of the time using This would come up in production settings when implementing specs like Verifiable Credentials where there was a division between spec writers on whether to use plain JSON or RDF representations for the VCs. The compromise was that framed JSON-LD would be used. The specification uses |
The json middleware does not parse results as json if the content type is anything but
application/json
, I would expect it to also work on content types likeapplication/ld+json
. For instance:returns
On the other hand
returns
The text was updated successfully, but these errors were encountered: