Update thrift to 0.23.0#17945
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #17945 +/- ##
============================================
- Coverage 41.21% 41.21% -0.01%
Complexity 318 318
============================================
Files 5258 5259 +1
Lines 366348 366351 +3
Branches 47389 47390 +1
============================================
- Hits 151004 151002 -2
- Misses 215344 215349 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Removed obsolete repository configuration for apache-iotdb-1191.
HTHou
changed the title
HTHou
changed the title
JackieTien97
left a comment
JackieTien97
left a comment
There was a problem hiding this comment.
Thanks for the Thrift upgrade. I found two release-facing issues that should be fixed before merging: the binary license metadata is missing a newly bundled runtime dependency, and the JDBC Karaf feature descriptor can publish an unresolved Maven property.
| @@ -241,7 +241,7 @@ org.eclipse.jetty.ee10:jetty-ee10-servlet:12.0.36 | |||
| org.eclipse.jetty:jetty-util:12.0.36 | |||
| com.google.code.findbugs:jsr305:3.0.2 | |||
There was a problem hiding this comment.
libthrift:0.23.0 now brings org.apache.commons:commons-lang3:3.18.0 as a runtime transitive dependency (iotdb-server -> libthrift -> commons-lang3), and the server/confignode assemblies include module dependencies under lib. Since the binary LICENSE only updates the libthrift entry and does not list commons-lang3, the released binary would contain an Apache-2.0 dependency that is not disclosed here. Please add org.apache.commons:commons-lang3:3.18.0 to this section, or otherwise exclude it and verify Thrift does not require it at runtime.
There was a problem hiding this comment.
Addressed in 1eba3b7d4e by excluding org.apache.commons:commons-lang3 from the managed libthrift dependency. I also verified that iotdb-core/datanode and iotdb-core/confignode no longer resolve commons-lang3 via mvn dependency:tree -Dincludes=org.apache.commons:commons-lang3, and confirmed IoTDB does not use org.apache.thrift.partial APIs.
| @@ -27,7 +27,7 @@ | |||
| <bundle>mvn:org.apache.iotdb/service-rpc/${project.version}</bundle> | |||
| <bundle>mvn:org.apache.iotdb/iotdb-thrift/${project.version}</bundle> | |||
There was a problem hiding this comment.
This placeholder can be published literally in the JDBC Karaf features artifact. The resources step filters src/main/feature into target/classes/feature, but iotdb-client/jdbc/pom.xml still attaches src/main/feature/feature.xml directly as the features classifier. After this change, the attached *-features.xml would contain mvn:org.apache.thrift/libthrift/${thrift.version}, which Karaf cannot resolve. Please attach the filtered file instead, or keep a concrete 0.23.0 version here.
There was a problem hiding this comment.
Addressed in 1eba3b7d4e by keeping the source feature descriptor publishable directly: the Karaf feature now uses the concrete mvn:org.apache.thrift/libthrift/0.23.0 coordinate. Verified with mvn process-resources -pl iotdb-client/jdbc -DskipTests; both the source and filtered feature descriptors contain 0.23.0 rather than ${thrift.version}.
There was a problem hiding this comment.
Pull request overview
This PR upgrades Apache Thrift across IoTDB to address CVE-2026-43869, refreshes Maven dependency exclusions for the new libthrift dependency graph, and adapts server-side Thrift event handlers to Thrift 0.23’s non-null ServerContext expectation. It also updates client CI workflows to avoid using disallowed GitHub Actions.
Changes:
- Bump
org.apache.thrift:libthriftto0.23.0and align related versioning/feature references (including JDBC Karaf feature and binary license list). - Introduce a shared
NoopServerContextand return it from Thrift server event handlers that don’t maintain per-connection state. - Replace
dorny/paths-filter@v3usage in client workflows with localgit diff-based path detection.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Bumps Thrift/tooling versions and refreshes libthrift dependency exclusions for 0.23.0. |
| LICENSE-binary | Updates the recorded libthrift binary dependency version. |
| iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/service/NoopServerContext.java | Adds a shared no-op ServerContext implementation for Thrift 0.23 compatibility. |
| iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/service/NoopServerContextTest.java | Adds unit tests validating NoopServerContext wrapper behavior. |
| iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/client/mock/MockInternalRPCService.java | Updates test mock event handler to return a non-null ServerContext. |
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/execution/exchange/MPPDataExchangeServiceThriftHandler.java | Returns NoopServerContext instead of null from createContext. |
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/handler/InternalServiceThriftHandler.java | Returns NoopServerContext instead of null from createContext. |
| iotdb-core/datanode/src/main/java/org/apache/iotdb/db/protocol/thrift/handler/BaseServerContextHandler.java | Ensures createContext never returns null and makes disconnect handling safer with instanceof. |
| iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/pipe/service/IoTConsensusV2RPCServiceHandler.java | Returns NoopServerContext instead of null from createContext. |
| iotdb-core/consensus/src/main/java/org/apache/iotdb/consensus/iot/service/IoTConsensusRPCServiceHandler.java | Returns NoopServerContext instead of null from createContext. |
| iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/service/thrift/ConfigNodeRPCServiceHandler.java | Returns NoopServerContext instead of null from createContext. |
| iotdb-client/service-rpc/src/main/java/org/apache/iotdb/rpc/TSocketWrapper.java | Updates documentation comment referencing Thrift constructor exception behavior. |
| iotdb-client/service-rpc/src/main/java/org/apache/iotdb/rpc/TNonblockingTransportWrapper.java | Updates documentation comment referencing Thrift constructor exception behavior. |
| iotdb-client/jdbc/src/main/feature/feature.xml | Aligns Karaf feature bundle version with ${thrift.version}. |
| .github/workflows/multi-language-client.yml | Replaces paths-filter action with custom git diff path detection to decide per-language jobs. |
| .github/workflows/client-cpp-package.yml | Replaces paths-filter action with custom git diff path detection for rc-branch packaging gating. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * In Thrift 0.14.1, TSocket's constructor throws a never-happened exception. So, we screen the | ||
| * exception https://issues.apache.org/jira/browse/THRIFT-5412 | ||
| * TSocket's constructor throws a never-happened exception. So, we screen the exception | ||
| * https://issues.apache.org/jira/browse/THRIFT-5412 | ||
| */ |
There was a problem hiding this comment.
Fixed in 1eba3b7d4e by rewording the Javadoc to explain that the constructor declares TTransportException for compatibility, but this code path is not expected to throw it.
| /** | ||
| * In Thrift 0.14.1, TNonblockingSocket's constructor throws a never-happened exception. So, we | ||
| * screen the exception https://issues.apache.org/jira/browse/THRIFT-5412 | ||
| * TNonblockingSocket's constructor throws a never-happened exception. So, we screen the exception | ||
| * https://issues.apache.org/jira/browse/THRIFT-5412 | ||
| */ |
There was a problem hiding this comment.
Fixed in 1eba3b7d4e by rewording the Javadoc to explain that the constructor declares TTransportException for compatibility, but this code path is not expected to throw it.
|
Summary
org.apache.thrift:libthriftfrom0.14.1to0.23.0to address CVE-2026-43869.org.apache.iotdb.tools:iotdb-tools-thriftto0.23.0.0.libthriftexclusions for the 0.23.0 dependency graph:libthrift:0.23.0(tomcat-embed-core,javax.annotation-api, old HttpClient 4 artifacts);httpclient5,httpcore5,httpcore5-h2);jakarta.servlet-apibecause IoTDB does not use Thrift's servlet transport (TServlet/TExtensibleServlet).jakarta.annotation-apimanaged by IoTDB dependency management; it resolves to3.0.0and is compatible with Thrift 0.23.0.${thrift.version}property and update the binary license dependency list.dorny/paths-filter@v3in client workflows with local Bash path detection so the workflows comply with the repository action allowlist.ServerContextexpectation by returning an explicit shared no-op context from handlers that do not need per-connection state, instead of wrapping all handlers centrally.Validation
mvn spotless:apply -pl iotdb-client/service-rpcmvn dependency:tree -pl iotdb-protocol/thrift-commons -Dincludes=org.apache.httpcomponents.client5,org.apache.httpcomponents.core5 -DskipTestsmvn compile -pl iotdb-protocol/thrift-commons,iotdb-protocol/thrift-datanode,iotdb-client/service-rpc,iotdb-client/jdbc -am -DskipTestsruby -e 'require "yaml"; ARGV.each { |f| YAML.load_file(f); puts "OK #{f}" }' .github/workflows/multi-language-client.yml .github/workflows/client-cpp-package.ymlpom.xml,client-cpp,jdbc, and workflow path changes.org.apache.iotdb.tools:iotdb-tools-thrift:0.23.0.0:mac-aarch64from the staging repository during Thrift code generation.mvn spotless:apply -pl iotdb-core/node-commons,iotdb-core/confignode,iotdb-core/consensus,iotdb-core/datanodemvn test -pl iotdb-core/node-commons -Dtest=NoopServerContextTest,ClientManagerTestmvn dependency:tree -pl iotdb-protocol/thrift-commons -Dincludes=jakarta.annotation:jakarta.annotation-api,jakarta.servlet:jakarta.servlet-api,org.apache.tomcat.embed:tomcat-embed-core,org.apache.httpcomponents.client5,org.apache.httpcomponents.core5 -DskipTestsmvn dependency:tree -pl external-service-impl/rest -Dincludes=jakarta.servlet:jakarta.servlet-api,jakarta.annotation:jakarta.annotation-api -DskipTestsmvn compile -pl iotdb-core/node-commons,iotdb-core/consensus -DskipTestsNotes
A broader local compile including
confignodeanddatanodecurrently hits generated-source cache mismatches unrelated to this change, such as missingTDatabaseInfo#get/setMinSchemaRegionNum,TDatabaseInfo#get/setMinDataRegionNum, and relational grammar generated methods. The focused modules above compile successfully.