Skip to content

Align RVV Base64 input validation with the scalar decoder - #3662

Open
wasphin wants to merge 1 commit into
apache:masterfrom
wasphin:fix-rvv-base64-input-validation
Open

wasphin wants to merge 1 commit into
apache:masterfrom
wasphin:fix-rvv-base64-input-validation

Conversation

@wasphin

@wasphin wasphin commented Oct 25, 2026

Copy link
Copy Markdown
Member

What problem does this PR solve?

Problem Summary:

The RVV Base64 decoder accepts incomplete groups and strips more padding than the scalar decoder.

What is changed and the side effects?

Changed:

  • Require nonempty padded inputs to have a length divisible by four.
  • Remove at most two trailing padding characters so extra padding is rejected.
  • Add common malformed-length and padding cases.

Side effects:

  • Performance effects: Constant-time validation before vector decoding.
  • Breaking backward compatibility: Malformed inputs now follow the scalar decoder behavior.

Check List:

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.

🟢 Approval recommended

The narrowly scoped validation change has focused regression coverage and no identified blocking issues; tests were not executed during this review.

No open issues 🎉

What changed in this PR

This PR aligns the RVV Base64 decoder’s input validation with the scalar decoder, rejecting malformed lengths and excess padding while preserving empty-input handling.

Changes:

  • Require nonempty inputs to have a length divisible by four and strip at most two trailing padding characters.
  • Add regression cases for malformed inputs and valid boundary cases.
File Description
test/​base64_unittest.cc Adds malformed-length, padding, and valid-input regression cases.
src/​butil/​third_party/​modp_b64/​modp_b64_rvv.cc Enforces scalar-compatible length and padding validation.

Review effort: Balanced

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.

2 participants