Skip to content

Improve encryption metadata validation - #925

Merged
HTHou merged 3 commits into
developfrom
codex/fix-encryption-metadata-validation
Aug 28, 2026
Merged

Improve encryption metadata validation#925
HTHou merged 3 commits into
developfrom
codex/fix-encryption-metadata-validation

Conversation

@HTHou

@HTHou HTHou commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Restrict encryption class resolution to the TsFile encryption namespace.
  • Validate encryption implementations before creating instances.
  • Bound encoded key metadata parsing and add localized messages.
  • Add regression coverage for class resolution and key length handling.

Tests

  • ./mvnw test -P with-java -pl java/tsfile -am -Dtest=EncryptTest,TsFileMetadataTest -Dsurefire.failIfNoSpecifiedTests=false

@HTHou
HTHou requested a balanced review from Copilot August 28, 2026 07:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improves Java encryption metadata safety and class validation.

Changes:

  • Restricts and validates encryption class loading.
  • Bounds encryption-key metadata parsing with localized errors.
  • Adds regression tests for class and key-length validation.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
EncryptTest.java Adds validation regression tests.
TsFileMetadata.java Validates key size before allocation.
IEncryptor.java Uses validated encryption classes.
IDecryptor.java Uses validated encryption classes.
EncryptUtils.java Restricts class loading and bounds keys.
messages.properties Adds English validation messages.
messages_zh.properties Adds Chinese validation messages.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread java/tsfile/src/main/java/org/apache/tsfile/encrypt/EncryptUtils.java Outdated
@HTHou
HTHou requested a balanced review from Copilot August 28, 2026 07:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

@codecov-commenter

codecov-commenter commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.32%. Comparing base (b4b40c0) to head (d07df13).
⚠️ Report is 3 commits behind head on develop.

Files with missing lines Patch % Lines
...n/java/org/apache/tsfile/encrypt/EncryptUtils.java 84.37% 5 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #925      +/-   ##
===========================================
+ Coverage    61.26%   61.32%   +0.06%     
===========================================
  Files          746      746              
  Lines        51560    51583      +23     
  Branches      8353     8357       +4     
===========================================
+ Hits         31586    31635      +49     
+ Misses       18359    18324      -35     
- Partials      1615     1624       +9     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ColinLeeo

Copy link
Copy Markdown
Contributor

LGTM

@HTHou
HTHou merged commit 52eb36f into develop Aug 28, 2026
18 checks passed
@HTHou
HTHou deleted the codex/fix-encryption-metadata-validation branch August 28, 2026 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants