[7.x] Support string type as query in HTTP Client?

[irazasyed]

• Laravel Version: 7.1.2
• PHP Version: 7.4.3

Description:

So I just started using the new HTTP client and noticed there's an issue with how the get method is handling the query param.

Guzzle supports 2 types of query requests, array and string but the HTTP client that ships with Laravel is forcing and turning a query string to an array which is causing a weird behavior when a request is processed as the requests for some sites that use query strings is supposed to go without applying URL encoding which is the case with array based query type (Guzzle encodes if its an array, ref this).

I'm working with a site which has a URL like this for example:

https://example.com/list?7;100;90;p=2

Now if this request goes through as is, then the response from the site is just fine and those weird looking numbers are basically multiple filters 7;100;90; and if I make a call directly using Guzzle client, it's fine.

BUT because Laravel is sending the query as an array instead supporting string type, Guzzle encodes that array making a request to the final URL which looks like this:

https://example.com/list?7%3B100%3B90%3Bp=2

Now because of this encoding, the site is unable to properly understand my request and thus responds without applying any filters.

Yes, I know it's a weird way to have filters set up but unfortunately the site I'm making a request to isn't under my control and that's how it has been set up for long and gotta work with the way it works. However, I wouldn't have reported this issue if Guzzle itself wasn't supporting string type as the query as well.

So it would be great if the get method supports array and string both. Guzzle simply will not encode if it finds the query to be a string type.

Steps To Reproduce:

Here are some cases.

Case 1

Http::get('https://example.com/list?7;100;90;p=2');

Expected:

Should've worked fine. And the expected final URL was supposed to be as is with no encoding whatsoever. Ex: https://example.com/list?7;100;90;p=2

Problem:

Laravel parses the URL, gets the query and sends it as an array to Guzzle which later causes it to get encoded during the flow. Because of which, the URL ends up like so: https://example.com/list?7%3B100%3B90%3Bp=2

Case 2

$query = '7;100;90;p=2';

Http::get('https://example.com/list', $query);

Expected:

This was supposed to work fine as well but due to the type hinting, it fails obviously. For the sake of this report, here's the error.

Error:

TypeError
Argument 2 passed to Illuminate\Http\Client\PendingRequest::get() must be of the type array, string given

Problem:

The get method is type hinted with second param as an array and basically forcing us to pass an array only.

I modified and dropped the type hint in my copy just to test but there's another issue in the code which is the parseQueryParams() in the send() method turning my query string into an array.

After dropping the typehint, here's the query that goes through to Guzzle because of the parseQueryParams() method:

"query" => array:1 [▼
  0 => "7;100;90;p=2"
]

The resulted URL looks like this:

https://example.com/list?0=7%3B100%3B90%3Bp%3D2

I commented out that line that merges the query with parseQueryParams() and it passes my query as is which is what was expected from it and Guzzle knows how to handle such kinda request with such query type.

Case 3

Http::get('https://example.com/list?7;100;90;p=2');

Expected:

Should've worked fine if the parser was disabled.

Problem:

If I disable the parser while still letting the get method have the optional query param as an array and I don't pass anything. It causes another issue because Laravel is passing a default array that's empty. The end result is, it drops my query from the URL completely which makes the URL look like this: https://example.com/list discarding the query from the URL. But if the get method's optional query param is set to null instead of [], then the URL is fine and it won't discard it.

Request / Solution

Now this might seem like a rare case but it does have an impact on how the request is processed and it took me a few hours to figure out the flow and identity the issues. I was honestly not at all knowing there was any issue since the request was still being processed and was successfully getting a response from the site I was calling (with only difference being, the filters weren't applying due to the encoding issue). I only noticed a difference in response after I opened my old copy that was fortunately still live online using cURL vs the latest Laravel 7 with HTTP Client version and both had different results with the latter not actually having the expected result.

So my request is to either support string and array both types or just simply drop the array typehint for the query param and remove the parser which isn't really needed and I have no idea what it really does since Guzzle is already smart enough to take care of both kinda requests and there is no need of parsing and processing the way it does right now.

[taylorotwell]

If you feel like there is a smarter approach to this please feel free to PR it.

[driesvints]

@irazasyed moving this to the ideas repo.

[irazasyed]

@taylorotwell Sure, I'll play around with this and see if I can come up with something and create a PR accordingly.

@driesvints Cool.

Thanks for looking into this. Appreciate it.

[irazasyed]

Merged laravel/framework#31996

Thanks!