Skip to content

Conversation

sam
Copy link

@sam sam commented Apr 20, 2016

The json4s-native backend is broken in several ways. It encodes all JSON, which is allowed, but non-standard since JSON is required to be UTF-8 encoded, and it fails to properly escape UTF-8 control characters, which can often show up in localized strings. See: json4s/json4s#333 (comment)

The breakage introduced in json4s 3.3.x was rolled back and corrected in json4s/json4s#339 as part of 3.4.0-SNAPSHOT, but that hasn't been published yet. Using the Jackson backend works around this because the breakage is limited to the native implementation.

This depends on the previous sbt-pgp fix PR.

Sam Smoot added 2 commits April 20, 2016 15:08
The json4s-native backend is broken in several ways. It encodes all JSON, which is allowed, but non-standard since JSON is required to be UTF-8 encoded, and it fails to properly escape UTF-8 control characters, which can often show up in localized strings. See: json4s/json4s#333 (comment)

The breakage introduced in json4s 3.3.x was rolled back and corrected in PR339 as part of 3.4.0-SNAPSHOT, but that hasn't been published yet. Using the Jackson backend works around this because the breakage is limited to the native implementation.
@adamw
Copy link
Member

adamw commented Apr 21, 2016

Thanks! That's interesting to know about json4s-native in general :) Do you know when 3.4.0 will be available?

I'm a bit reluctant to use json4s-jackson as this introduces significant new dependencies to the project. Or ideally, would be best not to depend on any specific json library, but then it would complicate the rather simple jwt module significantly

@sam
Copy link
Author

sam commented Apr 21, 2016

I don't know when 3.4.0 will be available. It's been a few months, I would've thought the issue deserved a quick release since it was a regression from the working 3.2.x (and the broken 3.3.0 has been out since October 2015).

That said, if you're using json4s-jackson 3.3.x is fine (AFAIK; it's what we've used since February and haven't run into any further issues) since this only affects json4s-native.

I'm pretty sure if your JWT payload includes control characters (like \u0009, the ASCII tab-character), the code right now will produce invalid JSON. I would guess browser JSON parsers might be liberal enough to make this a non-issue, but I haven't tested it. It could just blow up.

I'll see if I can submit a PR with a failing spec?

Or any other tiny JSON library is cool too. I use and mostly like json4s. Just knowing that it's possible to produce non-conforming, broken JSON with json4s-native gets under my skin and I don't want to get the bug report for my app to fix an issue that could've been avoided.

Not to be too pushy. Just trying to make sure I'm describing the issue clearly and trying to justify why I think anything but json4s-native would be preferable. Thanks for your consideration.

@adamw
Copy link
Member

adamw commented Apr 22, 2016

Ok, let's take the safe route and use the jackson version for now. Thanks! :)

@adamw adamw merged commit f885415 into softwaremill:master Apr 22, 2016
@adamw
Copy link
Member

adamw commented Apr 22, 2016

Released as 0.2.6 - thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants