Skip to content

Commit

Permalink
don't create http_build_query from POSTFIELDS array
Browse files Browse the repository at this point in the history
  • Loading branch information
@kaluax
kaluax committed May 17, 2016
1 parent 712f3f7 commit 781195b
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions src/Ixudra/Curl/Builder.php
View file
Expand Up @@ -299,11 +299,11 @@ protected function forgeOptions()
foreach( $this->curlOptions as $key => $value ) {
$array_key = constant( 'CURLOPT_' . $key );

if( $key == 'POSTFIELDS' && is_array( $value ) ) {
$results[ $array_key ] = http_build_query( $value, null, '&' );
} else {
// if( $key == 'POSTFIELDS' && is_array( $value ) ) {
// $results[ $array_key ] = http_build_query( $value, null, '&' );
// } else {
$results[ $array_key ] = $value;
}
// }
}

return $results;
Expand Down

7 comments on commit 781195b

@elimentz
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you elaborate what exactly it is you are doing here? Is there an error in the package that is fixed by this? Can you show me a use case?

@kaluax
Copy link
Owner Author

@kaluax kaluax commented on 781195b May 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all, thanks for your great package!

My use case: I had to send a file with the curl request (using CURLFile) and found out curl doesn't set the correct header when http_build_query is used instead of an array (like in your forgeOptions function).

According to PHP curl docs on CURLOPT_POSTFIELDS (http://php.net/manual/en/function.curl-setopt.php):
"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. As of PHP 5.5.0, the @ prefix is deprecated and files can be sent using CURLFile."

I was a bit in a hurry so just quick-fixed it for my use case for now... not sure if it can cause problems for other use cases?

@elimentz
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to me this would be a good use case for a utility method. Could you send me the code that you use to upload the file using the package?

@kaluax
Copy link
Owner Author

@kaluax kaluax commented on 781195b May 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

function uploadFile($serverPath, $filePath)  {
    $url = 'https://...';

    $cfile = curl_file_create($filePath);

    $data = [
        'action' => 'upload',
        'href' => $serverPath,
        'userfile' => $cfile
    ];

    $curl = Curl::to($url);

    $curl = $curl->withOption('HTTPAUTH', CURLAUTH_BASIC);
    $curl = $curl->withOption('USERPWD', "user:pass");

    $curl = $curl->withData($data);

    $response = $curl->post();

    return $response;
}

@elimentz
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I've added an issue in my repo and I hope to tackle it soon (no promises).

Btw, did you know you can uptimize your code by taking advantage of the fluid query builder?

function uploadFile($serverPath, $filePath)  {
    $url = 'https://...';

    $cfile = curl_file_create($filePath);

    $data = [
        'action' => 'upload',
        'href' => $serverPath,
        'userfile' => $cfile
    ];

    return Curl::to($url)
        ->withOption('HTTPAUTH', CURLAUTH_BASIC);
        ->withOption('USERPWD', "user:pass")
        ->withData($data);
        ->post();
}

Just saying :-)

@kaluax
Copy link
Owner Author

@kaluax kaluax commented on 781195b May 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Jan, looking forward to it!

You're right that this piece of code could be optimized with a fluid query syntax. In the original code I'm using if statements to check wether withData should be included or not. Also makes debugging a little bit easier ;-)

@Thedigit
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kaluax & @elimentz,

i added the pull request to ixudra#21

Please sign in to comment.