-
Notifications
You must be signed in to change notification settings - Fork 6.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgraded third-party dependencies to the latest versions and fixed CVE vulnerabilities #34409
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed a bunch of suspicious issues. However, there are no separate issues for the related topics.
@@ -38,7 +40,7 @@ class EncryptInsertSelectSupportedCheckerTest { | |||
@Test | |||
void assertIsCheck() { | |||
InsertStatementContext sqlStatementContext = mock(InsertStatementContext.class, RETURNS_DEEP_STUBS); | |||
when(sqlStatementContext.getSqlStatement().getInsertSelect().isPresent()).thenReturn(true); | |||
when(sqlStatementContext.getSqlStatement().getInsertSelect()).thenReturn(Optional.of(mock(SubquerySegment.class))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Why does updating some third-party dependencies cause a large number of unit tests to change? Where is the original issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new version of mock will prompt that the thenReturn type is wrong, and Optional.of(mock(SubquerySegment.class)) is required to return
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This involves a problem. The new version of mockito does not support JDK8, so it makes no sense to do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- You can simply rebese the remote branch to resolve the CI issue. Fix agent file error #34412 has been merged.
<exclusion> | ||
<groupId>org.springframework</groupId> | ||
<artifactId>*</artifactId> | ||
</exclusion> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Actually, it might be better to handle it as follows. Because this involves a PR on the unreleased Seata 2.3.0 milestone. See optimize: remove the dependency conflict for spring-webmvc incubator-seata#7046 .
<dependency>
<groupId>org.apache.seata</groupId>
<artifactId>seata-all</artifactId>
<version>${seata.version}</version>
<scope>test</scope>
<exclusions>
<exclusion>
<groupId>org.antlr</groupId>
<artifactId>antlr4-runtime</artifactId>
</exclusion>
<exclusion>
<groupId>commons-lang</groupId>
<artifactId>commons-lang</artifactId>
</exclusion>
<exclusion>
<groupId>org.apache.commons</groupId>
<artifactId>commons-pool2</artifactId>
</exclusion>
<exclusion>
<groupId>org.springframework</groupId>
<artifactId>spring-webmvc</artifactId>
</exclusion>
</exclusions>
</dependency>
<exclusions> | ||
<exclusion> | ||
<groupId>org.apache.commons</groupId> | ||
<artifactId>commons-compress</artifactId> | ||
</exclusion> | ||
</exclusions> | ||
</dependency> | ||
<dependency> | ||
<groupId>org.apache.commons</groupId> | ||
<artifactId>commons-compress</artifactId> | ||
<version>${commons-compress.version}</version> | ||
<scope>test</scope> | ||
<exclusions> | ||
<exclusion> | ||
<groupId>org.apache.commons</groupId> | ||
<artifactId>commons-io</artifactId> | ||
</exclusion> | ||
</exclusions> | ||
</dependency> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Allow me to express my doubts about this change. Versions 1.25.0 and later of
org.apache.commons:commons-compress
actually break the public API of testcontainers-java. See [Bug]: Vulnerable dependency commons-compress 1.24.0 testcontainers/testcontainers-java#8338 . - I'm guessing you actually want to control the dependency tree of jetcd-test?
<dependency>
<groupId>io.etcd</groupId>
<artifactId>jetcd-test</artifactId>
<version>${jetcd.version}</version>
<scope>test</scope>
<exclusions>
<exclusion>
<groupId>org.apache.commons</groupId>
<artifactId>commons-compress</artifactId>
</exclusion>
</exclusions>
</dependency>
</dependency> | ||
<dependency> | ||
<groupId>com.mysql</groupId> | ||
<artifactId>mysql-connector-j</artifactId> | ||
<scope>provided</scope> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- My understanding is that this is not enough. If a dependency's maven scope is not
test
, then the dependency will be affected by https://www.apache.org/legal/resolved.html . This requires implementing something like [FEATURE] Would consider replacing mybatis with hibernate? gravitino#4352 (comment) for<optional>
, i.e.
<dependency>
<groupId>com.mysql</groupId>
<artifactId>mysql-connector-j</artifactId>
<scope>provided</scope>
<optional>true</optional>
</dependency>
- The core reason is that MySQL JDBC Driver is a standard GPL. The FOSS exception clause of MySQL JDBC Driver is actually not paid attention to by ASF.
<zookeeper.version>3.9.2</zookeeper.version> | ||
<audience-annotations.version>0.12.0</audience-annotations.version> | ||
<audience-annotations.version>0.15.0</audience-annotations.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
org.apache.yetus:audience-annotations:0.12.0
is actually just a transitive dependency oforg.apache.zookeeper:zookeeper:3.9.2
.- And since
org.apache.yetus:audience-annotations:0.14.0
, the metadata provided by this dependency is invalid on JDK8, refer to https://issues.apache.org/jira/browse/YETUS-1132 . Why do you need to change the version oforg.apache.yetus:audience-annotations
?
<jetcd.version>0.7.7</jetcd.version> | ||
<vertx.version>4.5.1</vertx.version> | ||
<vertx.version>4.5.11</vertx.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Shardingsphere doesn't actually use vertx.
io.vertx:vertx-grpc:4.5.1
is a transitive dependency ofio.etcd:jetcd-grpc:0.7.7
, why do we need to change the dependency tree of jetcd?
Upgraded third-party dependencies to the latest versions and fixed CVE vulnerabilities
Before committing this PR, I'm sure that I have checked the following options:
./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e
.