Incorrect Query Params by Arr::query()

[comes]

• Laravel Version: 8.83.16 & 9.17.0
• PHP Version: 8.0.19 & 8.1.6
• Database Driver & Version: unneeded

Description:

Query params built by Arr::query() didn't match the expected result.

Steps To Reproduce:

$query = ['foo' => 1, 'bar' => 'hello world', 'nullable' => null, 'empty' => ''];

dump(http_build_query($query, '', '&', PHP_QUERY_RFC3986)); // foo=1&bar=hello%20world&empty=

dump(\Arr::query($query)); // foo=1&bar=hello%20world&empty=

the expected output should be foo=1&bar=hello%20world&empty=&nullable=

Arr::query() use an internal php function which did not produce the expected results.

Route::any('test', function (Request $request) {
    $laravelQuery = $request->all();
    $vanillaQuery = $_GET;

    return print_r([$vanillaQuery,
        $laravelQuery,
        $vanillaQuery === $laravelQuery,
        \Arr::query($vanillaQuery),
        \Arr::query($laravelQuery)
    ]);
});

output:

Array
(
    [0] => Array
        (
            [foo] => bar
            [empty] => 
        )

    [1] => Array
        (
            [foo] => bar
            [empty] => 
        )

    [2] => 
    [3] => foo=bar&empty=
    [4] => foo=bar
)

The reason for the null-value is the default middleware \Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull::class, in https://github.com/laravel/laravel/blob/1f9eee06ddb38abf86e2f7df37a20281cd774e3d/app/Http/Kernel.php#L23

but the reason for the wrong query parameters is in http_build_query see https://www.php.net/manual/en/function.http-build-query.php#60523

The follow up error here is, if you create a pagination withQueryParams(), the resulting query parameter is something different, than the original one.

[rodrigopedra]

Sent PR #42795

[rodrigopedra]

Hey @comes , I tried PR #42795 targeting 8.x, as it seemed a bug fix, and PR #42806 targeting master as the first PR was rejected due to being a breaking change on expected behavior.

But this last PR was also rejected as maintainers are not willing to change this behavior at all. And I kind agree that, besides one might expect the use case you describe to work like this issue report, Arr::query(), with its current behavior, is around for so long, that many other apps might be expecting the current behavior as the default.

My suggestion is that you add a macro to the Illuminate\Support\Arr class, adding a custom method that handle null values as you'd expect.

Inside a Service Provider boot method, for example in your app's AppServiceProvider@boot add this:

use Illuminate\Support\Arr;

// ...

public function boot()
{
    Arr::macro('queryWithNulls', function (array $array) {
        array_walk_recursive($array, function (&$value) {
            if ($value === null) {
                $value = '';
            }
        });

        return http_build_query($array, '', '&', PHP_QUERY_RFC3986);
    });
}

This will allow you to call Arr::queryWithNulls() throughout your project's codebase..

[comes]

Hey @rodrigopedra, thanks for your work and I highly appricate it. And yes, you're right. I can implement this by my own. So you are complete right, thanks for that!

But let me give you a simple example and I do not have any idea how to solve this (the fact beside, that I just need to implement my own pagination object)

Imagine we do have a basic Laravel setup, fresh out of the box. We do have a freshly seeded database with 20 Users and a tiny little modification on the users table with an extra field called active to indicate if a user is active or not. Whatever - just an example, right.

\App\Models\User::factory(20)->create();

Now do have a super basic route in routes/web.php:

Route::get('/sample', function (Request $request) {
    return \App\Models\User::where('active', $request->has('active'))
        ->paginate(5)
        ->withQueryString();
});

Now we send a basic Request to this endpoint:

