Curl request() encodes $data regardless the Content-type header. #1221

Merged
merged 3 commits into from Jun 14, 2012

Projects

None yet

3 participants

@dianaprajescu

To post a file using curl the '@' prefix must be used with the file's full path. As of PHP 5.2.0, the CURLOPT_POSTFIELDS option must be an array if this prefix is used. That means the $data parameter of the request() method must not be encoded if the Content-type header is multipart/form-data.

@piotr-cz

There might be a charset appended to the header, so then full header body would be multipart/form-data; charset=utf-8.

I'd also move it under if (!isset($headers['Content-type'])) block so condition will look like
if (is_scalar($data) || strpos($headers['Content-type'], 'multipart/form-data') === 0)

Yes, the header has a charset appended indeed.

I don't think moving it under if (isset($headers['Content-type'])) block is a good idea. We need to do $options[CURLOPT_POSTFIELDS] = $data if $data is a scalar, no matter if the header is set or not, or if the header is set to multipart/form-data; charset=utf-8 in which case $data will be an array.

Yes, but if the block is moved under $headers['Content-type'] = 'application/x-www-form-urlencoded'; Then we don't have to check if Content-type is set, or I', missing something?

Anyway, in other transport classes fallback to Content-type header is at the bottom of the block so probably it's good idea to keep the order you suggested. Thanks for the commit, every addition to make JHttp package robust is very welcome

Yes, by moving the if (is_scalar($data) || strpos($headers['Content-type'], 'multipart/form-data') === 0) block under $headers['Content-type'] = 'application/x-www-form-urlencoded'; we don't have to check it twice. I misunderstood the first comment.

So should I make this change or leave the current order, changing only the condition to add strpos?

Just change the condition. It will be more robust like this. Sorry for confusion

Done. No problem.

@chdemko chdemko merged commit 5ca2b31 into joomla:staging Jun 14, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment