Support more JSON ContentTypes in isJSON#1274
Conversation
Evynglais
force-pushed
the
main
branch
3 times, most recently
from
dbd47b9 to
dcc94f6
Compare
|
Thx, overall LGTM. Pinging @duglin if there's some SPEC-specifics which we need to consider here. |
| "strings" | ||
| ) | ||
|
|
||
| const ( |
There was a problem hiding this comment.
They're used in the StringOfApplicationJSON(), StringOfApplicationXML(), etc. functions below (which are pretty widely used), and they're also used directly in numerous places too.
|
Apologies, had to fix one of the test cases for invalid mediatype parameters. Have set it to ignore parameter issues, since this function doesn't use parameters (it won't block us from checking the subtype). Not sure how I missed that the first time! |
| } | ||
|
|
||
| // fallback to checking the suffix of the entire Content-Type: | ||
| return strings.HasSuffix(contentType, "json") |
There was a problem hiding this comment.
I might be wrong, but I think needs to check for "/json" and not just "json" because otherwise a contentType of "foojson" would return true and I'm not sure that's what we want. Thoughts?
Let's add a test for "foojson" either way to make sure it always returns the correct result
There was a problem hiding this comment.
Agreed, added the test and made it check for /json. Thanks for the suggestion :)
|
looks good - just one minor suggestion |
…nt-Types Signed-off-by: Matthew Hehir <hehir45@googlemail.com>
|
I think you need to rebase |
Done ✅ |
3 participants
Adds support for other JSON Content-Types outside of those defined in
content_type.go.See: Issue