Framework: [5.4] The intersect method on the $request object doesn't work properly

[vinterskogen]

• Laravel Version: 5.4.30
• PHP Version: 7.0.20
• Database Driver & Version: -

Description:

The intersect method on the $request object doesn't work properly. As documented is works like only method, but doesn't substitute null's to the result array, if the given keys are not presented in request parameters. See the "Retrieving A Portion Of The Input Data" at - https://laravel.com/docs/5.4/requests#retrieving-input

But in fact intersect does not respect boolean (or "boolean-like") values on parameters in request, in only case when they have false (or "false-like") value.

Is other words your app can not correctly retrieving a portion of potentially important data from input parameters, when some of that parameters have "boolean-like" values and presented as false (or "false-like").

This is not proper behavior, in my opinion.

Steps To Reproduce:

Current implementation of intersect fails the following expanded test (see original test here):

public function testIntersectMethod()
{
    $request = Request::create('/', 'GET', [ // or 'POST', or whatever else
        'name' => 'Taylor',
        'age' => null,
        'flagTrue' => true,
        'flagOne' => 1,
        'flagOneStr' => "1",
        'flagFalse' => false,
        'flagZero' => 0,
        'flagZeroStr' => "0",
    ]);

    $actualPotionOfInputData = $request->intersect(
        'name',
        'age',
        'email',
        'flagTrue',
        'flagOne',
        'flagOneStr',
        'flagFalse',
        'flagZero',
        'flagZeroStr'
    );

    $expectedPotionOfInputData = [
        'name' => 'Taylor',
        'flagTrue' => true,
        'flagOne' => 1,
        'flagOneStr' => "1",
        'flagFalse' => false,
        'flagZero' => 0,
        'flagZeroStr' => "0",
    ];

    $this->assertEquals($expectedPotionOfInputData, $actualPotionOfInputData);
}

With faill massage:

vinter@vinter-pc ~/Projects/laravel-framework $ ./vendor/bin/phpunit tests/Http/HttpRequestTest.php --filter testIntersect 
PHPUnit 5.7.21 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.0.20-2~ubuntu14.04.1+deb.sury.org+1 with Xdebug 2.5.4
Configuration: /home/vinter/Projects/laravel-framework/phpunit.xml.dist

F                                                                   1 / 1 (100%)

Time: 102 ms, Memory: 6.00MB

There was 1 failure:

1) Illuminate\Tests\Http\HttpRequestTest::testIntersectMethod
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
     'name' => 'Taylor'
     'flagTrue' => true
     'flagOne' => 1
     'flagOneStr' => '1'
-    'flagFalse' => false
-    'flagZero' => 0
-    'flagZeroStr' => '0'
 )

Hope this will be fixed. Thanks.

[ghost]

You can make new pull request if you are sure from this solution

[vinterskogen]

Right! The reason I'd created this issue now is that my PR, fixing the problem described above, was rejected by Taylor a few days ago, because of if "sounds like a breaking change" - https://github.com/laravel/framework/pull/20287. But, hmm... personally I do not think it breaks anything. So this issue was created in order to hear more opinions on the problem.

[laurencei]

@vinterskogen - it is breaking because it will change the behavior. If people are relying on that function in their current 5.4 apps, it will change.

Your best bet is to resubmit the PR - but send it to the master branch (rather than 5.4 that you sent it to).

That way Taylor can re-consider the PR in a different context (i.e. it might be acceptable to have a breaking change to fix a bug on the master branch).

[vinterskogen]

Got that. @laurencei, thanks for an usefull advice!

The misunderstanding was causes the fact, that I thought sending a bug fix PR for existing feature (intersect is such) directly to master branch violates the contribution process, as it described in Contribution Guide (which I tried to follow):

All bug fixes should be sent to the latest stable branch or to the current LTS branch (5.1). Bug fixes should never be sent to the master branch unless they fix features that exist only in the upcoming release.

I'll try sending that bug fix PR to master.

[vinterskogen]

No, I will not. :)
They fixed it master in totally another way - https://github.com/laravel/framework/pull/18695

[vinterskogen]

It is still related to Laravel 5.4, so reopening it.

[themsaid]

@vinterskogen can't make any changes to how it works now since it'll start breaking people apps, just deal with it the way it is, we've fixed the whole thing in 5.5 :)

[vinterskogen]

As a temporary solution for an app based on 5.4 we implemented a simple macro to provide a desired correct logic. May be some one will find this helpful.

Request::macro('intersectOnly', function ($keys) {
    return array_filter(
        $this->only(is_array($keys) ? $keys : func_get_args()),
         function ($value) {
            return ! is_null($value);
        }
    );
});

@taylorotwell, what do you think, should Laravel 5.4 pull in an additional method above to Illuminate\Http\Request class?

[justinwilson-apollidon]

To me, this is a bug in Laravel 5.4.

Docs on https://laravel.com/docs/5.4/requests#retrieving-input say:

The only method returns all of the key / value pairs that you request, even if the key is not present on the incoming request. When the key is not present on the request, the value will be null. If you would like to retrieve a portion of input data that is actually present on the request, you may use the intersect method

This is not default behavior. If you pass ['test' => 0] in the request, intersect will not include it even though it is indeed present and therefore making either the docs incorrect or a bug in the intersect method.