Skip to content
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

RDF convertors should not read or write plain JSON #1242

Closed
RubenVerborgh opened this issue Mar 28, 2022 · 8 comments
Closed

RDF convertors should not read or write plain JSON #1242

RubenVerborgh opened this issue Mar 28, 2022 · 8 comments
Assignees
Labels
🐛 bug Something isn't working

Comments

@RubenVerborgh
Copy link
Member

As discovered by @joepio in #275

Terminal 1

git checkout 3c32466d8873e73328f678acce1260fea02cabc9 # latest main
npm ci
npm start

Terminal 2

echo '{}' | curl -T - -H "Content-Type: application/json" http://localhost:3000/test
curl -H "Accept: text/turtle" http://localhost:3000/test

Log

2022-03-28T20:27:59.225Z [BaseHttpServerFactory] info: Received GET request for /test
2022-03-28T20:27:59.241Z [WebAclReader] info: Reading ACL statements from http://localhost:3000/.acl
2022-03-28T20:27:59.249Z [StreamUtil] warn: Piped stream errored with Missing context link header for media type application/json on http://localhost:3000/test
2022-03-28T20:27:59.250Z [StreamUtil] warn: Piped stream errored with Missing context link header for media type application/json on http://localhost:3000/test
2022-03-28T20:27:59.252Z [StreamUtil] warn: Piped stream errored with Missing context link header for media type application/json on http://localhost:3000/test
2022-03-28T20:27:59.252Z [BasicResponseWriter] error: Writing to HttpResponse failed with message Missing context link header for media type application/json on http://localhost:3000/test
2022-03-28T20:28:00.255Z [GuardedStream] error: No error listener was attached but error was thrown: aborted

Expected

200 or 406

@RubenVerborgh RubenVerborgh added the 🐛 bug Something isn't working label Mar 28, 2022
@RubenVerborgh
Copy link
Member Author

RubenVerborgh commented Mar 28, 2022

Conceptually, we need to extend RdfToQuadConverter with

  public async canHandle(args: RepresentationConverterArgs): Promise<void> {
    await super.canHandle(args);

    const { metadata } = args.representation;
    if (metadata.contentType === APPLICATION_JSON) {
      // Check if the Link header for the context is present
    }
  }

Whereas we can assume that our backend will always use application/ld+json, clients unfortunately may not.

In that sense, this converter has an interesting issue:

const data = pipeSafely(rawQuads, pass, (error): Error => new BadRequestHttpError(error.message));

Who says it's the client making the error? In this case, it's the server, so it would be a 500 (see #1243).

@joachimvh
Copy link
Member

I thought there was an issue about this somewhere but I couldn't find it so maybe I'm wrong. But yes, that's an issue if you use an RDF accept header on a JSON document. The getOutputTypes function should also be updated so it doesn't return results if the input type is JSON. Easiest is to just drop JSON from the input types being passed to the base class in the constructor.

@RubenVerborgh
Copy link
Member Author

Easiest is to just drop JSON from the input types being passed to the base class in the constructor.

But then clients sending valid JSON-LD might will be in trouble.

@joachimvh
Copy link
Member

But then clients sending valid JSON-LD might will be in trouble.

...if they are using application/json as content-type. And our server does not support this anyway as this would require us to parse the context header and put it in metadata to be used later by the converter.

@RubenVerborgh
Copy link
Member Author

Perfect. But then we need to reject it.

@joachimvh
Copy link
Member

That is going to require a MetadataParser that instead of putting header data into metadata throws an error if it sees a context header (specifically with application/json content type perhaps).

@RubenVerborgh
Copy link
Member Author

Split off that task at #1244

@RubenVerborgh RubenVerborgh changed the title Reading a JSON resource with RDF Accept headers crashes request RDF convertors should not read or write plain JSON Mar 29, 2022
@Falx Falx self-assigned this Jun 28, 2022
@joachimvh
Copy link
Member

Done in #1359

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants