Skip to content

Report unknown size instead of passing false or 0 bytes to the inner validator - #1843

Merged
alganet merged 1 commit into
Respect:mainfrom
ZamboniL:improve-size-error-return
Sep 3, 2026
Merged

Report unknown size instead of passing false or 0 bytes to the inner validator#1843
alganet merged 1 commit into
Respect:mainfrom
ZamboniL:improve-size-error-return

Conversation

@ZamboniL

@ZamboniL ZamboniL commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Size casted the return of filesize() to an integer, and (int) false === 0, so a
filename that does not exist or is not readable validated as 0 bytes. Every upper-bound
check passed on a file that was never there:

v::size('KB', v::lessThan(2))->assert('/path/that/does/not/exist');
// Validation passes successfully

SplFileInfo::getSize() throws when stat fails, so the same missing file escaped as an
uncaught RuntimeException instead of a validation failure; its documented return type is
int|false, so overrides may return false rather than throw (Symfony's Stream
does exactly that). PSR-7 streams and uploaded files may legitimately report null for an
unknown size, and that was reported as TEMPLATE_WRONG_TYPE, which is misleading: the type
is fine, only the size is not knowable.

Check the input type up front, so unsupported types keep reporting TEMPLATE_WRONG_TYPE,
and report a supported input whose size cannot be determined under the new
TEMPLATE_UNKNOWN_SIZE. Guard filesize(), catch the RuntimeException from SplFileInfo
and narrow every getSize() result with is_int(). getSize() was also being called twice
per evaluation; it is now called once.

Inputs that previously validated against a bogus 0 now fail, which is a behaviour change. The existing
template constant and message are untouched.


There is no open issue for this. I found it in a Symfony API validating user-supplied
files, which arrive as HttpFoundation\File\File an SplFileInfo subclass.

@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.14%. Comparing base (4bd2d68) to head (f78e012).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1843      +/-   ##
============================================
- Coverage     97.16%   97.14%   -0.03%     
- Complexity     1087     1096       +9     
============================================
  Files           198      198              
  Lines          2542     2554      +12     
============================================
+ Hits           2470     2481      +11     
- Misses           72       73       +1     

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

@ZamboniL
ZamboniL force-pushed the improve-size-error-return branch from 6966376 to d0c5f5b Compare September 1, 2026 21:37
@alganet

alganet commented Sep 1, 2026

Copy link
Copy Markdown
Member

Looks good to me! Thanks for your contribution 🐼

I'll wait a couple of days to give a chance for @henriquemoody to review it.

I would probably remove the // coments. It's our style to not have them, but currently nothing enforces their absence so I won't require you to remove them (would be nice though; that would also be best as an amend instead of a separate commit).

This will probably be in either 3.2 or 4.0 (the main branch will increase one of those counters soon). Let me know if you need a backport for a 3.1.x or 2.x release, if it's something important I can work it out.

@ZamboniL
ZamboniL force-pushed the improve-size-error-return branch from d0c5f5b to 3d205ba Compare September 2, 2026 12:21
@ZamboniL

ZamboniL commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the quick response, I deleted the comments with a ammend.

I can wait til the next minor/major, no problem!

@henriquemoody henriquemoody left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for those changes, @ZamboniL!

I made a few comments, all nick pick. Not required, but would make the code a bit neater.

@alganet, I'm fine if you want to merge it as is.

Comment thread src/Validators/Size.php Outdated
Comment thread src/Validators/Size.php Outdated
"Size" cast the result of "filesize()" to an integer, so a filename that
does not exist or is not readable became a size of 0 bytes, and every
"lessThan" style validation passed on a file that was never there.
"SplFileInfo::getSize()" throws when "stat" fails, which surfaced as an
uncaught "RuntimeException", and PSR-7 streams and uploaded files may
legitimately report NULL, which was passed along to the inner validator.

Input types are now checked up front, so unsupported types still report
"TEMPLATE_WRONG_TYPE", and inputs of a supported type whose size cannot
be determined report the new "TEMPLATE_UNKNOWN_SIZE".
@ZamboniL
ZamboniL force-pushed the improve-size-error-return branch from 3d205ba to f78e012 Compare September 2, 2026 23:40
@ZamboniL

ZamboniL commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

Nits applied, thanks for the review.

@alganet
alganet merged commit 639f02f into Respect:main Sep 3, 2026
7 checks passed
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.

3 participants