Switch to other JSON library

Create issue
Issue #239 new
Former user created an issue

I'm wondering, would you consider changing json-smart for another JSON provider? Java EE has standardized on an API for JSON parsing, so maybe it would make sense to leverage that?

The json-smart developer has abandoned the project, indicating he doesn't use it any longer.

Comments (29)

  1. Connect2id Support

    This library makes relatively limited use of JSON and features of the JSON Smart library. Everything that is JSON related is well tested, and there are no known or reported JSON bugs. So, if maintenance of JSON Smart has ceased, that is not much of an issue right now.

    Yes, the new JEE JSON API could be one way ahead. The switch will be a major breaking change, which will require a new major version.

  2. Matteo Gazzetta

    Another issue to consider is the support of latest java version. Currently json-smart depends on accessors-smart which depends on asm 5.0.4.

  3. Fat Conan

    It might be worth noting that the json-smart (2.3) library will throw an illegal access exception that can potentially kill the JVM in the right circumstances (Akka, for example, will catch this as a fatal error and exit the JVM if not told otherwise) if it's passed a LocalDate. In version 2.1.0 of json-smart this appeared to be handled somewhat gracefully by seemingly ignoring the task of serialising LocalDate and LocalDateTime into the JSON. Since then, however, LocalDateTime appears to behave as before, but LocalDates throw this exception.

    This bit me recently when I updated pac4j (which shifted from using 4.35 to 4.36 of nimbus) on a play project and suddenly found that tokens being created as part of a password reset process (that contained a LocalDate expiry internally) caused the site to shut down.

    I've made a little demonstration of the issue here https://github.com/FatConan/pac4jjwttest and I've also checked that it still affects version 7.0.1 of nimbus. It might not be an immediate concern for the project, but clearly it can cause issues further up the chain.

  4. Christophe

    Hi, Another reason for such a move would be to ensure Java 11+ support. This is the warning i got when running my app with JDK 11.0.2.

    WARNING: An illegal reflective access operation has occurred
    WARNING: Illegal reflective access by net.minidev.asm.DynamicClassLoader (file:/....gradle/caches/modules-2/files-2.1/net.minidev/accessors-smart/1.2/c592b500269bfde36096641b01238a8350f8aa31/accessors-smart-1.2.jar) to method java.lang.ClassLoader.defineClass(java.lang.String,byte[],int,int)
    WARNING: Please consider reporting this to the maintainers of net.minidev.asm.DynamicClassLoader
    WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
    WARNING: All illegal access operations will be denied in a future release
    
  5. Jason Emerick

    +1 for updating the JSON library usage. Running into issues with ASM that Matteo Gazzetta reported above.

  6. Michael Shafrir

    I work on the Stripe Android SDK. We're using Nimbus-JOSE-JWT as part of a new dependency, and have been extremely happy with it. The only issue we have, as mentioned above, is that the JSON dependency still relies on asm 5.0.4. This is conflicting with Robolectric 4’s dependency on asm 7.0.

  7. Evan Tatarka

    Note: it doesn’t look like asm is actually being used by nimbus, we were able to exclude it to fix the robolectric issue:

    implementation('com.nimbusds:nimbus-jose-jwt:7.1) {
        exclude(group: 'org.ow2.asm', module: 'asm')
    }
    
  8. Jurrian Fahner

    We have the following problem on java 11:

    java.lang.IllegalAccessError: class net.minidev.asm.java.net.URLAccAccess tried to access private field java.net.URL.protocol (net.minidev.asm.java.net.URLAccAccess is in unnamed module of loader net.minidev.asm.DynamicClassLoader @1a3ef3d1; java.net.URL is in module java.base of loader 'bootstrap')
        at net.minidev.asm.java.net.URLAccAccess.get(Unknown Source) ~[na:na]
        at net.minidev.json.reader.BeansWriterASM.writeJSONString(BeansWriterASM.java:21) ~[json-smart-2.3.jar:na]
        at net.minidev.json.JSONValue.writeJSONString(JSONValue.java:586) ~[json-smart-2.3.jar:na]
        at net.minidev.json.reader.JsonWriter.writeJSONKV(JsonWriter.java:392) ~[json-smart-2.3.jar:na]
        at net.minidev.json.reader.JsonWriter$7.writeJSONString(JsonWriter.java:145) ~[json-smart-2.3.jar:na]
        at net.minidev.json.reader.JsonWriter$7.writeJSONString(JsonWriter.java:1) ~[json-smart-2.3.jar:na]
        at net.minidev.json.JSONObject.writeJSON(JSONObject.java:186) ~[json-smart-2.3.jar:na]
        at net.minidev.json.JSONObject.toJSONString(JSONObject.java:74) ~[json-smart-2.3.jar:na]
        at net.minidev.json.JSONObject.toString(JSONObject.java:272) ~[json-smart-2.3.jar:na]
        at com.nimbusds.jose.Payload.toString(Payload.java:386) ~[nimbus-jose-jwt-8.2.1.jar:8.2.1]
        at com.nimbusds.jose.Payload.toBytes(Payload.java:418) ~[nimbus-jose-jwt-8.2.1.jar:8.2.1]
        at com.nimbusds.jose.Payload.toBase64URL(Payload.java:434) ~[nimbus-jose-jwt-8.2.1.jar:8.2.1]
        at com.nimbusds.jose.JWSObject.<init>(JWSObject.java:121) ~[nimbus-jose-jwt-8.2.1.jar:8.2.1]
    

    It affects also lower versions:

    Caused by: java.lang.IllegalAccessError: class net.minidev.asm.java.net.URLAccAccess tried to access private field java.net.URL.protocol (net.minidev.asm.java.net.URLAccAccess is in unnamed module of loader net.minidev.asm.DynamicClassLoader @78a6e264; java.net.URL is in module java.base of loader 'bootstrap')
        at net.minidev.asm.java.net.URLAccAccess.get(Unknown Source) ~[na:na]
        at net.minidev.json.reader.BeansWriterASM.writeJSONString(BeansWriterASM.java:21) ~[json-smart-2.3.jar:na]
        at net.minidev.json.JSONValue.writeJSONString(JSONValue.java:586) ~[json-smart-2.3.jar:na]
        at net.minidev.json.reader.JsonWriter.writeJSONKV(JsonWriter.java:392) ~[json-smart-2.3.jar:na]
        at net.minidev.json.reader.JsonWriter$7.writeJSONString(JsonWriter.java:145) ~[json-smart-2.3.jar:na]
        at net.minidev.json.reader.JsonWriter$7.writeJSONString(JsonWriter.java:1) ~[json-smart-2.3.jar:na]
        at net.minidev.json.JSONObject.writeJSON(JSONObject.java:186) ~[json-smart-2.3.jar:na]
        at net.minidev.json.JSONObject.toJSONString(JSONObject.java:74) ~[json-smart-2.3.jar:na]
        at net.minidev.json.JSONObject.toString(JSONObject.java:272) ~[json-smart-2.3.jar:na]
        at com.nimbusds.jose.Payload.toString(Payload.java:386) ~[nimbus-jose-jwt-7.8.jar:7.8]
        at com.nimbusds.jose.Payload.toBytes(Payload.java:418) ~[nimbus-jose-jwt-7.8.jar:7.8]
        at com.nimbusds.jose.Payload.toBase64URL(Payload.java:434) ~[nimbus-jose-jwt-7.8.jar:7.8]
        at com.nimbusds.jose.JWSObject.<init>(JWSObject.java:121) ~[nimbus-jose-jwt-7.8.jar:7.8]
    

    Is there a work around, so we can use java 11?

  9. Connect2id Support

    If you set JSON smart to net.minidev:json-smart:1.3.1 it should work just fine with Java 11.

  10. Rogier Selie

    Regarding the comment of Jurrian Fahrer, I have the same issue. I noticed that when I use immutable objects for json serialisation I get the same IllegalAccessError exception. When apply the lombok annotation @Setter at the same class, then the exception does not occur.

    I have a preference to use immutable objects.

  11. Pieter-jan Pintens

    In OSGi the fix to set it to 1.3.1 is broken, the manifest forces you to use 2.3. We are trying to combine this with the OIDC sdk bundle which forces you to have 1.3, it’s a bit of a mess now. Would it be possible to force the manifest version to >= 1.3.1 instead of 2.3? We rewrite the manifest now to work around this.

  12. Mario Casola

    I have the issue below with OpenJDK 11, even using json-smart 1.3.1.

    java.lang.ClassCastException: class [B cannot be cast to class [Ljava.lang.Object; ([B and [Ljava.lang.Object; are in module java.base of loader 'bootstrap')
        at net.minidev.json.reader.JsonWriter$9.writeJSONString(JsonWriter.java:231)
        at net.minidev.json.JSONValue.writeJSONString(JSONValue.java:527)
        at net.minidev.json.reader.JsonWriter.writeJSONKV(JsonWriter.java:432)
        at net.minidev.json.reader.JsonWriter$7.writeJSONString(JsonWriter.java:136)
        at net.minidev.json.reader.JsonWriter$7.writeJSONString(JsonWriter.java:118)
        at net.minidev.json.JSONObject.writeJSON(JSONObject.java:127)
        at net.minidev.json.JSONObject.toJSONString(JSONObject.java:74)
        at net.minidev.json.JSONObject.toString(JSONObject.java:215)
        at com.nimbusds.jose.Payload.toString(Payload.java:386)
        at com.nimbusds.jose.Payload.toBytes(Payload.java:418)
        at com.nimbusds.jose.Payload.toBase64URL(Payload.java:434)
        at com.nimbusds.jose.JWSObject.<init>(JWSObject.java:121)
        at com.nimbusds.jwt.SignedJWT.<init>(SignedJWT.java:57)
    

  13. Mario Casola

    @Yavor Vasilev Hi Yavor, I have been too fast in assuming the issue was because of OpenJDK 11, sorry for that. Actually the issue is when I add an object as a claim that has a byte[] variable. The error happens here:

    at net.minidev.json.reader.ArrayWriter.writeJSONString(ArrayWriter.java:12)
    
    public <E> void writeJSONString(E value, Appendable out, JSONStyle compression) throws IOException {
            compression.arrayStart(out);
            boolean needSep = false;
            for (Object o : ((Object[]) value)) {
                if (needSep)
                    compression.objectNext(out);
                else
                    needSep = true;
                JSONValue.writeJSONString(o, out, compression);
            }
            compression.arrayStop(out);
        }
    

    It tries to cast it to Object[].

    As a workaround I write my object as a string using jackson and then I use JSONObjectUtils to parse the string.

    Best,

    Mario

  14. Yavor Vasilev

    @Mario Casola Alright, I see now. The JSON lib and JSON itself doesn’t know how to deal with binary data. If the byte array represents a proper string, you could save it like that. Otherwise you could use a BASE64 encoding for binary data.

  15. Mario Casola

    @Yavor Vasilev I need to keep the binary data for some reasons and it’s not just one variable. Also, I need to exclude some other variables when I add the claim and I can do it easily using jackson annotations. The workaround does the job for now.

    Best,

    Mario

  16. Mark Lagendijk

    It seems json-smart does not support Kotlin data classes. At least we were unable to get it to work with either json-smart 2.3 or 1.3.1.

    In the end we worked around it by using Jackson’s ObjectMapper to convert any objects to Maps, via convertValue:

    claimSet.claim(name, objectMapper.convertValue(value, Any::class.java))
    

  17. Jacob Dilles

    So in contrast to https://bitbucket.org/connect2id/nimbus-jose-jwt/pull-requests/50, I was able to “shim” away JSONSmart in about 500 lines (including source headers and tests) - i.e. the JSONObject and JSONArray classes are now included in the project, and none of the API classes are changed, but serialization is handled using Jackson (which is already a dependency). I don’t have permission to create a branch but I’ve created a diff (https://www.jsdilles.com/work/nimbus-jose-jwt-drop-json-smart.diff).

    Since this project is a dependency for oauth2-oidc-sdk, it removes the json-smart dependency from there, too (with only one breaking change, since I chose LinkedHashMap over HashMap, Map.Entry needs to be explicitly added to com.nimbusds.oauth2.sdk.util.OrderedJSONObject, though this implementation is no longer necessary.

    Some notes on the implementation:

    • The majority of the implementation is in net.minidev.json.JSONShim.java including a Jackson module that handles serialization and deserialization using the shim implementations.
    • You can probably ignore the changes to POM except for swapping of json-smart to jackson-databind. BC after 1.6.0 has issues with RSASSA-PSS so I changed that from <version>[1.52,)</version>to <version>[1.52,1.60]</version>. And I bumped tink to 1.3.0, which seems to be a sweet spot between 1.2.2 and 1.4.0 with the fewest dependencies (although, it still needs org.json 🙄 )
    • The dependencies on json-smart between the two projects span 3 packages, net.minidev.json, .parser and .writer .
    • My JSONObject and JSONArray do NOT implement JSONAware interface, but do have the toJSONString() method that seems to be the primary use case for this library. This is to avoid serialization issues in the custom Jackson module without needing to use any new annotations that could cause issues with older Java versions.
    • Note that with Jackson, anything that implements JSONAware only to use something like toJSONString() { return '“' + toString() + '“'; } does not need to do so - by default, Jackson will call toString() on otherwise unannotated classes… So these can be cleaned up unless you need them elsewhere.
    • I have tried to mark @deprecated on the shim, to give a migration path forward
    • There are a couple of System.out.println in the shim class. I did not try to build your other projects, keep an eye out for -- SERIALIZE USING --> during tests ; this means that the class implements JSONAware but probably shouldn't, especially if it uses IsBasicallyString .

    There is probably some low-hanging fruit somewhere between the full culling and this, for example, com.nimbusds.jose.util.JSONObjectUtils might be updated to not create a new JSONParser instance for each parsing (this would be replaced with JSONShim.parseLegacyJSONSmart(Object) ), etc. But, without breaking changes to 3rd party code, I’m not sure how you would get rid of the rest of the usages.

  18. Yavor Vasilev

    Thanks Jacob. I hope we’ll soon find time to review this PR. We’re still mulling over what the ideal strategy would be to remove the strong JSON Smart dependency while not making the change to much of a hassle for developers.

  19. Jacob Dilles

    Sorry if that was confusing, the diff from master, not PR50 (it is completely independent of the PR).

    To see the very much smaller set of changes,

    git clone git@bitbucket.org:connect2id/nimbus-jose-jwt.git
    wget https://www.jsdilles.com/work/nimbus-jose-jwt-drop-json-smart.diff
    cd nimbus-jose-jwt
    git checkout -b iss239-shim-json-smart
    git apply ../nimbus-jose-jwt-drop-json-smart.diff
    

    etc…

    P.S. Thanks for the awesome library!

  20. Toma Velev

    Hi All!

    I'm new to the project, but I hope the two cents I have about the issue are valuable.

    The issues found around json-smart noted here (LocalDate, binary data) and not only (https://github.com/netplex) - probably should be fixed in the json-smart (v1 and v2) lib, as - they are open source and someone could pick up the support and nimbus bump the version of the dependency up. This way the solution will be long term, so - no new developer will encounter the old issues and will have the need to re-do the current workarounds. And also - This will require less changes in nimbus-jose-jwt.

    It seems the use of the library has expanded too much, so there are a lot of platforms to support - JEE, different JDKs, Android, Java Versions, Kotlin so - Obviously There is a need to make json-smart replaceable. So, embedding (shadowing) some of the json implementations within the library is not good idea IMO.

    I like the idea in this pull request: https://bitbucket.org/connect2id/nimbus-jose-jwt/pull-requests/50/wip-allow-replacing-json-smart-with/

    Having the classpath (or some external configuration) decide the specific implementation is very widespread idea in the Java Libraries in different forms - OSGI manifest attributes, Spring configuration files or annotations, classpath presense of class definitions.

    There could be also a Json Provider Setter in the JSONParserProvider and json-smart could be dropped of the classpath because of this:

    https://stackoverflow.com/questions/27216131/can-import-cause-crash-error-after-compilation "If the program never reaches the part ....., the program will normally run fine without crashing"

    I would've placed the current nimbus-json-smart and nimbus-json - Json Provider in the pull request as a part of the parent Nimbus project - so if someone just bumps the nimbus lib version up in the dependency and forgets to add the provider implementation - it will not crush with class not found. It could be integrated any other external configuration choice of json provider- and if no such is present - the json-smart to be picked.

    All the above choices of change of json lib will not require additional changes in other code - if the usage of the numbus is in "client mode" - if the client code imports and uses only com.nimbusds.jose, com.nimbusds.jwt and the standard java packages.

    Because there are several public classes and methods that return "net.minidev.json" classes that are replaced by HashMap in PR50 - the change will probably be breaking and the recommended way in my opinion is to be in another git branch or repo and not within the current master-repo, so any fixes and changes in jwt could be integrated in both versions.

  21. Vladimir Dzhuvinov

    Hi all,

    Thanks to Toma in a 9.0 dev branch the JSON Smart dependency got shaded and all JSONObject / JSONArray types in visible methods got replaced with Map<String,Object> / List<Object>.

    https://bitbucket.org/connect2id/nimbus-jose-jwt/branch/9.0-dev

    The branch will rest now for a week or two and if nothing comes up we’re going to release it.

    If you have any comments about the changes the time to speak is now :)

  22. Jacob Dilles

    @Vladimir Dzhuvinov I built this branch and have minimal errors in my application (there were a few dangling toJSONObject().toJSONString() s hanging around), so I am good with this approach. One inconsistency is there is a missing serialize() method on Payload , but it seems the toString() does the necessary work there.

    Hopefully in a future release the json-smart library can be dropped completely (see my diff for how Jackson can provide equivalent functionality) for performance reasons, but I do understand that this will take significant testing.

    Very nice work, and thank you for taking the time to address this concern!

  23. Vladimir Dzhuvinov

    @Jacob Dilles Thanks for the feedback!

    The Payload.toString() was intended for debugging purposes, to print a meaningful representation of the payload, depending on its original type.

    If you need the JOSE serialisation - call Payload.toBase64URL().toString(). Just filed a ticket to consider a serialize() shorthand: https://bitbucket.org/connect2id/nimbus-jose-jwt/issues/364

    The shimming approach of taking over net.minidev.json has a drawback - people who want to keep JSON Smart (like us) won’t be able to do that. So the shading was found to be the most neutral approach.

    Regarding performance, I remember back in the early days (2012?) we did some benchmarks and settled specifically on JSON Smart because it topped the other available libs at the time. We also decided to stay clear from data mapping (not just here), which is not at all easy to secure: https://www.cvedetails.com/vulnerability-list/vendor_id-15866/product_id-42991/Fasterxml-Jackson-databind.html

  24. Wade McAtee

    @Vladimir Dzhuvinov

    I see as of 09/06/2020 when nimbus jose 9.0 was release you removed the dependency to JSON Smart. This has had an adverse reaction to azure-spring-boot-2.2.0 which uses nimbus jose.

    Any idea if this is going to be reverted back or are there other ways that should be utilized now?

    This is my current error i get when I am using 2.2.0 and it has nimbus 9.0 attached

    Forwarding to error page from request [] due to exception [com.nimbusds.jose.shaded.json.JSONArray cannot be cast to net.minidev.json.JSONArray]
    java.lang.ClassCastException: com.nimbusds.jose.shaded.json.JSONArray cannot be cast to net.minidev.json.JSONArray
    at com.microsoft.azure.spring.autoconfigure.aad.AADAppRoleStatelessAuthenticationFilter.doFilterInternal(AADAppRoleStatelessAuthenticationFilter.java:59) ~[azure-spring-boot-2.2.0.jar:?]

  25. Vladimir Dzhuvinov

    Hi Wade,

    The objective of the 9.0 release was to shade the JSON Smart dep so it will not interfere in cases when it’s imported for some other purpose in a project, esp if that is done with a different version.

    The project you mention can get fixed by fixing the Nimbus JOSE+JWT dep to the latest stable 8.x version.

  26. Log in to comment