Skip to content

FileUpload: fix getters calls on empty FileUpload #195

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 11 commits into from

Conversation

EdaCZ
Copy link

@EdaCZ EdaCZ commented Mar 26, 2021

  • bug fix: yes
  • BC break: no

This happened to me recently: I forgot to mark form item with file upload as mandatory, so users were able to send form without any file. But on backend I have complex system of validators, which want to validate name, size etc. Validators are build using composition and each of them do just one thing (for example one of them just calls getName (newly getSanitizedName / getUntrustedName) on received FileUpload and checks its name...). It's not really handy to repeat in each of them condition with $file->isOk()... But if I didn't have it there, it would crash on the error: Return value of Nette\Http\FileUpload::getName() must be of the type string, null returned (in PHP 7.3), because properties don't have default values and null is not compatible with typehint string...

I believe that getters should be callable without an error even on empty file upload, to make behavior of our FileUpload more predictable and consistent.

I tried to add failing test and fix it.

I hope it's ok for you, just let me know if some changes needed.

Thanks.

@dg dg force-pushed the master branch 4 times, most recently from 6ee31b8 to 4f4a403 Compare April 27, 2021 21:33
@dg dg force-pushed the master branch 4 times, most recently from a8895b6 to 9409a5f Compare August 25, 2021 15:26
@dg dg added this to the v4.0 milestone Aug 25, 2021
@dg dg force-pushed the master branch 8 times, most recently from a85dc37 to 12e2e54 Compare September 3, 2021 22:27
@dg dg force-pushed the master branch 4 times, most recently from 2a0c595 to 4ca7b03 Compare January 26, 2023 22:47
@dg dg force-pushed the master branch 6 times, most recently from da24b94 to 540335c Compare March 20, 2023 13:43
@dg dg force-pushed the master branch 2 times, most recently from e7c7e2d to bf945f3 Compare August 5, 2023 19:08
@dg dg force-pushed the master branch 3 times, most recently from 9a14e6e to a20fb8f Compare November 14, 2023 18:31
@dg dg force-pushed the master branch 5 times, most recently from 55488bd to 2042d2e Compare December 11, 2023 13:01
@dg dg force-pushed the master branch 2 times, most recently from 4960652 to 5e67add Compare May 2, 2024 10:56
dg pushed a commit that referenced this pull request Nov 4, 2024
@dg
Copy link
Member

dg commented Nov 4, 2024

fixed

@dg dg closed this Nov 4, 2024
dg pushed a commit that referenced this pull request Nov 4, 2024
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