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

Make the JSON library configurable #27

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

SachsKaylee
Copy link

@SachsKaylee SachsKaylee commented Nov 29, 2020

This library currently is hardcoded to use Poison.

As I would like to be able to completely replace the Poison dependency with Jason in all projects this PR adds a config option to configure the module.

THIS PR COULD BE A BREAKING CHANGE, since the Poison dependency is now marked as test only. As far as I am aware, it will thus not be included in releases if a project does not explicitly depend on Poison.
It might be warranted to increase the Major version for this reason.


There is also the problem with atom keys in the maps returned by the library. Currently non existing atom keys in the JWT are created on demand, which can lead to DOS memory leaks on the server. It would thus be better to return the keys as strings or add a setting to only allow existing atoms: https://hexdocs.pm/poison/Poison.html#module-parser

Should I add this feature in this PR or open a separate for this?

@whossname
Copy link

@garyf this is an important pull request that makes this library relevant again. Modern elixir mostly uses Jason as the json library and modern elixir libraries allow users to choose which json library to use.

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