Skip to content

Fix byte-length validation for memoryviews - #395

Merged
AndreyVMarkelov merged 2 commits into
dropbox:mainfrom
Shubham-Padkonde:fix/memoryview-byte-length
Oct 7, 2026
Merged

AndreyVMarkelov merged 2 commits into
dropbox:mainfrom
Shubham-Padkonde:fix/memoryview-byte-length

Conversation

@Shubham-Padkonde

Copy link
Copy Markdown
Contributor

The Bytes validator accepts memoryviews but checks len(value), which counts elements (or the first dimension) rather than bytes. An eight-byte view cast to two integers is consequently reported as two bytes: valid minimum lengths fail, and maximum lengths can be exceeded.

Use memoryview.nbytes for memoryviews while retaining len() for bytes. The original value is returned unchanged. Regression tests cover typed and multidimensional views, including the length in the validation error.

Validation: all 191 tests pass on Linux. Both new regressions fail before the fix. Changed-file Flake8 and git diff --check pass. The Windows run passes 189 tests and fails two platform-sensitive tests (temporary-file reopening and whitespace parsing); both pass on Linux.

@CLAassistant

CLAassistant commented Oct 2, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@ineil-dbx

Copy link
Copy Markdown

Hi @Shubham-Padkonde, I’ve raised this with our engineering team for further review. They’ll be able to review the pull request and address it as appropriate.

@AndreyVMarkelov
AndreyVMarkelov self-requested a review October 7, 2026 17:09
@AndreyVMarkelov
AndreyVMarkelov merged commit 8f19a4f into dropbox:main Oct 7, 2026
12 of 13 checks passed
@codecov

codecov Bot commented Oct 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.36%. Comparing base (1ad079b) to head (c4330c6).
⚠️ Report is 38 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #395      +/-   ##
==========================================
+ Coverage   49.98%   52.36%   +2.37%     
==========================================
  Files          37       42       +5     
  Lines        8808    11452    +2644     
  Branches     1892     2393     +501     
==========================================
+ Hits         4403     5997    +1594     
- Misses       4087     4922     +835     
- Partials      318      533     +215     
Flag Coverage Δ
unit 52.36% <100.00%> (+2.37%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

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