Use a shaded version of org.json in spring-boot-configuration-metadata

[tschut]

Hi,
spring-boot-configuration-metadata has a dependency on com.vaadin.external.google:android-json:0.0.20131108.vaadin1 (managed here).

This library was last released in 2013, and having it on the classpath often causes warnings like

Found multiple occurrences of org.json.JSONObject on the class path:

	jar:file:/Users/tschut/.m2/repository/com/vaadin/external/google/android-json/0.0.20131108.vaadin1/android-json-0.0.20131108.vaadin1.jar!/org/json/JSONObject.class
	jar:file:/Users/tschut/.m2/repository/org/json/json/20250107/json-20250107.jar!/org/json/JSONObject.class

(in this particular case the org.json dependency is coming in through com.google.cloud:google-cloud-bigquery:jar:2.49.0)

Interestingly, spring-boot-configuration-processor follows a different approach.

I'm not sure what the best way forward is here, perhaps spring-boot-configuration-metadata should use a similar (or the same) shaded approach as spring-boot-configuration-processor.

[snicoll]

Thanks for the report.

This library was last released in 2013

That isn't a problem. It's simple JSON reader that we need and that is mature enough not to warrant a release. org.json was the same.

I'm not sure what the best way forward is here, perhaps spring-boot-configuration-metadata should use a similar (or the same) shaded approach as spring-boot-configuration-processor.

It is a possibility. This module is primarily used by IDE teams or build-time plugins. It wasn't really meant to be embedded in an application. What is your use case?

[tschut]

Sure, age in itself is not an issue. Still, there have been vulnerabilities in org.json (example 1, example 2) and those have been fixed in newer releases. Also, the license is no longer restictive as it was before, removing what I think was the main impediment to use this library.

Our use-case is that we're providing a Spring Boot-based internal framework, and we include spring-boot-properties-migrator, which in turn depends on spring-boot-configuration-metadata.

[snicoll]

Our use-case is that we're providing a Spring Boot-based internal framework, and we include spring-boot-properties-migrator, which in turn depends on spring-boot-configuration-metadata.

Unrelated to this issue but just to reiterate what I've stated previously, you shouldn't be doing that. This artifact is only meant to be used temporary when you upgrade locally, see the documentation.

[philwebb]

Also, the license is no longer restictive as it was before, removing what I think was the main impediment to use this library.

It was indeed, but unfortunately "public domain" still doesn't work for some users so I think we'd prefer to keep the Apache 2.0 fork. The response to stleary/JSON-java#706 (comment) doesn't make me think we'll see another license change for org.json.

[tschut]

Unrelated to this issue but just to reiterate what I've stated previously, you shouldn't be doing that. This artifact is only meant to be used temporary when you upgrade locally, see the documentation.

Thanks yes, I'll see if we can remove it in an upcoming release.

It was indeed, but unfortunately "public domain" still doesn't work for some users so I think we'd prefer to keep the Apache 2.0 fork. The response to stleary/JSON-java#706 (comment) doesn't make me think we'll see another license change for org.json.

Also makes total sense. Note: some discussion on the license here, although I'm not sure the author is going to participate in that thread 🤞

Some additional considerations:

• We also see runtime issues:

java.lang.NoSuchMethodError: 'java.lang.String[] org.json.JSONObject.getNames(org.json.JSONObject)'
    at com.google.cloud.bigquery.storage.v1.JsonToProtoMessage.convertToProtoMessage(JsonToProtoMessage.java:309)

This method exists in the JSON-java library, but not the android-json one. I did a quick visual comparison of the API and I'm pretty sure there are more differences people might run into.

• spring-boot-starter-test also includes the android-json dependency, through jsonassert, although that's moving towards the JSON-java implementation as well (albeit slowly - RC release here).

I think we can agree that the current state of things is not ideal, and I sympathize with your position on the JSON-java license. Just wondering if there's anything else that can be done to improve this.

In the meantime, I'll see about adding some exclusions in our pom files, so at least our users hopefully won't run into runtime issues anymore.

[snicoll]

Both of those considerations don't look valid to me, at least in the current state of things.

spring-boot-starter-test also includes the android-json dependency, through jsonassert

We are aware and we're using this dependency for that exact reason (i.e. not adding yet another json impl, or an incompatible version).

We also see runtime issues:

I think we've established that you're not supposed to use this at runtime (in the sense of using concretely the app with it). The logic behind the properties migrator is to use it on startup, collect the report, act on the configuration, run again, etc.

That being said, 4.0.x is a good opportunity to remove com.vaadin.external.google:android-json for good as we're going to migrate to the new jsonassert version.

[tschut]

I think we've established that you're not supposed to use this at runtime (in the sense of using concretely the app with it). The logic behind the properties migrator is to use it on startup, collect the report, act on the configuration, run again, etc.

Yes we have :D I merely meant that, although I didn't see it yet, this might just as well happen in tests, when android-json will also be on the classpath.

Either way, the outcome of migrating away from this in 4.0.x sounds like a reasonable solution to me! Thanks.

[snicoll]

Yes we have :D I merely meant that, although I didn't see it yet, this might just as well happen in tests, when android-json will also be on the classpath.

You could blame jsonassert then. You're not supposed to run your tests with the properties migrator either.