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

Illuminate\Support\Str::contains crashes on php 8.0.1 #35891

Closed
jesseheidstra opened this issue Jan 14, 2021 · 11 comments
Closed

Illuminate\Support\Str::contains crashes on php 8.0.1 #35891

jesseheidstra opened this issue Jan 14, 2021 · 11 comments

Comments

@jesseheidstra
Copy link

  • Laravel Version: 8.22.1
  • PHP Version: 8.0.1
  • Database Driver & Version: mysql 15.1/mariaDB 10.2.36

Description:

The string support method contains throws an exception when $haystack is not a string.

Example:
Running any request that interacts with the InteractsWithContentTypes trait and uses the isJson method crashes. Make sure to send the request without a content-type header, this causes the $haystack variable to be null and throws an exception.

Php 8.0.0 and older always returned false
Php 8.0.1 throws an exception

Steps To Reproduce:

Install the above php version and laravel version. Request an API without a content-type header. Response should be a 500 server error.

The following code is what crashes.

public static function contains($haystack, $needles)
    {
        foreach ((array) $needles as $needle) {
            if ($needle !== '' && mb_strpos($haystack, $needle) !== false) {
                return true;
            }
        }

        return false;
    }

Exception: Uncaught TypeError: mb_strpos(): Argument #1 ($haystack) must be of type string, null given

The contains method should either type-hint a string or internally check if its a string.

@taylorotwell
Copy link
Member

Can fix.

@taylorotwell
Copy link
Member

If you want to submit a PR feel free but the doc block already says you should only pass a string. I'm not sure there is anything broken here.

@jesseheidstra
Copy link
Author

The Request checks in the InteractsWithContentTypes trait if the request is a json or not. If i send a request without the content-type header then it will be null and crashes. $this->header('CONTENT_TYPE') will be null in the example below.

    /**
     * Determine if the request is sending JSON.
     *
     * @return bool
     */
    public function isJson()
    {
        return Str::contains($this->header('CONTENT_TYPE'), ['/json', '+json']);
    }

So i am forced to send a content-type header with every request?

@GrahamCampbell
Copy link
Member

No. The isJson method needs updating to check if the header is actually set. Please send a PR to fix that.

@rodrigopedra
Copy link
Contributor

On a clean Laravel installation with just this route on its ./routes/api.php file:

<?php

use Illuminate\Http\Request;
use Illuminate\Support\Facades\Route;


Route::get('/', function (Request $request) {
    return ($request->isJson() ? 'yes' : 'no') . ' / ' . PHP_VERSION;
});

If I run php artisan serve and then:

$ curl http://127.0.0.1:8000/api
no / 8.0.1

No errors are logged.

Your issue must be happening somewhere else.

@AEK-BKF
Copy link

AEK-BKF commented Jan 19, 2021

Same error here !
I've just upgrade php to 8.0.1 on server Ubuntu 18,
Laravel 8.22.1

PHP Fatal error:  Uncaught TypeError: mb_strpos(): Argument #1 ($haystack) must be of type string, null given, called in /home/akaderb/Projects/Menu/server/vendor/laravel/framework/src/Illuminate/Support/Str.php on line 183 and defined in /home/akaderb/Projects/Menu/server/vendor/symfony/polyfill-mbstring/bootstrap80.php:60
Stack trace:
#0 /home/akaderb/Projects/Menu/server/vendor/laravel/framework/src/Illuminate/Support/Str.php(183): mb_strpos()
#1 /home/akaderb/Projects/Menu/server/vendor/laravel/framework/src/Illuminate/Http/Concerns/InteractsWithContentTypes.php(16): Illuminate\Support\Str::contains()
#2 /home/akaderb/Projects/Menu/server/vendor/laravel/framework/src/Illuminate/Http/Request.php(371): Illuminate\Http\Request->isJson()
#3 /home/akaderb/Projects/Menu/server/vendor/laravel/framework/src/Illuminate/Http/Request.php(435): Illuminate\Http\Request->getInputSource()
#4 /home/akaderb/Projects/Menu/server/vendor/laravel/framework/src/Illuminate/Http/Request.php(64): Illuminate\Http\Request::createFromBase()
#5 /home/akaderb/Projects/Menu/server/public/index.php(52): Illuminate\Http\Request::capture()
#6 /home/akaderb/Projects/Menu/server/server.php(21): require_once('...')
#7 {main}
  thrown in /home/akaderb/Projects/Menu/server/vendor/symfony/polyfill-mbstring/bootstrap80.php on line 60
[Tue Jan 19 22:31:09 2021] 127.0.0.1:53964 Closing

@rodrigopedra
Copy link
Contributor

@AEK-BKF are you setting error_reporting() to something different than Laravel's default?

@AEK-BKF
Copy link

AEK-BKF commented Jan 19, 2021

@rodrigopedra
I've already fixed it ;) Thanks.
The issue was php8.0-mbstring & php8.0-xml, just install them :)

@GrahamCampbell
Copy link
Member

Oh, so this is a bug in the symfony polyfill?

@GrahamCampbell
Copy link
Member

symfony/polyfill#329

@GrahamCampbell
Copy link
Member

GrahamCampbell commented Jan 20, 2021

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

No branches or pull requests

5 participants