Spring-boot: Consider replacing org.json in spring-boot-configuration-processor due to licence

[jgoldhammer]

Using Spring Boot 1.3.4, we recognized that spring boot is using the json library.

Details here:
https://wiki.debian.org/qa.debian.org/jsonevil

Dependency graph:
--- org.springframework.boot:spring-boot-configuration-processor:1.3.4.RELEASE
+--- org.json:json:20140107
--- org.springframework:spring-core:4.2.4.RELEASE (*)

[philwebb]

That's a very annoying clause in the license. We intentionally chose something lightweight for the configuration processor since it's a compiler plugin. Perhaps json-simple might be an option.

You can always remove spring-boot-configuration-processor if it's causing a big legal issue for you (although you won't get generated meta-data).

[jgoldhammer]

Just started hacking on it and I hit some challenges:

• org-json uses reflection to write deep objects as json
• simple-son does not use reflection to write complex objects- it just uses toString which does not work for ItemMetadata class...

What do you think? Is rewriting the tests useful?

[snicoll]

That's what the debian community states. Here is what the Apache foundation states

[philwebb]

@snicoll Interesting, thanks!

@jgoldhammer have you actually hit a legal issue here?

[jgoldhammer]

No, we are just in preparation phase for an audit of our application and
just scanned the licences of the libraries we are using. I wanna make sure
that we have no findings in terms of licences....

What do you think, Phil?

Thanks,
Jens
Phil Webb [email protected] schrieb am Fr., 13. Mai 2016 um 22:41:

@snicoll https://github.com/snicoll Interesting, thanks!

@jgoldhammer https://github.com/jgoldhammer have you actually hit a
legal issue here?

—
You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
https://github.com/spring-projects/spring-boot/issues/5929#issuecomment-219153554

[philwebb]

Unless someone wants to step-up and migrate the code I hope that the Apache legal statement will be enough for most people. Don't forget that the configuration processor is only used at compile time and isn't usually distributed with your jar.

[Davio]

The statement by Apache was updated:
CAN APACHE PRODUCTS INCLUDE WORKS LICENSED UNDER THE JSON LICENSE?
No. As of 2016-11-03 this has been moved to the 'Category X' license list. Prior to this, use of the JSON Java library was allowed. See Debian's page for a list of alternatives.

[kartoffelsup]

I've started working on this -> https://github.com/kartoffelsup/spring-boot/commit/c11f6d64d70a6f4a0b25fdcd14bb234d5c0acc14

Replaced org.json in spring-boot-configuration-metadata. If you like it, I will gladly do the same for spring-boot-configuration-processor

[philwebb]

@kartoffelsup Yes please!

[kartoffelsup]

There is indeed an issue with the writing of JSON with json-simple. Though if I'm not mistaken, the reflection serialization is only used/needed within the tests(?). I've added GSON to the test dependencies and used that to write the JSON for the tests and they run through fine. But I'm not sure if this is something we want to do. @philwebb @wilkinsona what do you think?
https://github.com/kartoffelsup/spring-boot/commit/a0407da8244c08c1f757aaf2199e2dc50ae60db9

[wilkinsona]

I think a test only dependency on something more sophisticated is fine. I have a preference for using Jackson over GSON, though. Purely because Jackson is the default JSON library that's used elsewhere in the codebase.

[kartoffelsup]

I guess it's not as simple as I had hoped. :(
I was happy as the tests ran through but it turns out there are just a lot of test cases missing. For example, having a char []as default value will result in the following JSON:

      "defaultValue": [
        w,
        o,
        r,
        d
      ],

which is invalid. This is due to the fact that '#toString' gets called if json-simple does not 'recognize' (have an instanceof check inside the writer for) the type.

[philwebb]

This might be an option https://github.com/skyscreamer/JSONassert/commit/2f3576c72a28db3292740e5c67ffbc856394af96

[snicoll]

Some public methods are now throwing Exception following to that change

[snicoll]

@philwebb those throws Exception on public methods bug me. I see that the move to the new JSON api makes it so that JSONException is now a checked exception. Can we please restore a state of affair where a we wrap that checked exception into a "runtime" JSONException as before. If we don't want to do that, can we please throw IOException, JSONException rather than a raw Exception?

[snicoll]

OK, ignore me. I realized one of my tool was using a class from the configuration processor directly. Not really something that we should call public API.