Skip to content
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

fix: add parameter checking for max_size #6261

Merged
merged 1 commit into from
Jul 14, 2022

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Jul 13, 2022

Description
Fixes #6255

  • add parameter checking

Before:

ErrorException: array_key_exists(): Using array_key_exists() on objects is deprecated. Use isset() or property_exists() instead from SYSTEMPATH\HTTP\Files\FileCollection.php

After:

InvalidArgumentException: Invalid max_size parameter: "userfile.100"

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

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

I'm okay with this but I also don't expect our validation class to verify all rule formats. If the docs describe the parameter types clearly then it is on the developer to follow them.

@MGatner
Copy link
Member

MGatner commented Jul 13, 2022

Fails if the uploaded file named in the
parameter is larger than the second
parameter in kilobytes (kb).

@kenjis
Copy link
Member Author

kenjis commented Jul 13, 2022

I also don't think we should validate the parameters perfectly.
But this error is hard to understand and , and . are easy to mistake.
So I think this checking is good addition.

@MGatner
Copy link
Member

MGatner commented Jul 14, 2022

Sounds good!

@samsonasik samsonasik merged commit d63e8b6 into codeigniter4:develop Jul 14, 2022
@samsonasik
Copy link
Member

Thank you @kenjis

@kenjis kenjis deleted the fix-Validation-max_size branch July 14, 2022 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
3 participants