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

Added null check to avoid errors with passing null to a count parameter #1

Merged
merged 2 commits into from
Feb 15, 2019

Conversation

Robbt
Copy link

@Robbt Robbt commented Feb 13, 2019

We have ran into this when attempting to validate a form where the file is empty with LibreTime (and is allowed to be) see libretime/libretime#707
This just checks to make sure the file variable is not null before passing it to count which as of 7.2 has tighter type checking.

@falkenhawk
Copy link
Member

@Robbt shouldn't $this->_count be set to 0 in case when $this->_files is null? Otherwise the self::TOO_FEW error won't be thrown, if $this->_min is set. Or am I wrong?

@Robbt
Copy link
Author

Robbt commented Feb 14, 2019

Ok, I think that makes sense - let me rewrite this to try to do it that way.

@Robbt
Copy link
Author

Robbt commented Feb 15, 2019

@unhawkable So I just did a slight cleanup of the commit but I tested it and it is working to resolve the issue and should also handle the case where a minimum is required. Any other obstacles to getting this merged ?


$this->_count = count($this->_files);
if ($this->_files === null) {
$this->_count = 0;}
Copy link
Member

Choose a reason for hiding this comment

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

$this->_count = $this->_files !== null ? count($this->_files) : 0;

if (($this->_max !== null) && ($this->_count > $this->_max)) {
return $this->_throw($file, self::TOO_MANY);
}

Copy link
Member

Choose a reason for hiding this comment

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

removing empty lines not necessary

@falkenhawk
Copy link
Member

@Robbt sorry I forgot to click on 'finish review' button

if (($this->_min !== null) && ($this->_count < $this->_min)) {
return $this->_throw($file, self::TOO_FEW);
}

Copy link
Member

Choose a reason for hiding this comment

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

removing empty lines not necessary

@@ -253,16 +253,17 @@ public function isValid($value, $file = null)
if (($file === null) || !empty($file['tmp_name'])) {
$this->addFile($value);
}

$this->_count = count($this->_files);
if ($this->_files === null) {
Copy link
Member

Choose a reason for hiding this comment

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

use spaces instead of tabs for indentation (for consistent style)

@falkenhawk
Copy link
Member

@Robbt before I merge it, may I ask you to apply the style changes I requested, please?

@Robbt
Copy link
Author

Robbt commented Feb 15, 2019

@unhawkable so I refactored it to match your style suggestion and I think it is ready for merging. Let me know if there is anything else. Thanks.

@falkenhawk falkenhawk merged commit 424607a into zf1s:master Feb 15, 2019
@falkenhawk
Copy link
Member

Thank you for your effort.

falkenhawk added a commit that referenced this pull request Dec 12, 2022
- substr(): Passing null to parameter #1 ($string) of type string is deprecated
- strtolower(): Passing null to parameter #1 ($string) of type string is deprecated
- strlen(): Passing null to parameter #1 ($string) of type string is deprecated
- trim(): Passing null to parameter #1 ($string) of type string is deprecated
- str_replace(): Passing null to parameter #2 ($replace) of type array|string is deprecated
- strstr(): Passing null to parameter #1 ($haystack) of type string is deprecated
- http_build_query(): Passing null to parameter #2 ($numeric_prefix) of type string is deprecated
- current(): Calling current() on an object is deprecated
- preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated
- preg_match(): Passing null to parameter #4 ($flags) of type int is deprecated
- always construct Zend_Exception with string message, avoid null
- Implicit conversion from float-string "x.xyz" to int loses precision
- PDOStatement::fetch(): Passing null to parameter #2 ($cursorOrientation) of type int is deprecated
- ctype_space(): Argument of type null will be interpreted as string in the future
- file_get_contents(): Passing null to parameter #2 ($use_include_path) of type bool is deprecated
- imagefilledpolygon(): Using the $num_points parameter is deprecated
- Implicit conversion from float to int loses precision
- PDOStatement::bindParam(): Passing null to parameter #4 ($maxLength) of type int is deprecated
- preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated
- ctype_print(): Argument of type int will be interpreted as string in the future
- trim(): Passing null to parameter #1 ($string) of type string is deprecated
etc.
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