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

[9.x] Handle when $haystack is null in Str::contains #41419

Closed
wants to merge 1 commit into from
Closed

[9.x] Handle when $haystack is null in Str::contains #41419

wants to merge 1 commit into from

Conversation

hmazter
Copy link
Contributor

@hmazter hmazter commented Mar 10, 2022

Fixes this deprecation message from str_contains:

str_contains(): Passing null to parameter #1 ($haystack) of type string is deprecated

The deprecation warning appears when running this in a fresh install:

LOG_DEPRECATIONS_CHANNEL=stderr php artisan route:list --name foo

I opted to place it first in the method since there is no need for anything else to run.
This also has the bonus to fix the potential deprecation warning of passing null to mb_strtolower

Fixes this deprecation message from str_contains:
```
str_contains(): Passing null to parameter #1 ($haystack) of type string is deprecated
```

The deprecation warning appears when running this in a fresh install:
```
LOG_DEPRECATIONS_CHANNEL=stderr php artisan route:list --name foo
```
@GrahamCampbell
Copy link
Member

This method does not allow you to pass null here. The code that is calling this with null is what should be fixed, here.

@hmazter
Copy link
Contributor Author

hmazter commented Mar 10, 2022

Okay. The problem, in this case, is these Str::contains checks.

if (($this->option('name') && ! Str::contains($route['name'], $this->option('name'))) ||
($this->option('path') && ! Str::contains($route['uri'], $this->option('path'))) ||
($this->option('method') && ! Str::contains($route['method'], strtoupper($this->option('method')))) ||
($this->option('domain') && ! Str::contains($route['domain'], $this->option('domain'))) ||
($this->option('except-vendor') && $route['vendor'])) {
return;
}

Not really sure what the best approach is to fix that

@GrahamCampbell
Copy link
Member

Cast those to a string, probably, before calling.

@GrahamCampbell GrahamCampbell changed the title Handle when $haystack is null in Str::contains [9.x] Handle when $haystack is null in Str::contains Mar 10, 2022
@GrahamCampbell
Copy link
Member

Likely this bug is also present on 8.x, so needs fixing there too?

@driesvints
Copy link
Member

This method does not allow passing a string. Typecast the value before passing.

@driesvints driesvints closed this Mar 10, 2022
@hmazter
Copy link
Contributor Author

hmazter commented Mar 10, 2022

I have a failing test for it here at least https://github.com/hmazter/framework/commit/efc8bc8ffb5831e4506bb2a0dddd513ee882d94d
Not sure what to do with it though

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