Open jeswr opened 8 months ago
express.json() just for Content-Type: application/json try this middleware ? like this demo `import express from 'express'; const app = express();
app.use((req, res, next) => { if (req.is('application/ld+json')) { req.body = JSON.parse(req.rawBody); } 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 application/json
to also be able to handle application/ld+json
or any other media type with the +json
suffix as "Suffix is an augmentation to the media type definition to additionally specify the underlying structure of that media type, allowing for generic processing based on that structure and independent of the exact type's particular semantics" (https://en.wikipedia.org/wiki/Media_type).
Moved this to the correct repo.
How would you expect to see the req.body
with ls+json
? I would expect an array where each line is an item parsed as json individually. I feel personally like this would be a new middleware exported by this package if we chose to add support? Not sure I have ever seen anyone use ldjson as input to an http api before, so maybe would be good to give a description of your use case as well?
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 "application/*json
doesn't work. That is a type-is
limitation. I agree that we should have a way to allow parsing of all json subtypes.
If you receive input with +json
media type it is understood to be safe to parse as a JSON doc because it must be one. NDJSON cannot be parsed as a single JSON doc so it has a distinct mimetype application/x-ndjson
, for example.
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 am open to this being the default behavior
I don't think this should be a default behavior for .json
. I don't think api's accepting ld-json are common (elastic bulk updates is the only one I can think of) and I think it is pretty acceptable to have to turn it on. If a client started sending it and my server started crashing because I expected always an object but .json
started populating .body
with an array I would be pretty miffed.
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": "jane.doe@example.com"
},
{
"@context": "https://schema.org",
"@type": "Person",
"name": "John Smith",
"jobTitle": "Graphic Designer",
"affiliation": "Creative Design Inc.",
"email": "john.smith@creativedesign.com"
}
]
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.
Lets make sure we are talking about the same thing.
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.
application/x-ndjson
Mime has the + subclasses to deal with OP's topic specifically.
Any +json
should be fully parseable by JSON.parse
application/x-ndjson
would not be matched by something like *+json
in the proposed changes.
+1 on @jonchurch's points
How would you expect to see the req.body with ls+json? I feel personally like this would be a new middleware exported by this package if we chose to add support?
Just the result of calling JSON.parse
on the body, no different to the current behavior of the handler on the application/json
content type; since "Any +json
should be fully parseable by JSON.parse
"
Not sure I have ever seen anyone use ldjson as input to an http api before, so maybe would be good to give a description of your use case as well?
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 @rdfjs/express-handler
.
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 POST
ing of framed JSON-LD in order to issue credentials.
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