getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.32k stars 168 forks source link

Kirby\Http\Remote CURLOPT_POSTFIELDS always turned to string #2913

Closed hdodov closed 2 years ago

hdodov commented 4 years ago

Describe the bug
In the Kirby\Http\Remote class, CURLOPT_POSTFIELDS is always turned to a string. Specifically, Remote.php, line 239.

The postfields() method looks like this:

if (is_object($data) || is_array($data)) {
    return http_build_query($data);
} else {
    return $data;
}

It always turns arrays to strings, even though PHP has special treatment for arrays. From the docs (emphasis mine):

This parameter can either be passed as a urlencoded string like 'para1=val1&para2=val2&...' or as an array with the field name as key and field data as value. If value is an array, the Content-Type header will be set to multipart/form-data. As of PHP 5.2.0, value must be an array if files are passed to this option with the @ prefix.

Yesterday, I had a problem where Remote::request() caused unexpected responses, while a pure PHP cURL had no problems. The only difference was CURLOPT_POSTFIELDS being an array or string.

Expected behavior
It should leave arrays as-is.

Kirby Version
3.4.4

lukasbestle commented 4 years ago

I agree it can be useful to pass an array to cURL to make requests with multipart/form-data instead of application/x-www-form-urlencoded.

The issue is: If we change this now, it would be a breaking-change as users who have relied on the automatic conversion so far would now suddenly get different results. For your use-case it works with multipart/form-data and not for application/x-www-form-urlencoded, but for other users it could be the opposite.

What we could do is to introduce a new option like dataType that could be set to the preferred MIME type. If an array is passed, the Remote class would then decide based on that option which variant to use. If a string is passed however, only application/x-www-form-urlencoded would be supported.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.