Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
luoluoyuyu
left a comment
luoluoyuyu
left a comment
There was a problem hiding this comment.
Review summary
Hardens legacy pipe file transfer: resolveFileInFileDataDir validates file names and resolves paths safely; transportFile uses resolved files for index checks; ClientRPCServiceImpl enforces login and USE_PIPE before handshake, sendPipeData, and sendFile; legacy sink opens a session before transfer.
Recommend merging. Optional IT for malicious file names noted inline.
| + illegalError); | ||
| } | ||
|
|
||
| return PipeReceiverFilePathUtils.resolveFilePath(Paths.get(fileDir), fileName).toFile(); |
There was a problem hiding this comment.
Good fix: PipeReceiverFilePathUtils.resolveFilePath plus illegal-name checks block path traversal in transportFile and handleTsFilePipeData. Consider adding an integration test with a malicious fileName expecting SYNC_FILE_ERROR.
| return getNotLoggedInStatus(); | ||
| } | ||
| return AuthorityChecker.getTSStatus( | ||
| AuthorityChecker.checkSystemPermission(clientSession.getUsername(), PrivilegeType.USE_PIPE), |
There was a problem hiding this comment.
Requiring login and USE_PIPE before legacy pipe RPCs closes unauthenticated file transfer. Document that legacy sinks must call openSession (as done in IoTDBLegacyPipeSink in this PR) when upgrading.
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #17741 +/- ##
============================================
+ Coverage 40.55% 40.70% +0.15%
- Complexity 2574 2610 +36
============================================
Files 5179 5184 +5
Lines 349896 350891 +995
Branches 44727 44893 +166
============================================
+ Hits 141890 142821 +931
- Misses 208006 208070 +64 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 participants
Description
This PR hardens the legacy pipe receiver file-transfer path and aligns its RPC access checks with normal client-
session expectations.
Changes
.., and normalized paths outside the receiverfile-datadirectory.TsFilePipeData.USE_PIPEprivilege for legacyhandshake,sendFile, andsendPipeData.connector behavior.
Tests
mvn '-Ddevelocity.off=true' '-Dscan=false' '-pl' 'iotdb-core/datanode' 'spotless:apply'git diff --checkmvn '-Ddevelocity.off=true' '-Dscan=false' '-pl' 'iotdb-core/datanode' '-Dtest=IoTDBLegacyPipeReceiverAgentTest' 'test'This PR has:
for an unfamiliar reader.
for code coverage.
Key changed/added classes (or packages if there are too many classes) in this PR