search

digitalbazaar / jsonld.js

A JSON-LD Processor and API implementation in JavaScript
https://json-ld.org/
Other
1.66k stars 195 forks source link

Remove BOM character from context if there is one #380

Closed jiaola closed 4 years ago

jiaola commented 4 years ago

Indirectly related to #358. When the context file contains a BOM character, there's an error: 

cause: SyntaxError: Unexpected token  in JSON at position 0
    at JSON.parse (<anonymous>)
    at ContextResolver._fetchContext (.../jsonld.js/lib/ContextResolver.js:159:24)

This PR fixes it.

gkellogg commented 4 years ago

Why is this something a client needs to do at all? From RFC7159, it’s illegal for, JSON data to include a BOM.

https://tools.ietf.org/html/rfc7159#section-8.1

jiaola commented 4 years ago

Good point! Let me talk with @hsolbrig about this. I wasn't aware that BOM is illegal in JSON. I'll close this PR.

hsolbrig commented 4 years ago

We'll fix the emitter.

That said, it would be handy emit a warning or error that says something a we bit more descriptive than "bad character"? My hunch is that we're not the only ones that will encounter this problem and I suspect that a goodly number of JSON users may never have heard of byte-ordering marks.

Is there a site out there that tells folks how to fix this problem in their language of choice? If not, should we add one to the best practices site?

gkellogg commented 4 years ago

There’s a good page here: https://www.w3.org/International/questions/qa-byte-order-mark

The RFC I cited shows how it’s inappropriate for application/json, and this JSON-LD

davidlehn commented 4 years ago

On the playground the BOM files were getting parsed. So the XHR loader or browser XHR itself is handling it. Node loader not so much. A "bad character" error isn't wrong, but worth considering also doing a BOM check and making that particular error more clear. Also a future documentLoader implementation could have a BOM check/ignore option for those who want it.