twilio / twilio-java

A Java library for communicating with the Twilio REST API and generating TwiML.
MIT License
482 stars 421 forks source link

org.json:json dependency scope #742

Closed xak2000 closed 1 year ago

xak2000 commented 1 year ago

Issue Summary

Twilio SDK includes org.json:json dependency starting from version 9.0.0.

According to the commit message it looks like the dependency was added for testing purposes: https://github.com/twilio/twilio-java/pull/709/commits/60adbe0a16a3a3c23789dad870bfbe9445291866.

But this dependency doesn't have test scope (intentially or not?), so it has default compile scope and is visible as a transitive dependency at compile time by any project, that uses com.twilio.sdk:twilio library.

Do this dependency really needs to have compile scope? Could it be moved to the test scope?

The problem with this dependency is that some lawyers consider it as inappropriate due to the license. See redis/jedis#3189 and the discussion here stleary/JSON-java#706. Also, it clashes at runtime classpath with other dependencies that implement the same classes. E.g. org.springframework.boot:spring-boot-starter-test depends on org.skyscreamer:jsonassert:1.5.1, that depends on com.vaadin.external.google:android-json:0.0.20131108.vaadin1, that implements the same classes as in org.json:json library, but with different licensing. So, at runtime there is multiple occurrences of org.json.JSONObject on the class path that could lead to unpredictable runtime behavior.

Anyway, if org.json:json dependency is only needed by tests, could its scope be changed to test in pom.xml, please? Even without taking into account all these problems, less transitive dependencies is a good thing.

Steps to Reproduce

  1. Inlcude both org.springframework.boot:spring-boot-starter-test:3.0.6 and com.twilio.sdk:twilio:9.6.0 in the classpath.
  2. Run any spring boot test.
  3. Spring's own DuplicateJsonObjectContextCustomizer will detect multiple occurrences of org.json.JSONObject on the class path.

Exception/Log

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

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

You may wish to exclude one of them to ensure predictable runtime behavior

Technical details:

AsabuHere commented 1 year ago

This dependency is not required anymore and is removed now. The changes will be available from next version