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

[snussbaumer]

Spring cloud function comes with a org.json.JSONObject.class that is different than the one packaged with spring boot starter test. This creates the following warning when starting the application :

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

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

You can reproduce this really easily :

• go to start.spring.io
• add only spring cloud function
• download and unpack the zip file
• mvn verify will show the problem

To save some time, here is the zip file ready for download : demo.zip

Workaround : exclude android-json from the test dependencies (seems the safer bet, in order to have the same version that the one that will run in prod) :

  <exclusions>
        <exclusion>
            <groupId>com.vaadin.external.google</groupId>
            <artifactId>android-json</artifactId>
        </exclusion>
    </exclusions>

but it's a bit sad/messy to have to to that, and only do it for projects that use spring-cloud-function

[olegz]

We actually do have an exclusion - https://github.com/spring-cloud/spring-cloud-function/blob/main/spring-cloud-function-context/pom.xml#L69. Perhaps it needs to be excluded at the boot level

[snussbaumer]

I can open the issue somewhere else if you think I should do that ... could you point me where ?

[hgarus]

@olegz I don't think you excluding the transitive json-dependency of spring-boot-starter-test in spring-cloud-function-context does a lot to alleviate this issue.

A lot of users of spring-cloud-function will depend on spring-boot-start-test directly. Unless I'm missing something, once I depend on both spring-boot-starter-test and spring-cloud-function-context I will have to conflicting Dependencies on the test class path unless I explicitly exclude one of the transitive json Dependencies. spring-boot-properties-migrator also has a transitive dependency on com.vaadin.external.google:android-json.

Issues like this seems to crop somewhat regularly (https://github.com/spring-projects/spring-boot/issues?q=is%3Aissue+org.json+is%3Aclosed). I'm not sure about the best way to solve this. Looking at https://github.com/spring-cloud/spring-cloud-function/commit/be45a47818861a6eb9bc74a78f9c309b411ae826#diff-1599bf441292c497eccbc0839b2beb4de2dcb70d0cd13d40db7055e620c9fb60 I think there should be a way to solve the underlying bug without adding the new dependency to org.json:json. I'll look into this in the next few days.

[olegz]

Correct, but the dependency in question is transitively brought up by spring-boot-starter-test not s-c-function

[hgarus]

Yes.

My point ist the following: spring-cloud-function-context now brings in org.json:json, many users of spring cloud function will also be using spring-boot and thus probably use spring-boot-starter-test, which has been bringing in com.vaadin.external.google:android-json for quite some time.

Now after updating to the latest spring-boot and spring cloud releases I'm tracking down conflicting transitive dependencies between a spring cloud project and spring-boot-starter-test.

If you say "This is not a problem with spring-cloud-function, please take this up with spring-boot." That is fine by me, although I'd expect them to not be able to solve this super easily, looking at the bugs in their bugtracker from the previous few years mentioning org.json:json.

[hgarus]

@olegz After looking into this some more I am somewhat confused:

Looking at SimpleFunctionRegistryTests.testSCF1094 I'm very unsure why the different Exceptions each represent a positive outcome. Especially the IllegalStateException in the second case seems to be pretty much the error which was raised in #1094 in the first place: SimpleFunctionRegistry.fluxifyInput() hands the protobuf encoded payload to JacksonMapper which then fails trying to parse the message. new JSONArray("\n[aa...aa]"); does not throw, but instead returns a JSONArray containing a single String "aa...aa".

In general I'm not sure if trying to parse the input to check if it is valid JSON is the correct approach: There are several calls to the isJson* methods directly followed by calls to JsonMapper.fromJson() (in which case the value is unnecessarily parsed twice). Furthermore there could be inputs which aren't meant to be interpreted as json, which happen to be fully valid Json (addding quotes to the long protobuf String yields \n["a...a"] which happens to be a valid JSON-Array).

If you want to continue on this route anyway, it would be nice to replace the usage of JSONArray and JSONObject with calls toObjectMapper.readTree() - basically the same thing you are already doing in JsonMapper.isJsonString(). To get rid of the org.json:json dependency. I can prepare a PR if you are interested.

[dsyer]

+1 to fixing this in spring-cloud-function. If Spring Boot wants us to use the Vaadin JSON lilbrary, we should just do it, and avoid making things difficult for users otherwise. Is it possible it could even be an optional dependency of spring-cloud-function?

[hgarus]

I think I can achieve the current behaviour - or presumably intended behaviour, using the already existing Jackson Dependency and remove the JSON-Dependency completely. I'll prepare a PR over the weekend.