Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: remove CURLRequest headers sharing from $_SERVER #5249

Merged
merged 21 commits into from
Nov 1, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
d1edaee
fix: reset unneeded headers to prevent request error
kenjis Oct 19, 2021
245dade
fix: reset unneeded configs
kenjis Oct 19, 2021
fa3620b
test: add test for CURLRequest unshared headers
kenjis Oct 19, 2021
4b51e01
test: fix test method names
kenjis Oct 19, 2021
e307ef5
fix: remove populateHeaders() not to share request headers form brows…
kenjis Oct 28, 2021
9cbe0f2
fix: make $unsharedHeaders protected
kenjis Oct 29, 2021
c9bac5f
fix: reset $unsharedHeaders after sending request
kenjis Oct 29, 2021
d39543e
docs: improve changelogs/v4.1.5.rst format
kenjis Oct 29, 2021
9a6a0d3
docs: add upgrading and changelog
kenjis Oct 29, 2021
613b980
fix: remove $unsharedHeaders and add $defaultOptions
kenjis Oct 29, 2021
8b08642
docs: fix changelogs/v4.1.5.rst
kenjis Oct 29, 2021
182c252
docs: fix upgrade_415.rst
kenjis Oct 29, 2021
aed89d9
fix: add config $CURLRequestShareOptions for the CURLRequest
kenjis Oct 29, 2021
436013e
test: add tests for $CURLRequestShareOptions = false
kenjis Oct 29, 2021
9e88ec4
test: add tests for sharing options
kenjis Oct 29, 2021
a6c06cc
refactor: remove unnecessary second param
kenjis Oct 29, 2021
f53a1da
refactor: move Config\App::$CURLRequestShareOptions to Config\CURLReq…
kenjis Oct 30, 2021
772a408
fix: change from private to protected
kenjis Oct 30, 2021
3901496
docs: add PHPDoc comments
kenjis Oct 30, 2021
8a5973b
config: add curlrequest.shareOptions
kenjis Oct 30, 2021
88963ba
docs: add about CURLRequest $shareOptions
kenjis Oct 30, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 20 additions & 12 deletions system/HTTP/CURLRequest.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,16 @@ class CURLRequest extends Request
*/
protected $delay = 0.0;

/**
* Request headers that are not shared between requests.
*
* @var string[]
*/
private $unsharedHeaders = [
kenjis marked this conversation as resolved.
Show resolved Hide resolved
'Content-Length',
'Content-Type',
];

/**
* Takes an array of options to set the following possible class properties:
*
Expand Down Expand Up @@ -106,6 +116,11 @@ public function __construct(App $config, URI $uri, ?ResponseInterface $response
*/
public function request($method, string $url, array $options = []): ResponseInterface
{
// Reset unshared headers
foreach ($this->unsharedHeaders as $header) {
$this->removeHeader($header);
}

$this->parseOptions($options);

$url = $this->prepareURL($url);
Expand All @@ -114,6 +129,9 @@ public function request($method, string $url, array $options = []): ResponseInte

$this->send($method, $url);

// Reset unshared configs
unset($this->config['multipart'], $this->config['form_params']);
kenjis marked this conversation as resolved.
Show resolved Hide resolved

return $this->response;
}

Expand Down Expand Up @@ -350,27 +368,17 @@ public function send(string $method, string $url)
}

/**
* Takes all headers current part of this request and adds them
* to the cURL request.
* Adds $this->headers to the cURL request.
*/
protected function applyRequestHeaders(array $curlOptions = []): array
{
if (empty($this->headers)) {
$this->populateHeaders();
// Otherwise, it will corrupt the request
$this->removeHeader('Host');
$this->removeHeader('Accept-Encoding');
}

$headers = $this->headers();

if (empty($headers)) {
return $curlOptions;
}

$set = [];

foreach (array_keys($headers) as $name) {
foreach (array_keys($this->headers) as $name) {
$set[] = $name . ': ' . $this->getHeaderLine($name);
}

Expand Down
55 changes: 36 additions & 19 deletions tests/system/HTTP/CURLRequestTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -176,48 +176,65 @@ public function testOptionsHeaders()
/**
* @backupGlobals enabled
*/
public function testOptionHeadersUsingPopulate()
public function testOptionsHeadersNotUsingPopulate()
{
$_SERVER['HTTP_HOST'] = 'site1.com';
$_SERVER['HTTP_ACCEPT_LANGUAGE'] = 'en-US';
$_SERVER['HTTP_ACCEPT_ENCODING'] = 'gzip, deflate, br';

$options = [
'base_uri' => 'http://www.foo.com/api/v1/',
'headers' => [
'Host' => 'www.foo.com',
'Accept-Encoding' => '',
],
];
$request = $this->getRequest($options);
$request->get('example');
// if headers for the request are defined we use them
$this->assertNull($request->header('Accept-Language'));
$this->assertSame('www.foo.com', $request->header('Host')->getValue());
$this->assertSame('', $request->header('Accept-Encoding')->getValue());
}

public function testHeaderContentLengthNotSharedBetweenRequests()
{
$options = [
'base_uri' => 'http://www.foo.com/api/v1/',
];
$request = $this->getRequest($options);

$request->post('example', [
'form_params' => [
'q' => 'keyword',
],
]);
$request->get('example');
// we fill the Accept-Language header from _SERVER when no headers are defined for the request
$this->assertSame('en-US', $request->header('Accept-Language')->getValue());
// but we skip Host header - since it would corrupt the request
$this->assertNull($request->header('Host'));
// and Accept-Encoding
$this->assertNull($request->header('Accept-Encoding'));

$this->assertNull($request->header('Content-Length'));
}

/**
* @backupGlobals enabled
*/
public function testOptionHeadersNotUsingPopulate()
public function testHeaderContentLengthNotSharedBetweenClients()
{
$_SERVER['HTTP_HOST'] = 'site1.com';
$_SERVER['HTTP_ACCEPT_LANGUAGE'] = 'en-US';
$_SERVER['HTTP_ACCEPT_ENCODING'] = 'gzip, deflate, br';
$_SERVER['HTTP_CONTENT_LENGTH'] = '10';

$options = [
'base_uri' => 'http://www.foo.com/api/v1/',
'headers' => [
'Host' => 'www.foo.com',
'Accept-Encoding' => '',
],
];
$request = $this->getRequest($options);
$request->post('example', [
'form_params' => [
'q' => 'keyword',
],
]);

$request = $this->getRequest($options);
$request->get('example');
// if headers for the request are defined we use them
$this->assertNull($request->header('Accept-Language'));
$this->assertSame('www.foo.com', $request->header('Host')->getValue());
$this->assertSame('', $request->header('Accept-Encoding')->getValue());

$this->assertNull($request->header('Content-Length'));
}

public function testOptionsDelay()
Expand Down