educoder / pest

A proper REST client for PHP.
356 stars 123 forks source link

Fixed curl default options for headers being ignored #46

Open ghost opened 11 years ago

ghost commented 11 years ago

Methods put, post, patch, get, delete did provide an $header parameter, which (in all cases) overwrites the default options set in public $curl_opts, even if values are not present in the $header parameter. This has been changed so that the parameter data is only merged into the default parameters.

djsipe commented 11 years ago

So the idea is that the default headers are retained and only overwritten when the user provides an override?

ghost commented 11 years ago

Pretty much. Due to the fact that the $curl_opt[CURL_HTTPHEADERS] array can take both named and numeric entries, there two possibilities:

  1. The parameters are set as numeric parameters. Then the both parameters are joined by curl. This does not make sense, obviously, when a parameter should not contain multiple values
  2. The parameters are set as named parameters / the array is used in an associative way. Then the function parameter overwrites the default parameter.

Examples:

<?php require_once('Pest.php');

if(!isset($_GET['sub'])) { $host = ((isset($_SERVER['HTTPS']) && !empty($_SERVER['HTTPS'])) ? 'https://' : 'http://').$_SERVER['HTTP_HOST'];

$pest = new \Pest($host);
$pest->curl_opts[CURLOPT_HTTPHEADER][] = 'Accept: text/html';
$result = $pest->get($_SERVER['PHP_SELF'], array('sub' => 'yes'), array('Accept: application/xhtml+xml'));
echo 'received headers: '.$result;
// Results in [Host] => _your_hostname_ [Accept] => text/html, application/xhtml+xml [Connection] => close

echo '<br />';

$pest = new \Pest($host);
$pest->curl_opts[CURLOPT_HTTPHEADER]['Content-type'] = 'text/plain';
$result = $pest->get($_SERVER['PHP_SELF'], array('sub' => 'yes'));
echo 'received headers: '.$result;
// Results in [Host] => _your_hostname_ [Accept] => */* [Content-type] => text/plain [Connection] => close

echo '<br />';

$pest = new \Pest($host);
$pest->curl_opts[CURLOPT_HTTPHEADER]['Content-type'] = 'text/plain';
$result = $pest->get($_SERVER['PHP_SELF'], array('sub' => 'yes'), array('Content-type' => 'application/json'));
echo 'received headers: '.$result;
// Results in [Host] => www.ebuero.de [Accept] => */* [Content-type] => application/json [X-Forwarded-For] => 192.168.1.249 [Connection] => close

} else { print_r(apache_request_headers()); } ?>

While this will work well when programmers know what they are doing, it might lead to problems when mixing up named and numeric parameters or mixing up casing, because Content-type does not overwrite Content-Type etc.

The better solution would be adding a method which takes all default parameters and making the $curl_opt private. If you are interested in something like that, I'd be happy to supply you with a solution once I got some spare time.

zuk commented 10 years ago

This makes sense but there's some horrible whitespace stuff going on in this pull request.