▶ curl http://sample.test/sample\?active | json_pp
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  1760    0  1760    0     0  52509      0 --:--:-- --:--:-- --:--:-- 73333
{
   "current_page" : 1,
   "data" : [
      {
         "created_at" : "2022-06-15T12:58:28.000000Z",
         "email" : "hill.kane@example.net",
         "email_verified_at" : "2022-06-15T12:58:28.000000Z",
         "id" : 1,
         "name" : "Lily Funk",
         "updated_at" : "2022-06-15T12:58:28.000000Z"
      },
      {
         "created_at" : "2022-06-15T12:58:28.000000Z",
         "email" : "kihn.zachary@example.net",
         "email_verified_at" : "2022-06-15T12:58:28.000000Z",
         "id" : 2,
         "name" : "Dominique Wehner V",
         "updated_at" : "2022-06-15T12:58:28.000000Z"
      },
      {
         "created_at" : "2022-06-15T12:58:28.000000Z",
         "email" : "jarvis50@example.com",
         "email_verified_at" : "2022-06-15T12:58:28.000000Z",
         "id" : 3,
         "name" : "Prof. Nicholas Kiehn V",
         "updated_at" : "2022-06-15T12:58:28.000000Z"
      },
      {
         "created_at" : "2022-06-15T12:58:28.000000Z",
         "email" : "bmarvin@example.net",
         "email_verified_at" : "2022-06-15T12:58:28.000000Z",
         "id" : 4,
         "name" : "Prof. Natalia Keebler",
         "updated_at" : "2022-06-15T12:58:28.000000Z"
      },
      {
         "created_at" : "2022-06-15T12:58:28.000000Z",
         "email" : "russ.ratke@example.org",
         "email_verified_at" : "2022-06-15T12:58:28.000000Z",
         "id" : 5,
         "name" : "Nia Ziemann",
         "updated_at" : "2022-06-15T12:58:28.000000Z"
      }
   ],
   "first_page_url" : "http://sample.test/sample?page=1",
   "from" : 1,
   "last_page" : 4,
   "last_page_url" : "http://sample.test/sample?page=4",
   "links" : [
      {
         "active" : false,
         "label" : "« Previous",
         "url" : null
      },
      {
         "active" : true,
         "label" : "1",
         "url" : "http://sample.test/sample?page=1"
      },
      {
         "active" : false,
         "label" : "2",
         "url" : "http://sample.test/sample?page=2"
      },
      {
         "active" : false,
         "label" : "3",
         "url" : "http://sample.test/sample?page=3"
      },
      {
         "active" : false,
         "label" : "4",
         "url" : "http://sample.test/sample?page=4"
      },
      {
         "active" : false,
         "label" : "Next »",
         "url" : "http://sample.test/sample?page=2"
      }
   ],
   "next_page_url" : "http://sample.test/sample?page=2",
   "path" : "http://sample.test/sample",
   "per_page" : 5,
   "prev_page_url" : null,
   "to" : 5,
   "total" : 20
}

Have a look at the next_page_url. Where is our active query Parameter which impacts the Eloquent Query? Without this query parameter there is maybe no page two or maybe more than just some more pages.

PHP is able to handle this, if we inspect the $_GET or $_REQUEST variable. The RFC3986 which is the definiton of how a URI is structured and what is the defintion of query parameters didn't say anything about, that a query param always should have a value.

Don't judge me wrong, but in my opinion, laravel is doing something wrong here.

all the best, comes

[rodrigopedra]

@comes thanks for the example. I understand, and agree, that the behavior should be changed.

But although I try contribute to Laravel whenever I can, in a way to give back how much I earned from using it, I am not a core team member.

You can leave this issue open, so maybe one of the maintainers sees this enriched example and come up with a improved solution.

As a workaround, I think you might know it already, you can try this:

curl http://sample.test/sample\?active=1

By the way, I wasn't aware of the json_pp utility, thanks for that. Have a nice day =)

[comes]

Highly appreciate this, @rodrigopedra. if you like json_pp, see what's happened if you pipe it to bat. json_pp | bat -l json

Edit: Oh and a little side note here. Yes, in that case, active=1 may help, but there is a difference between having an empty string and a null value. I do not what to mess with anybody, but with the little example above, there should be a solution or a note in the docs. something like: "Laravel did not support empty query parameters anywhere, you should define default values, whenever you can"

[rodrigopedra]

The json_pp | bat -l json is a very nice tip ! Thanks a lot!

[driesvints]

Heya. We don't plan to change this behavior, sorry. We'd appreciate a PR to the docs if anything can be clarified. Thanks