search

credential-handler / authn.io

Credential Mediator Polyfill
https://github.com/w3c-ccg/credential-handler-api
Other
44 stars 8 forks source link

Use JSON parsing not media types to determine if a response is JSON #163

Closed BigBlueHat closed 9 months ago

BigBlueHat commented 9 months ago

These lines only looks for a couple variations of JSON media types (for one thing) and do not handle situations where a server may be misconfigured to not (yet?) support filename-to-media-type mappings such as .webmanifest to application/manifest+json: https://github.com/credential-handler/authn.io/blob/e35a7253a262554e1d2a973bb4736289ffef41e0/web/mediator/manifest.js#L46

Consequently, servers that do not know that mapping will often default to text/html or application/octet-stream instead. However, the response is still actually valid JSON (or may be), so we should at least attempt a JSON.parse().

Related to #159

dlongley commented 9 months ago

Well, we might just want to fail in those cases to help improve the situation. I'd prefer we continue to fail -- and that we have documentation and good errors that explain to devs why their manifest isn't loading, so they can fix it.

The only reason, IMO, to accept a bad media type here -- is if we discover that it's just not feasible (for some sufficiently large dev user base) to properly set the media type even if they knew they needed to.

BigBlueHat commented 9 months ago

Well...it came up because we didn't have it set in our Apache deployment for https://github.com/credential-handler/chapi-demo-wallet That said, it was an easy fix to update the Apache config...so, I'll agree getting the world to improve vs. paving the (broken) cattle trail is best.