From d1366d3a8d5d286ee485d2af91e629d3f30ea7d1 Mon Sep 17 00:00:00 2001 From: Philipp Date: Wed, 7 Oct 2020 20:59:55 +0200 Subject: [PATCH 01/15] Add guzzlehttp/guzzle as composer requirement --- composer.json | 1 + composer.lock | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index efd4bd0812..08216159e3 100644 --- a/composer.json +++ b/composer.json @@ -31,6 +31,7 @@ "divineomega/password_exposed": "^2.8", "ezyang/htmlpurifier": "^4.7", "friendica/json-ld": "^1.0", + "guzzlehttp/guzzle": "^6.5", "league/html-to-markdown": "^4.8", "level-2/dice": "^4", "lightopenid/lightopenid": "dev-master", diff --git a/composer.lock b/composer.lock index 8ba31ecb03..0d650ad155 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "fd22bd8c29dcea3d6b6eeb117d79af52", + "content-hash": "aa279ba22954af464cd7b4a5e2d74eec", "packages": [ { "name": "asika/simple-console", From f238f4efbe63d5a888e5480e17a922d8cffcd3d2 Mon Sep 17 00:00:00 2001 From: Philipp Date: Wed, 7 Oct 2020 21:45:22 +0200 Subject: [PATCH 02/15] Introduce IHTTPResult Interface as abstraction for CurlResult --- src/Network/CurlResult.php | 72 ++++++---------------------- src/Network/IHTTPResult.php | 93 +++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 57 deletions(-) create mode 100644 src/Network/IHTTPResult.php diff --git a/src/Network/CurlResult.php b/src/Network/CurlResult.php index 9f52edfad6..072ab15c27 100644 --- a/src/Network/CurlResult.php +++ b/src/Network/CurlResult.php @@ -29,7 +29,7 @@ use Friendica\Util\Network; /** * A content class for Curl call results */ -class CurlResult +class CurlResult implements IHTTPResult { /** * @var int HTTP return code or 0 if timeout or failure @@ -229,33 +229,19 @@ class CurlResult } } - /** - * Gets the Curl Code - * - * @return string The Curl Code - */ + /** {@inheritDoc} */ public function getReturnCode() { return $this->returnCode; } - /** - * Returns the Curl Content Type - * - * @return string the Curl Content Type - */ + /** {@inheritDoc} */ public function getContentType() { return $this->contentType; } - /** - * Returns the Curl headers - * - * @param string $field optional header field. Return all fields if empty - * - * @return string the Curl headers or the specified content of the header variable - */ + /** {@inheritDoc} */ public function getHeader(string $field = '') { if (empty($field)) { @@ -273,13 +259,7 @@ class CurlResult return ''; } - /** - * Check if a specified header exists - * - * @param string $field header field - * - * @return boolean "true" if header exists - */ + /** {@inheritDoc} */ public function inHeader(string $field) { $field = strtolower(trim($field)); @@ -289,11 +269,7 @@ class CurlResult return array_key_exists($field, $headers); } - /** - * Returns the Curl headers as an associated array - * - * @return array associated header array - */ + /** {@inheritDoc} */ public function getHeaderArray() { if (!empty($this->header_fields)) { @@ -313,73 +289,55 @@ class CurlResult return $this->header_fields; } - /** - * @return bool - */ + /** {@inheritDoc} */ public function isSuccess() { return $this->isSuccess; } - /** - * @return string - */ + /** {@inheritDoc} */ public function getUrl() { return $this->url; } - /** - * @return string - */ + /** {@inheritDoc} */ public function getRedirectUrl() { return $this->redirectUrl; } - /** - * @return string - */ + /** {@inheritDoc} */ public function getBody() { return $this->body; } - /** - * @return array - */ + /** {@inheritDoc} */ public function getInfo() { return $this->info; } - /** - * @return bool - */ + /** {@inheritDoc} */ public function isRedirectUrl() { return $this->isRedirectUrl; } - /** - * @return int - */ + /** {@inheritDoc} */ public function getErrorNumber() { return $this->errorNumber; } - /** - * @return string - */ + /** {@inheritDoc} */ public function getError() { return $this->error; } - /** - * @return bool - */ + /** {@inheritDoc} */ public function isTimeout() { return $this->isTimeout; diff --git a/src/Network/IHTTPResult.php b/src/Network/IHTTPResult.php new file mode 100644 index 0000000000..be190c80c1 --- /dev/null +++ b/src/Network/IHTTPResult.php @@ -0,0 +1,93 @@ + Date: Wed, 7 Oct 2020 21:49:12 +0200 Subject: [PATCH 03/15] Replace IHTTPResult for CurlResult usages --- src/Model/GServer.php | 13 +++++++------ src/Network/CurlResult.php | 2 +- src/Network/IHTTPRequest.php | 6 +++--- tests/src/Core/InstallerTest.php | 26 +++++++++++++------------- 4 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/Model/GServer.php b/src/Model/GServer.php index 323a23f494..ac86ef38b9 100644 --- a/src/Model/GServer.php +++ b/src/Model/GServer.php @@ -30,7 +30,7 @@ use Friendica\Core\Worker; use Friendica\Database\DBA; use Friendica\DI; use Friendica\Module\Register; -use Friendica\Network\CurlResult; +use Friendica\Network\IHTTPResult; use Friendica\Protocol\Diaspora; use Friendica\Util\DateTimeFormat; use Friendica\Util\Network; @@ -630,18 +630,19 @@ class GServer /** * Detect server type by using the nodeinfo data * - * @param string $url address of the server - * @param CurlResult $curlResult + * @param string $url address of the server + * @param IHTTPResult $httpResult + * * @return array Server data * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - private static function fetchNodeinfo(string $url, CurlResult $curlResult) + private static function fetchNodeinfo(string $url, IHTTPResult $httpResult) { - if (!$curlResult->isSuccess()) { + if (!$httpResult->isSuccess()) { return []; } - $nodeinfo = json_decode($curlResult->getBody(), true); + $nodeinfo = json_decode($httpResult->getBody(), true); if (!is_array($nodeinfo) || empty($nodeinfo['links'])) { return []; diff --git a/src/Network/CurlResult.php b/src/Network/CurlResult.php index 072ab15c27..1187e45eb6 100644 --- a/src/Network/CurlResult.php +++ b/src/Network/CurlResult.php @@ -101,7 +101,7 @@ class CurlResult implements IHTTPResult * * @param string $url optional URL * - * @return CurlResult a CURL with error response + * @return IHTTPResult a CURL with error response * @throws InternalServerErrorException */ public static function createErrorCurl($url = '') diff --git a/src/Network/IHTTPRequest.php b/src/Network/IHTTPRequest.php index 3ebcc5dc1b..3f9b7f27a1 100644 --- a/src/Network/IHTTPRequest.php +++ b/src/Network/IHTTPRequest.php @@ -57,7 +57,7 @@ interface IHTTPRequest * @param string $accept_content supply Accept: header with 'accept_content' as the value * @param string $cookiejar Path to cookie jar file * - * @return CurlResult With all relevant information, 'body' contains the actual fetched content. + * @return IHTTPResult With all relevant information, 'body' contains the actual fetched content. */ public function fetchFull(string $url, bool $binary = false, int $timeout = 0, string $accept_content = '', string $cookiejar = ''); @@ -76,7 +76,7 @@ interface IHTTPRequest * 'cookiejar' => path to cookie jar file * 'header' => header array * - * @return CurlResult + * @return IHTTPResult */ public function get(string $url, bool $binary = false, array $opts = []); @@ -88,7 +88,7 @@ interface IHTTPRequest * @param array $headers HTTP headers * @param int $timeout The timeout in seconds, default system config value or 60 seconds * - * @return CurlResult The content + * @return IHTTPResult The content */ public function post(string $url, $params, array $headers = [], int $timeout = 0); diff --git a/tests/src/Core/InstallerTest.php b/tests/src/Core/InstallerTest.php index 00879680ee..37a754bc91 100644 --- a/tests/src/Core/InstallerTest.php +++ b/tests/src/Core/InstallerTest.php @@ -25,7 +25,7 @@ namespace Friendica\Core; use Dice\Dice; use Friendica\Core\Config\Cache; use Friendica\DI; -use Friendica\Network\CurlResult; +use Friendica\Network\IHTTPResult; use Friendica\Network\IHTTPRequest; use Friendica\Test\MockedTest; use Friendica\Test\Util\VFSTrait; @@ -297,14 +297,14 @@ class InstallerTest extends MockedTest $this->l10nMock->shouldReceive('t')->andReturnUsing(function ($args) { return $args; }); // Mocking the CURL Response - $curlResult = \Mockery::mock(CurlResult::class); - $curlResult + $IHTTPResult = \Mockery::mock(IHTTPResult::class); + $IHTTPResult ->shouldReceive('getReturnCode') ->andReturn('404'); - $curlResult + $IHTTPResult ->shouldReceive('getRedirectUrl') ->andReturn(''); - $curlResult + $IHTTPResult ->shouldReceive('getError') ->andReturn('test Error'); @@ -313,11 +313,11 @@ class InstallerTest extends MockedTest $networkMock ->shouldReceive('fetchFull') ->with('https://test/install/testrewrite') - ->andReturn($curlResult); + ->andReturn($IHTTPResult); $networkMock ->shouldReceive('fetchFull') ->with('http://test/install/testrewrite') - ->andReturn($curlResult); + ->andReturn($IHTTPResult); $this->dice->shouldReceive('create') ->with(IHTTPRequest::class) @@ -344,14 +344,14 @@ class InstallerTest extends MockedTest $this->l10nMock->shouldReceive('t')->andReturnUsing(function ($args) { return $args; }); // Mocking the failed CURL Response - $curlResultF = \Mockery::mock(CurlResult::class); - $curlResultF + $IHTTPResultF = \Mockery::mock(IHTTPResult::class); + $IHTTPResultF ->shouldReceive('getReturnCode') ->andReturn('404'); // Mocking the working CURL Response - $curlResultW = \Mockery::mock(CurlResult::class); - $curlResultW + $IHTTPResultW = \Mockery::mock(IHTTPResult::class); + $IHTTPResultW ->shouldReceive('getReturnCode') ->andReturn('204'); @@ -360,11 +360,11 @@ class InstallerTest extends MockedTest $networkMock ->shouldReceive('fetchFull') ->with('https://test/install/testrewrite') - ->andReturn($curlResultF); + ->andReturn($IHTTPResultF); $networkMock ->shouldReceive('fetchFull') ->with('http://test/install/testrewrite') - ->andReturn($curlResultW); + ->andReturn($IHTTPResultW); $this->dice->shouldReceive('create') ->with(IHTTPRequest::class) From fff94563d7d5d17e1568612fea0089e3cb35ba46 Mon Sep 17 00:00:00 2001 From: Philipp Date: Thu, 8 Oct 2020 21:20:41 +0200 Subject: [PATCH 04/15] Use Guzzle for HTTPRequest and Result --- src/Network/GuzzleResponse.php | 152 +++++++++++++++++++++++++++++++++ src/Network/HTTPRequest.php | 120 +++++++++++++------------- 2 files changed, 210 insertions(+), 62 deletions(-) create mode 100644 src/Network/GuzzleResponse.php diff --git a/src/Network/GuzzleResponse.php b/src/Network/GuzzleResponse.php new file mode 100644 index 0000000000..c6cab7c9e8 --- /dev/null +++ b/src/Network/GuzzleResponse.php @@ -0,0 +1,152 @@ +. + * + */ + +namespace Friendica\Network; + +use Friendica\Core\Logger; +use Friendica\Core\System; +use Friendica\Network\HTTPException\NotImplementedException; +use GuzzleHttp\Psr7\Response; +use Psr\Http\Message\ResponseInterface; + +/** + * A content wrapper class for Guzzle call results + */ +class GuzzleResponse extends Response implements IHTTPResult, ResponseInterface +{ + /** @var string The URL */ + private $url; + /** @var boolean */ + private $isTimeout; + /** @var boolean */ + private $isSuccess; + /** + * @var int the error number or 0 (zero) if no error + */ + private $errorNumber; + + /** + * @var string the error message or '' (the empty string) if no + */ + private $error; + + public function __construct(ResponseInterface $response, string $url, $errorNumber = 0, $error = '') + { + parent::__construct($response->getStatusCode(), $response->getHeaders(), $response->getBody(), $response->getProtocolVersion(), $response->getReasonPhrase()); + $this->url = $url; + $this->error = $error; + $this->errorNumber = $errorNumber; + + $this->checkSuccess(); + } + + private function checkSuccess() + { + $this->isSuccess = ($this->getStatusCode() >= 200 && $this->getStatusCode() <= 299) || $this->errorNumber == 0; + + // Everything higher or equal 400 is not a success + if ($this->getReturnCode() >= 400) { + $this->isSuccess = false; + } + + if (!$this->isSuccess) { + Logger::notice('http error', ['url' => $this->url, 'code' => $this->getReturnCode(), 'error' => $this->error, 'callstack' => System::callstack(20)]); + Logger::debug('debug', ['info' => $this->getHeaders()]); + } + + if (!$this->isSuccess && $this->errorNumber == CURLE_OPERATION_TIMEDOUT) { + $this->isTimeout = true; + } else { + $this->isTimeout = false; + } + } + + /** {@inheritDoc} */ + public function getReturnCode() + { + return $this->getStatusCode(); + } + + /** {@inheritDoc} */ + public function getContentType() + { + return $this->getHeader('Content-Type'); + } + + /** {@inheritDoc} */ + public function inHeader(string $field) + { + return $this->hasHeader($field); + } + + /** {@inheritDoc} */ + public function getHeaderArray() + { + return $this->getHeaders(); + } + + /** {@inheritDoc} */ + public function isSuccess() + { + return $this->isSuccess; + } + + /** {@inheritDoc} */ + public function getUrl() + { + return $this->url; + } + + /** {@inheritDoc} */ + public function getRedirectUrl() + { + return $this->url; + } + + public function getInfo() + { + // TODO: Implement getInfo() method. + } + + /** {@inheritDoc} */ + public function isRedirectUrl() + { + throw new NotImplementedException(); + } + + /** {@inheritDoc} */ + public function getErrorNumber() + { + return $this->errorNumber; + } + + /** {@inheritDoc} */ + public function getError() + { + return $this->error; + } + + /** {@inheritDoc} */ + public function isTimeout() + { + return $this->isTimeout; + } +} diff --git a/src/Network/HTTPRequest.php b/src/Network/HTTPRequest.php index e4ff041039..6341c07297 100644 --- a/src/Network/HTTPRequest.php +++ b/src/Network/HTTPRequest.php @@ -28,6 +28,11 @@ use Friendica\Core\Config\IConfig; use Friendica\Core\System; use Friendica\Util\Network; use Friendica\Util\Profiler; +use GuzzleHttp\Client; +use GuzzleHttp\Exception\RequestException; +use Psr\Http\Message\RequestInterface; +use Psr\Http\Message\ResponseInterface; +use Psr\Http\Message\UriInterface; use Psr\Log\LoggerInterface; /** @@ -54,12 +59,8 @@ class HTTPRequest implements IHTTPRequest /** * {@inheritDoc} - * - * @param int $redirects The recursion counter for internal use - default 0 - * - * @throws \Friendica\Network\HTTPException\InternalServerErrorException */ - public function get(string $url, bool $binary = false, array $opts = [], int &$redirects = 0) + public function get(string $url, bool $binary = false, array $opts = []) { $stamp1 = microtime(true); @@ -86,124 +87,119 @@ class HTTPRequest implements IHTTPRequest return CurlResult::createErrorCurl($url); } - $ch = @curl_init($url); + $curlOptions = []; - if (($redirects > 8) || (!$ch)) { - return CurlResult::createErrorCurl($url); - } - - @curl_setopt($ch, CURLOPT_HEADER, true); + $curlOptions[CURLOPT_HEADER] = true; if (!empty($opts['cookiejar'])) { - curl_setopt($ch, CURLOPT_COOKIEJAR, $opts["cookiejar"]); - curl_setopt($ch, CURLOPT_COOKIEFILE, $opts["cookiejar"]); + $curlOptions[CURLOPT_COOKIEJAR] = $opts["cookiejar"]; + $curlOptions[CURLOPT_COOKIEFILE] = $opts["cookiejar"]; } // These settings aren't needed. We're following the location already. - // @curl_setopt($ch, CURLOPT_FOLLOWLOCATION, true); - // @curl_setopt($ch, CURLOPT_MAXREDIRS, 5); + // $curlOptions[CURLOPT_FOLLOWLOCATION] =true; + // $curlOptions[CURLOPT_MAXREDIRS] = 5; if (!empty($opts['accept_content'])) { - curl_setopt( - $ch, - CURLOPT_HTTPHEADER, - ['Accept: ' . $opts['accept_content']] - ); + $curlOptions[CURLOPT_HTTPHEADER][] = ['Accept: ' . $opts['accept_content']]; } if (!empty($opts['header'])) { - curl_setopt($ch, CURLOPT_HTTPHEADER, $opts['header']); + $curlOptions[CURLOPT_HTTPHEADER][] = $opts['header']; } - @curl_setopt($ch, CURLOPT_RETURNTRANSFER, true); - @curl_setopt($ch, CURLOPT_USERAGENT, $this->getUserAgent()); + $curlOptions[CURLOPT_RETURNTRANSFER] = true; + $curlOptions[CURLOPT_USERAGENT] = $this->getUserAgent(); $range = intval($this->config->get('system', 'curl_range_bytes', 0)); if ($range > 0) { - @curl_setopt($ch, CURLOPT_RANGE, '0-' . $range); + $curlOptions[CURLOPT_RANGE] = '0-' . $range; } // Without this setting it seems as if some webservers send compressed content // This seems to confuse curl so that it shows this uncompressed. /// @todo We could possibly set this value to "gzip" or something similar - curl_setopt($ch, CURLOPT_ENCODING, ''); + $curlOptions[CURLOPT_ENCODING] = ''; if (!empty($opts['headers'])) { - @curl_setopt($ch, CURLOPT_HTTPHEADER, $opts['headers']); + $curlOptions[CURLOPT_HTTPHEADER][] = $opts['headers']; } if (!empty($opts['nobody'])) { - @curl_setopt($ch, CURLOPT_NOBODY, $opts['nobody']); + $curlOptions[CURLOPT_NOBODY] = $opts['nobody']; } - @curl_setopt($ch, CURLOPT_CONNECTTIMEOUT, 10); + $curlOptions[CURLOPT_CONNECTTIMEOUT] = 10; if (!empty($opts['timeout'])) { - @curl_setopt($ch, CURLOPT_TIMEOUT, $opts['timeout']); + $curlOptions[CURLOPT_TIMEOUT] = $opts['timeout']; } else { $curl_time = $this->config->get('system', 'curl_timeout', 60); - @curl_setopt($ch, CURLOPT_TIMEOUT, intval($curl_time)); + $curlOptions[CURLOPT_TIMEOUT] = intval($curl_time); } // by default we will allow self-signed certs // but you can override this $check_cert = $this->config->get('system', 'verifyssl'); - @curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, (($check_cert) ? true : false)); + $curlOptions[CURLOPT_SSL_VERIFYPEER] = ($check_cert) ? true : false; if ($check_cert) { - @curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, 2); + $curlOptions[CURLOPT_SSL_VERIFYHOST] = 2; } $proxy = $this->config->get('system', 'proxy'); if (!empty($proxy)) { - @curl_setopt($ch, CURLOPT_HTTPPROXYTUNNEL, 1); - @curl_setopt($ch, CURLOPT_PROXY, $proxy); + $curlOptions[CURLOPT_HTTPPROXYTUNNEL] = 1; + $curlOptions[CURLOPT_PROXY] = $proxy; $proxyuser = $this->config->get('system', 'proxyuser'); if (!empty($proxyuser)) { - @curl_setopt($ch, CURLOPT_PROXYUSERPWD, $proxyuser); + $curlOptions[CURLOPT_PROXYUSERPWD] = $proxyuser; } } if ($this->config->get('system', 'ipv4_resolve', false)) { - curl_setopt($ch, CURLOPT_IPRESOLVE, CURL_IPRESOLVE_V4); + $curlOptions[CURLOPT_IPRESOLVE] = CURL_IPRESOLVE_V4; } if ($binary) { - @curl_setopt($ch, CURLOPT_BINARYTRANSFER, 1); + $curlOptions[CURLOPT_BINARYTRANSFER] = 1; } - // don't let curl abort the entire application - // if it throws any errors. + $onRedirect = function( + RequestInterface $request, + ResponseInterface $response, + UriInterface $uri + ) { + $this->logger->notice('Curl redirect.', ['url' => $request->getUri(), 'to' => $uri]); + }; - $s = @curl_exec($ch); - $curl_info = @curl_getinfo($ch); + $client = new Client([ + 'allow_redirect' => [ + 'max' => 8, + 'on_redirect' => $onRedirect, + 'track_redirect' => true, + 'strict' => true, + 'referer' => true, + ], + 'curl' => $curlOptions + ]); - // Special treatment for HTTP Code 416 - // See https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/416 - if (($curl_info['http_code'] == 416) && ($range > 0)) { - @curl_setopt($ch, CURLOPT_RANGE, ''); - $s = @curl_exec($ch); - $curl_info = @curl_getinfo($ch); + try { + $response = $client->get($url); + return new GuzzleResponse($response, $url); + } catch (RequestException $exception) { + if ($exception->hasResponse()) { + return new GuzzleResponse($exception->getResponse(), $url, $exception->getCode(), $exception->getMessage()); + } else { + return new GuzzleResponse(null, $url, $exception->getCode(), $exception->getMessage()); + } + } finally { + $this->profiler->saveTimestamp($stamp1, 'network'); } - - $curlResponse = new CurlResult($url, $s, $curl_info, curl_errno($ch), curl_error($ch)); - - if (!Network::isRedirectBlocked($url) && $curlResponse->isRedirectUrl()) { - $redirects++; - $this->logger->notice('Curl redirect.', ['url' => $url, 'to' => $curlResponse->getRedirectUrl()]); - @curl_close($ch); - return $this->get($curlResponse->getRedirectUrl(), $binary, $opts, $redirects); - } - - @curl_close($ch); - - $this->profiler->saveTimestamp($stamp1, 'network'); - - return $curlResponse; } /** From 933ea7c9ceefc9b38080f56ee214099a2b1118a4 Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 9 Oct 2020 19:33:19 +0200 Subject: [PATCH 05/15] Fix IHTTPResult::getHeader/s() - Split functionality "getHeader()" and "getHeaders()" analog to IMessageInterface::getHeader/s() - Fix functionality at various places - Adapt CurlResultTest --- mod/parse_url.php | 11 ++++++----- src/Network/CurlResult.php | 18 ++++++++++++------ src/Network/HTTPRequest.php | 3 +-- src/Network/IHTTPResult.php | 22 +++++++++++++++++++--- src/Protocol/DFRN.php | 4 ++-- src/Protocol/OStatus.php | 6 ++++-- src/Protocol/Salmon.php | 2 +- src/Util/ParseUrl.php | 3 +-- tests/datasets/curl/about.head.php | 20 ++++++++++++++++++++ tests/datasets/curl/about.redirect.php | 21 +++++++++++++++++++++ tests/src/Network/CurlResultTest.php | 14 +++++++++----- 11 files changed, 96 insertions(+), 28 deletions(-) create mode 100644 tests/datasets/curl/about.head.php create mode 100644 tests/datasets/curl/about.redirect.php diff --git a/mod/parse_url.php b/mod/parse_url.php index a1faab6efb..39aae4a5a0 100644 --- a/mod/parse_url.php +++ b/mod/parse_url.php @@ -90,12 +90,13 @@ function parse_url_content(App $a) if ($curlResponse->isSuccess()) { // Convert the header fields into an array $hdrs = []; - $h = explode("\n", $curlResponse->getHeader()); + $h = $curlResponse->getHeaders(); foreach ($h as $l) { - $header = array_map('trim', explode(':', trim($l), 2)); - if (count($header) == 2) { - list($k, $v) = $header; - $hdrs[$k] = $v; + foreach ($l as $k => $v) { + if (empty($hdrs[$k])) { + $hdrs[$k] = $v; + } + $hdrs[$k] .= " " . $v; } } $type = null; diff --git a/src/Network/CurlResult.php b/src/Network/CurlResult.php index 1187e45eb6..b0e801010d 100644 --- a/src/Network/CurlResult.php +++ b/src/Network/CurlResult.php @@ -242,23 +242,29 @@ class CurlResult implements IHTTPResult } /** {@inheritDoc} */ - public function getHeader(string $field = '') + public function getHeader($header) { - if (empty($field)) { - return $this->header; + if (empty($header)) { + return ''; } - $field = strtolower(trim($field)); + $header = strtolower(trim($header)); $headers = $this->getHeaderArray(); - if (isset($headers[$field])) { - return $headers[$field]; + if (isset($headers[$header])) { + return $headers[$header]; } return ''; } + /** {@inheritDoc} */ + public function getHeaders() + { + return $this->getHeaderArray(); + } + /** {@inheritDoc} */ public function inHeader(string $field) { diff --git a/src/Network/HTTPRequest.php b/src/Network/HTTPRequest.php index 6341c07297..798b0cdea8 100644 --- a/src/Network/HTTPRequest.php +++ b/src/Network/HTTPRequest.php @@ -454,8 +454,7 @@ class HTTPRequest implements IHTTPRequest 'timeout' => $timeout, 'accept_content' => $accept_content, 'cookiejar' => $cookiejar - ], - $redirects + ] ); } diff --git a/src/Network/IHTTPResult.php b/src/Network/IHTTPResult.php index be190c80c1..5904bcfa38 100644 --- a/src/Network/IHTTPResult.php +++ b/src/Network/IHTTPResult.php @@ -2,6 +2,8 @@ namespace Friendica\Network; +use Psr\Http\Message\MessageInterface; + /** * Temporary class to map Friendica used variables based on PSR-7 HTTPResponse */ @@ -23,15 +25,25 @@ interface IHTTPResult /** * Returns the headers + * @see MessageInterface::getHeader() * - * @param string $field optional header field. Return all fields if empty + * @param string $header optional header field. Return all fields if empty * * @return string the headers or the specified content of the header variable */ - public function getHeader(string $field = ''); + public function getHeader($header); + + /** + * Returns all headers + * @see MessageInterface::getHeaders() + * + * @return string[][] + */ + public function getHeaders(); /** * Check if a specified header exists + * @see MessageInterface::hasHeader() * * @param string $field header field * @@ -41,8 +53,10 @@ interface IHTTPResult /** * Returns the headers as an associated array + * @see MessageInterface::getHeaders() + * @deprecated * - * @return array associated header array + * @return string[][] associated header array */ public function getHeaderArray(); @@ -62,6 +76,8 @@ interface IHTTPResult public function getRedirectUrl(); /** + * @see MessageInterface::getBody() + * * @return string */ public function getBody(); diff --git a/src/Protocol/DFRN.php b/src/Protocol/DFRN.php index d20864cf7f..f718e0a741 100644 --- a/src/Protocol/DFRN.php +++ b/src/Protocol/DFRN.php @@ -1358,7 +1358,7 @@ class DFRN return -9; // timed out } - if (($curl_stat == 503) && stristr($postResult->getHeader(), 'retry-after')) { + if (($curl_stat == 503) && $postResult->inHeader('retry-after')) { return -10; } @@ -1453,7 +1453,7 @@ class DFRN return -9; // timed out } - if (($curl_stat == 503) && (stristr($postResult->getHeader(), 'retry-after'))) { + if (($curl_stat == 503) && $postResult->inHeader('retry-after')) { return -10; } diff --git a/src/Protocol/OStatus.php b/src/Protocol/OStatus.php index 0635be87d1..ef38aaebe3 100644 --- a/src/Protocol/OStatus.php +++ b/src/Protocol/OStatus.php @@ -746,7 +746,8 @@ class OStatus $xml = ''; - if (stristr($curlResult->getHeader(), 'Content-Type: application/atom+xml')) { + if ($curlResult->inHeader('Content-Type') && + stristr($curlResult->getHeader('Content-Type'), 'application/atom+xml')) { $xml = $curlResult->getBody(); } @@ -939,7 +940,8 @@ class OStatus $xml = ''; - if (stristr($curlResult->getHeader(), 'Content-Type: application/atom+xml')) { + if ($curlResult->inHeader('Content-Type') && + stristr($curlResult->getHeader('Content-Type'), 'application/atom+xml')) { Logger::log('Directly fetched XML for URI ' . $related_uri, Logger::DEBUG); $xml = $curlResult->getBody(); } diff --git a/src/Protocol/Salmon.php b/src/Protocol/Salmon.php index 169a4d0cbd..8084625925 100644 --- a/src/Protocol/Salmon.php +++ b/src/Protocol/Salmon.php @@ -215,7 +215,7 @@ class Salmon return -1; } - if (($return_code == 503) && (stristr($postResult->getHeader(), 'retry-after'))) { + if (($return_code == 503) && $postResult->inHeader('retry-after')) { return -1; } diff --git a/src/Util/ParseUrl.php b/src/Util/ParseUrl.php index 1596e015be..c3cbda70e3 100644 --- a/src/Util/ParseUrl.php +++ b/src/Util/ParseUrl.php @@ -175,7 +175,6 @@ class ParseUrl return $siteinfo; } - $header = $curlResult->getHeader(); $body = $curlResult->getBody(); if ($do_oembed) { @@ -204,7 +203,7 @@ class ParseUrl $charset = ''; // Look for a charset, first in headers // Expected form: Content-Type: text/html; charset=ISO-8859-4 - if (preg_match('/charset=([a-z0-9-_.\/]+)/i', $header, $matches)) { + if (preg_match('/charset=([a-z0-9-_.\/]+)/i', $curlResult->getContentType(), $matches)) { $charset = trim(trim(trim(array_pop($matches)), ';,')); } diff --git a/tests/datasets/curl/about.head.php b/tests/datasets/curl/about.head.php new file mode 100644 index 0000000000..d0be0feb4f --- /dev/null +++ b/tests/datasets/curl/about.head.php @@ -0,0 +1,20 @@ + '', + 'date' => 'Thu, 11 Oct 2018 18:43:54 GMT', + 'content-type' => 'text/html; charset=utf-8', + 'vary' => 'Accept-Encoding', + 'server' => 'Mastodon', + 'x-frame-options' => 'SAMEORIGIN', + 'x-content-type-options' => 'nosniff', + 'x-xss-protection' => '1; mode=block', + 'etag' => 'W/"706e6c48957e1d46ecf9d7597a7880af"', + 'cache-control' => 'max-age=0, private, must-revalidate', + 'set-cookie' => '_mastodon_session=v3kcy%2FW3aZYBBvZUohuwksEKwzYIyEUlEuJ1KqTAfWPKvVQq%2F4UuJ39zp621VyfpQNlvY46TL%2FYutzXowSLYQBNFCJcrEiF04aU0TdtHls9zynMiyeHhoVgCijOXWXNt9%2FCmpQ49RkNEujkv9NaJ0cum32MCVZKjE9%2BMKmLM%2F8ZygZeLBGJ7sg%3D%3D--QGIiU0%2FpXc3Aym8F--he2iRRPePOdtEs3z%2BufSXg%3D%3D; path=/; secure; HttpOnly', + 'x-request-id' => 'a0c0b8e7-cd60-4efa-b79b-cf1b0d5a0784', + 'x-runtime' => '0.049566', + 'strict-transport-security' => 'max-age=31536000; includeSubDomains; preload', + 'referrer-policy' => 'same-origin', + 'content-security-policy' => "frame-ancestors 'none'; script-src 'self'; object-src 'self'; img-src * data: blob:; media-src 'self' data:; font-src 'self' data: https://fonts.gstatic.com/; connect-src 'self' blob: wss://mastodonten.de", +]; diff --git a/tests/datasets/curl/about.redirect.php b/tests/datasets/curl/about.redirect.php new file mode 100644 index 0000000000..5ae3fd88f7 --- /dev/null +++ b/tests/datasets/curl/about.redirect.php @@ -0,0 +1,21 @@ + '', + 'date' => 'Thu, 11 Oct 2018 18:43:54 GMT', + 'content-type' => 'text/html; charset=utf-8', + 'vary' => 'Accept-Encoding', + 'server' => 'Mastodon', + 'location' => 'https://test.other/some/', + 'x-frame-options' => 'SAMEORIGIN', + 'x-content-type-options' => 'nosniff', + 'x-xss-protection' => '1; mode=block', + 'etag' => 'W/"706e6c48957e1d46ecf9d7597a7880af"', + 'cache-control' => 'max-age=0, private, must-revalidate', + 'set-cookie' => '_mastodon_session=v3kcy%2FW3aZYBBvZUohuwksEKwzYIyEUlEuJ1KqTAfWPKvVQq%2F4UuJ39zp621VyfpQNlvY46TL%2FYutzXowSLYQBNFCJcrEiF04aU0TdtHls9zynMiyeHhoVgCijOXWXNt9%2FCmpQ49RkNEujkv9NaJ0cum32MCVZKjE9%2BMKmLM%2F8ZygZeLBGJ7sg%3D%3D--QGIiU0%2FpXc3Aym8F--he2iRRPePOdtEs3z%2BufSXg%3D%3D; path=/; secure; HttpOnly', + 'x-request-id' => 'a0c0b8e7-cd60-4efa-b79b-cf1b0d5a0784', + 'x-runtime' => '0.049566', + 'strict-transport-security' => 'max-age=31536000; includeSubDomains; preload', + 'referrer-policy' => 'same-origin', + 'content-security-policy' => "frame-ancestors 'none'; script-src 'self'; object-src 'self'; img-src * data: blob:; media-src 'self' data:; font-src 'self' data: https://fonts.gstatic.com/; connect-src 'self' blob: wss://mastodonten.de", +]; diff --git a/tests/src/Network/CurlResultTest.php b/tests/src/Network/CurlResultTest.php index e066fb89b0..03a5288124 100644 --- a/tests/src/Network/CurlResultTest.php +++ b/tests/src/Network/CurlResultTest.php @@ -53,6 +53,7 @@ class CurlResultTest extends TestCase public function testNormal() { $header = file_get_contents(__DIR__ . '/../../datasets/curl/about.head'); + $headerArray = include(__DIR__ . '/../../datasets/curl/about.head.php'); $body = file_get_contents(__DIR__ . '/../../datasets/curl/about.body'); @@ -65,7 +66,7 @@ class CurlResultTest extends TestCase $this->assertTrue($curlResult->isSuccess()); $this->assertFalse($curlResult->isTimeout()); $this->assertFalse($curlResult->isRedirectUrl()); - $this->assertSame($header, $curlResult->getHeader()); + $this->assertSame($headerArray, $curlResult->getHeaders()); $this->assertSame($body, $curlResult->getBody()); $this->assertSame('text/html; charset=utf-8', $curlResult->getContentType()); $this->assertSame('https://test.local', $curlResult->getUrl()); @@ -80,6 +81,7 @@ class CurlResultTest extends TestCase public function testRedirect() { $header = file_get_contents(__DIR__ . '/../../datasets/curl/about.head'); + $headerArray = include(__DIR__ . '/../../datasets/curl/about.head.php'); $body = file_get_contents(__DIR__ . '/../../datasets/curl/about.body'); @@ -93,7 +95,7 @@ class CurlResultTest extends TestCase $this->assertTrue($curlResult->isSuccess()); $this->assertFalse($curlResult->isTimeout()); $this->assertTrue($curlResult->isRedirectUrl()); - $this->assertSame($header, $curlResult->getHeader()); + $this->assertSame($headerArray, $curlResult->getHeaders()); $this->assertSame($body, $curlResult->getBody()); $this->assertSame('text/html; charset=utf-8', $curlResult->getContentType()); $this->assertSame('https://test.local/test/it', $curlResult->getUrl()); @@ -106,6 +108,7 @@ class CurlResultTest extends TestCase public function testTimeout() { $header = file_get_contents(__DIR__ . '/../../datasets/curl/about.head'); + $headerArray = include(__DIR__ . '/../../datasets/curl/about.head.php'); $body = file_get_contents(__DIR__ . '/../../datasets/curl/about.body'); @@ -119,7 +122,7 @@ class CurlResultTest extends TestCase $this->assertFalse($curlResult->isSuccess()); $this->assertTrue($curlResult->isTimeout()); $this->assertFalse($curlResult->isRedirectUrl()); - $this->assertSame($header, $curlResult->getHeader()); + $this->assertSame($headerArray, $curlResult->getHeaders()); $this->assertSame($body, $curlResult->getBody()); $this->assertSame('text/html; charset=utf-8', $curlResult->getContentType()); $this->assertSame('https://test.local/test/it', $curlResult->getRedirectUrl()); @@ -134,6 +137,7 @@ class CurlResultTest extends TestCase public function testRedirectHeader() { $header = file_get_contents(__DIR__ . '/../../datasets/curl/about.redirect'); + $headerArray = include(__DIR__ . '/../../datasets/curl/about.redirect.php'); $body = file_get_contents(__DIR__ . '/../../datasets/curl/about.body'); @@ -146,7 +150,7 @@ class CurlResultTest extends TestCase $this->assertTrue($curlResult->isSuccess()); $this->assertFalse($curlResult->isTimeout()); $this->assertTrue($curlResult->isRedirectUrl()); - $this->assertSame($header, $curlResult->getHeader()); + $this->assertSame($headerArray, $curlResult->getHeaders()); $this->assertSame($body, $curlResult->getBody()); $this->assertSame('text/html; charset=utf-8', $curlResult->getContentType()); $this->assertSame('https://test.local/test/it?key=value', $curlResult->getUrl()); @@ -204,7 +208,7 @@ class CurlResultTest extends TestCase 'url' => 'https://test.local' ]); - $this->assertNotEmpty($curlResult->getHeader()); + $this->assertNotEmpty($curlResult->getHeaders()); $this->assertEmpty($curlResult->getHeader('wrongHeader')); } } From 64004cf1c30ee4310d6e049605f2a67e97ed641d Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 9 Oct 2020 19:35:47 +0200 Subject: [PATCH 06/15] Update lock --- composer.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.lock b/composer.lock index 0d650ad155..02a31227ae 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "aa279ba22954af464cd7b4a5e2d74eec", + "content-hash": "6c141820d2160b278dffecc3e89f86e3", "packages": [ { "name": "asika/simple-console", From 1a42f35a3cd857e53504e811569be306072139a2 Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 9 Oct 2020 19:59:24 +0200 Subject: [PATCH 07/15] Use CurlResult for failed HTTPRequests (legacy usage) --- src/Network/HTTPRequest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Network/HTTPRequest.php b/src/Network/HTTPRequest.php index 798b0cdea8..e0400b6b94 100644 --- a/src/Network/HTTPRequest.php +++ b/src/Network/HTTPRequest.php @@ -195,7 +195,7 @@ class HTTPRequest implements IHTTPRequest if ($exception->hasResponse()) { return new GuzzleResponse($exception->getResponse(), $url, $exception->getCode(), $exception->getMessage()); } else { - return new GuzzleResponse(null, $url, $exception->getCode(), $exception->getMessage()); + return new CurlResult($url, '', ['http_code' => $exception->getCode()], $exception->getCode(), $exception->getMessage()); } } finally { $this->profiler->saveTimestamp($stamp1, 'network'); From f3cd973cbe9d41be6e5bb7c36dad96c716f2f4b3 Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 9 Oct 2020 20:52:52 +0200 Subject: [PATCH 08/15] HTTPRequest: Replace getInfo() with new parameter 'content_length' --- src/Network/CurlResult.php | 6 ------ src/Network/GuzzleResponse.php | 5 ----- src/Network/HTTPRequest.php | 14 ++++++++++++-- src/Network/IHTTPRequest.php | 1 + src/Network/IHTTPResult.php | 5 ----- src/Network/Probe.php | 7 +------ src/Util/ParseUrl.php | 7 +------ 7 files changed, 15 insertions(+), 30 deletions(-) diff --git a/src/Network/CurlResult.php b/src/Network/CurlResult.php index b0e801010d..b2eefcbaa9 100644 --- a/src/Network/CurlResult.php +++ b/src/Network/CurlResult.php @@ -319,12 +319,6 @@ class CurlResult implements IHTTPResult return $this->body; } - /** {@inheritDoc} */ - public function getInfo() - { - return $this->info; - } - /** {@inheritDoc} */ public function isRedirectUrl() { diff --git a/src/Network/GuzzleResponse.php b/src/Network/GuzzleResponse.php index c6cab7c9e8..69e88b4025 100644 --- a/src/Network/GuzzleResponse.php +++ b/src/Network/GuzzleResponse.php @@ -121,11 +121,6 @@ class GuzzleResponse extends Response implements IHTTPResult, ResponseInterface return $this->url; } - public function getInfo() - { - // TODO: Implement getInfo() method. - } - /** {@inheritDoc} */ public function isRedirectUrl() { diff --git a/src/Network/HTTPRequest.php b/src/Network/HTTPRequest.php index e0400b6b94..3e34c01e2a 100644 --- a/src/Network/HTTPRequest.php +++ b/src/Network/HTTPRequest.php @@ -30,6 +30,7 @@ use Friendica\Util\Network; use Friendica\Util\Profiler; use GuzzleHttp\Client; use GuzzleHttp\Exception\RequestException; +use GuzzleHttp\Exception\TransferException; use Psr\Http\Message\RequestInterface; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\UriInterface; @@ -177,10 +178,18 @@ class HTTPRequest implements IHTTPRequest $this->logger->notice('Curl redirect.', ['url' => $request->getUri(), 'to' => $uri]); }; + $onHeaders = function (ResponseInterface $response) use ($opts) { + if (!empty($opts['content_length']) && + $response->getHeaderLine('Content-Length') > $opts['content_length']) { + throw new TransferException('The file is too big!'); + } + }; + $client = new Client([ 'allow_redirect' => [ 'max' => 8, 'on_redirect' => $onRedirect, + 'on_headers' => $onHeaders, 'track_redirect' => true, 'strict' => true, 'referer' => true, @@ -191,8 +200,9 @@ class HTTPRequest implements IHTTPRequest try { $response = $client->get($url); return new GuzzleResponse($response, $url); - } catch (RequestException $exception) { - if ($exception->hasResponse()) { + } catch (TransferException $exception) { + if ($exception instanceof RequestException && + $exception->hasResponse()) { return new GuzzleResponse($exception->getResponse(), $url, $exception->getCode(), $exception->getMessage()); } else { return new CurlResult($url, '', ['http_code' => $exception->getCode()], $exception->getCode(), $exception->getMessage()); diff --git a/src/Network/IHTTPRequest.php b/src/Network/IHTTPRequest.php index 3f9b7f27a1..49993085e6 100644 --- a/src/Network/IHTTPRequest.php +++ b/src/Network/IHTTPRequest.php @@ -75,6 +75,7 @@ interface IHTTPRequest * 'nobody' => only return the header * 'cookiejar' => path to cookie jar file * 'header' => header array + * 'content_length' => int maximum File content length * * @return IHTTPResult */ diff --git a/src/Network/IHTTPResult.php b/src/Network/IHTTPResult.php index 5904bcfa38..77ee86976b 100644 --- a/src/Network/IHTTPResult.php +++ b/src/Network/IHTTPResult.php @@ -82,11 +82,6 @@ interface IHTTPResult */ public function getBody(); - /** - * @return array - */ - public function getInfo(); - /** * @return boolean */ diff --git a/src/Network/Probe.php b/src/Network/Probe.php index 3fe035286f..cfd0368439 100644 --- a/src/Network/Probe.php +++ b/src/Network/Probe.php @@ -423,16 +423,11 @@ class Probe */ private static function getHideStatus($url) { - $curlResult = DI::httpRequest()->get($url); + $curlResult = DI::httpRequest()->get($url, false, ['content_length' => 1000000]); if (!$curlResult->isSuccess()) { return false; } - // If the file is too large then exit - if (($curlResult->getInfo()['download_content_length'] ?? 0) > 1000000) { - return false; - } - // If it isn't a HTML file then exit if (($curlResult->getContentType() != '') && !strstr(strtolower($curlResult->getContentType()), 'html')) { return false; diff --git a/src/Util/ParseUrl.php b/src/Util/ParseUrl.php index c3cbda70e3..1a81a256ca 100644 --- a/src/Util/ParseUrl.php +++ b/src/Util/ParseUrl.php @@ -160,16 +160,11 @@ class ParseUrl return $siteinfo; } - $curlResult = DI::httpRequest()->get($url); + $curlResult = DI::httpRequest()->get($url, false, ['content_length' => 1000000]); if (!$curlResult->isSuccess()) { return $siteinfo; } - // If the file is too large then exit - if (($curlResult->getInfo()['download_content_length'] ?? 0) > 1000000) { - return $siteinfo; - } - // If it isn't a HTML file then exit if (($curlResult->getContentType() != '') && !strstr(strtolower($curlResult->getContentType()), 'html')) { return $siteinfo; From 80bd0a4d5a4f63accb4786d37dc488c8141d0cc3 Mon Sep 17 00:00:00 2001 From: Philipp Date: Fri, 9 Oct 2020 22:28:41 +0200 Subject: [PATCH 09/15] Fix IHTTPResult::getHeader() - Now returns a string array, like expected - Fix usages - Fix dataset --- src/Content/Text/BBCode.php | 4 +-- src/Model/Photo.php | 5 ++-- src/Model/User.php | 6 ++--- src/Network/CurlResult.php | 10 +++++--- src/Network/IHTTPResult.php | 4 +-- src/Network/Probe.php | 2 +- src/Protocol/OStatus.php | 4 +-- src/Util/Images.php | 18 ++++++++------ src/Util/ParseUrl.php | 8 +++--- tests/datasets/curl/about.head.php | 32 ++++++++++++------------ tests/datasets/curl/about.redirect.php | 34 +++++++++++++------------- 11 files changed, 68 insertions(+), 59 deletions(-) diff --git a/src/Content/Text/BBCode.php b/src/Content/Text/BBCode.php index 594cd9a6ce..5256c3c8ae 100644 --- a/src/Content/Text/BBCode.php +++ b/src/Content/Text/BBCode.php @@ -491,8 +491,8 @@ class BBCode } $i = $curlResult->getBody(); - $type = $curlResult->getContentType(); - $type = Images::getMimeTypeByData($i, $mtch[1], $type); + $contType = $curlResult->getContentType(); + $type = Images::getMimeTypeByData($i, $mtch[1], $contType); if ($i) { $Image = new Image($i, $type); diff --git a/src/Model/Photo.php b/src/Model/Photo.php index 6380f42789..dc60f25f37 100644 --- a/src/Model/Photo.php +++ b/src/Model/Photo.php @@ -424,16 +424,17 @@ class Photo if (!empty($image_url)) { $ret = DI::httpRequest()->get($image_url, true); $img_str = $ret->getBody(); - $type = $ret->getContentType(); + $contType = $ret->getContentType(); } else { $img_str = ''; + $contType = []; } if ($quit_on_error && ($img_str == "")) { return false; } - $type = Images::getMimeTypeByData($img_str, $image_url, $type); + $type = Images::getMimeTypeByData($img_str, $image_url, $contType); $Image = new Image($img_str, $type); if ($Image->isValid()) { diff --git a/src/Model/User.php b/src/Model/User.php index 68c42e40e0..48d9348961 100644 --- a/src/Model/User.php +++ b/src/Model/User.php @@ -1005,13 +1005,13 @@ class User $curlResult = DI::httpRequest()->get($photo, true); if ($curlResult->isSuccess()) { $img_str = $curlResult->getBody(); - $type = $curlResult->getContentType(); + $contType = $curlResult->getContentType(); } else { $img_str = ''; - $type = ''; + $contType = []; } - $type = Images::getMimeTypeByData($img_str, $photo, $type); + $type = Images::getMimeTypeByData($img_str, $photo, $contType); $Image = new Image($img_str, $type); if ($Image->isValid()) { diff --git a/src/Network/CurlResult.php b/src/Network/CurlResult.php index b2eefcbaa9..4ecdd68d10 100644 --- a/src/Network/CurlResult.php +++ b/src/Network/CurlResult.php @@ -245,7 +245,7 @@ class CurlResult implements IHTTPResult public function getHeader($header) { if (empty($header)) { - return ''; + return []; } $header = strtolower(trim($header)); @@ -256,7 +256,7 @@ class CurlResult implements IHTTPResult return $headers[$header]; } - return ''; + return []; } /** {@inheritDoc} */ @@ -289,7 +289,11 @@ class CurlResult implements IHTTPResult $parts = explode(':', $line); $headerfield = strtolower(trim(array_shift($parts))); $headerdata = trim(implode(':', $parts)); - $this->header_fields[$headerfield] = $headerdata; + if (empty($this->header_fields[$headerfield])) { + $this->header_fields[$headerfield] = [$headerdata]; + } elseif (!in_array($headerdata, $this->header_fields[$headerfield])) { + $this->header_fields[$headerfield][] = $headerdata; + } } return $this->header_fields; diff --git a/src/Network/IHTTPResult.php b/src/Network/IHTTPResult.php index 77ee86976b..acee2dde98 100644 --- a/src/Network/IHTTPResult.php +++ b/src/Network/IHTTPResult.php @@ -19,7 +19,7 @@ interface IHTTPResult /** * Returns the Content Type * - * @return string the Content Type + * @return string[] the Content Types */ public function getContentType(); @@ -29,7 +29,7 @@ interface IHTTPResult * * @param string $header optional header field. Return all fields if empty * - * @return string the headers or the specified content of the header variable + * @return string[] the headers or the specified content of the header variable */ public function getHeader($header); diff --git a/src/Network/Probe.php b/src/Network/Probe.php index cfd0368439..14dcdea336 100644 --- a/src/Network/Probe.php +++ b/src/Network/Probe.php @@ -429,7 +429,7 @@ class Probe } // If it isn't a HTML file then exit - if (($curlResult->getContentType() != '') && !strstr(strtolower($curlResult->getContentType()), 'html')) { + if (!in_array('html', $curlResult->getContentType())) { return false; } diff --git a/src/Protocol/OStatus.php b/src/Protocol/OStatus.php index ef38aaebe3..1d95a6f3a4 100644 --- a/src/Protocol/OStatus.php +++ b/src/Protocol/OStatus.php @@ -747,7 +747,7 @@ class OStatus $xml = ''; if ($curlResult->inHeader('Content-Type') && - stristr($curlResult->getHeader('Content-Type'), 'application/atom+xml')) { + in_array('application/atom+xml', $curlResult->getHeader('Content-Type'))) { $xml = $curlResult->getBody(); } @@ -941,7 +941,7 @@ class OStatus $xml = ''; if ($curlResult->inHeader('Content-Type') && - stristr($curlResult->getHeader('Content-Type'), 'application/atom+xml')) { + in_array('application/atom+xml', $curlResult->getHeader('Content-Type'))) { Logger::log('Directly fetched XML for URI ' . $related_uri, Logger::DEBUG); $xml = $curlResult->getBody(); } diff --git a/src/Util/Images.php b/src/Util/Images.php index f39b0db00d..f0aefb9f2c 100644 --- a/src/Util/Images.php +++ b/src/Util/Images.php @@ -75,23 +75,25 @@ class Images /** * Fetch image mimetype from the image data or guessing from the file name * - * @param string $image_data Image data - * @param string $filename File name (for guessing the type via the extension) - * @param string $mime default mime type + * @param string $image_data Image data + * @param string $filename File name (for guessing the type via the extension) + * @param string[] $mimeTypes possible mime types * * @return string * @throws \Exception */ - public static function getMimeTypeByData(string $image_data, string $filename = '', string $mime = '') + public static function getMimeTypeByData(string $image_data, string $filename = '', array $mimeTypes = []) { - if (substr($mime, 0, 6) == 'image/') { - Logger::info('Using default mime type', ['filename' => $filename, 'mime' => $mime]); - return $mime; + foreach ($mimeTypes as $mimeType) { + if (substr($mimeType, 0, 6) == 'image/') { + Logger::info('Using default mime type', ['filename' => $filename, 'mime' => $mimeTypes]); + return $mimeType; + } } $image = @getimagesizefromstring($image_data); if (!empty($image['mime'])) { - Logger::info('Mime type detected via data', ['filename' => $filename, 'default' => $mime, 'mime' => $image['mime']]); + Logger::info('Mime type detected via data', ['filename' => $filename, 'default' => $mimeTypes, 'mime' => $image['mime']]); return $image['mime']; } diff --git a/src/Util/ParseUrl.php b/src/Util/ParseUrl.php index 1a81a256ca..ee2bedd1ee 100644 --- a/src/Util/ParseUrl.php +++ b/src/Util/ParseUrl.php @@ -166,7 +166,7 @@ class ParseUrl } // If it isn't a HTML file then exit - if (($curlResult->getContentType() != '') && !strstr(strtolower($curlResult->getContentType()), 'html')) { + if (!in_array('html', $curlResult->getContentType())) { return $siteinfo; } @@ -198,8 +198,10 @@ class ParseUrl $charset = ''; // Look for a charset, first in headers // Expected form: Content-Type: text/html; charset=ISO-8859-4 - if (preg_match('/charset=([a-z0-9-_.\/]+)/i', $curlResult->getContentType(), $matches)) { - $charset = trim(trim(trim(array_pop($matches)), ';,')); + foreach ($curlResult->getContentType() as $type) { + if (preg_match('/charset=([a-z0-9-_.\/]+)/i', $type, $matches)) { + $charset = trim(trim(trim(array_pop($matches)), ';,')); + } } // Then in body that gets precedence diff --git a/tests/datasets/curl/about.head.php b/tests/datasets/curl/about.head.php index d0be0feb4f..b7773b81af 100644 --- a/tests/datasets/curl/about.head.php +++ b/tests/datasets/curl/about.head.php @@ -1,20 +1,20 @@ '', - 'date' => 'Thu, 11 Oct 2018 18:43:54 GMT', - 'content-type' => 'text/html; charset=utf-8', - 'vary' => 'Accept-Encoding', - 'server' => 'Mastodon', - 'x-frame-options' => 'SAMEORIGIN', - 'x-content-type-options' => 'nosniff', - 'x-xss-protection' => '1; mode=block', - 'etag' => 'W/"706e6c48957e1d46ecf9d7597a7880af"', - 'cache-control' => 'max-age=0, private, must-revalidate', - 'set-cookie' => '_mastodon_session=v3kcy%2FW3aZYBBvZUohuwksEKwzYIyEUlEuJ1KqTAfWPKvVQq%2F4UuJ39zp621VyfpQNlvY46TL%2FYutzXowSLYQBNFCJcrEiF04aU0TdtHls9zynMiyeHhoVgCijOXWXNt9%2FCmpQ49RkNEujkv9NaJ0cum32MCVZKjE9%2BMKmLM%2F8ZygZeLBGJ7sg%3D%3D--QGIiU0%2FpXc3Aym8F--he2iRRPePOdtEs3z%2BufSXg%3D%3D; path=/; secure; HttpOnly', - 'x-request-id' => 'a0c0b8e7-cd60-4efa-b79b-cf1b0d5a0784', - 'x-runtime' => '0.049566', - 'strict-transport-security' => 'max-age=31536000; includeSubDomains; preload', - 'referrer-policy' => 'same-origin', - 'content-security-policy' => "frame-ancestors 'none'; script-src 'self'; object-src 'self'; img-src * data: blob:; media-src 'self' data:; font-src 'self' data: https://fonts.gstatic.com/; connect-src 'self' blob: wss://mastodonten.de", + 'http/2 200' => [''], + 'date' => ['Thu, 11 Oct 2018 18:43:54 GMT'], + 'content-type' => ['text/html; charset=utf-8'], + 'vary' => ['Accept-Encoding'], + 'server' => ['Mastodon'], + 'x-frame-options' => ['DENY', 'SAMEORIGIN'], + 'x-content-type-options' => ['nosniff'], + 'x-xss-protection' => ['1; mode=block'], + 'etag' => ['W/"706e6c48957e1d46ecf9d7597a7880af"'], + 'cache-control' => ['max-age=0, private, must-revalidate'], + 'set-cookie' => ['_mastodon_session=v3kcy%2FW3aZYBBvZUohuwksEKwzYIyEUlEuJ1KqTAfWPKvVQq%2F4UuJ39zp621VyfpQNlvY46TL%2FYutzXowSLYQBNFCJcrEiF04aU0TdtHls9zynMiyeHhoVgCijOXWXNt9%2FCmpQ49RkNEujkv9NaJ0cum32MCVZKjE9%2BMKmLM%2F8ZygZeLBGJ7sg%3D%3D--QGIiU0%2FpXc3Aym8F--he2iRRPePOdtEs3z%2BufSXg%3D%3D; path=/; secure; HttpOnly'], + 'x-request-id' => ['a0c0b8e7-cd60-4efa-b79b-cf1b0d5a0784'], + 'x-runtime' => ['0.049566'], + 'strict-transport-security' => ['max-age=31536000; includeSubDomains; preload'], + 'referrer-policy' => ['same-origin'], + 'content-security-policy' => ["frame-ancestors 'none'; script-src 'self'; object-src 'self'; img-src * data: blob:; media-src 'self' data:; font-src 'self' data: https://fonts.gstatic.com/; connect-src 'self' blob: wss://mastodonten.de"], ]; diff --git a/tests/datasets/curl/about.redirect.php b/tests/datasets/curl/about.redirect.php index 5ae3fd88f7..f01689aadc 100644 --- a/tests/datasets/curl/about.redirect.php +++ b/tests/datasets/curl/about.redirect.php @@ -1,21 +1,21 @@ '', - 'date' => 'Thu, 11 Oct 2018 18:43:54 GMT', - 'content-type' => 'text/html; charset=utf-8', - 'vary' => 'Accept-Encoding', - 'server' => 'Mastodon', - 'location' => 'https://test.other/some/', - 'x-frame-options' => 'SAMEORIGIN', - 'x-content-type-options' => 'nosniff', - 'x-xss-protection' => '1; mode=block', - 'etag' => 'W/"706e6c48957e1d46ecf9d7597a7880af"', - 'cache-control' => 'max-age=0, private, must-revalidate', - 'set-cookie' => '_mastodon_session=v3kcy%2FW3aZYBBvZUohuwksEKwzYIyEUlEuJ1KqTAfWPKvVQq%2F4UuJ39zp621VyfpQNlvY46TL%2FYutzXowSLYQBNFCJcrEiF04aU0TdtHls9zynMiyeHhoVgCijOXWXNt9%2FCmpQ49RkNEujkv9NaJ0cum32MCVZKjE9%2BMKmLM%2F8ZygZeLBGJ7sg%3D%3D--QGIiU0%2FpXc3Aym8F--he2iRRPePOdtEs3z%2BufSXg%3D%3D; path=/; secure; HttpOnly', - 'x-request-id' => 'a0c0b8e7-cd60-4efa-b79b-cf1b0d5a0784', - 'x-runtime' => '0.049566', - 'strict-transport-security' => 'max-age=31536000; includeSubDomains; preload', - 'referrer-policy' => 'same-origin', - 'content-security-policy' => "frame-ancestors 'none'; script-src 'self'; object-src 'self'; img-src * data: blob:; media-src 'self' data:; font-src 'self' data: https://fonts.gstatic.com/; connect-src 'self' blob: wss://mastodonten.de", + 'http/2 301' => [''], + 'date' => ['Thu, 11 Oct 2018 18:43:54 GMT'], + 'content-type' => ['text/html; charset=utf-8'], + 'vary' => ['Accept-Encoding'], + 'server' => ['Mastodon'], + 'location' => ['https://test.other/some/'], + 'x-frame-options' => ['DENY', 'SAMEORIGIN'], + 'x-content-type-options' => ['nosniff'], + 'x-xss-protection' => ['1; mode=block'], + 'etag' => ['W/"706e6c48957e1d46ecf9d7597a7880af"'], + 'cache-control' => ['max-age=0, private, must-revalidate'], + 'set-cookie' => ['_mastodon_session=v3kcy%2FW3aZYBBvZUohuwksEKwzYIyEUlEuJ1KqTAfWPKvVQq%2F4UuJ39zp621VyfpQNlvY46TL%2FYutzXowSLYQBNFCJcrEiF04aU0TdtHls9zynMiyeHhoVgCijOXWXNt9%2FCmpQ49RkNEujkv9NaJ0cum32MCVZKjE9%2BMKmLM%2F8ZygZeLBGJ7sg%3D%3D--QGIiU0%2FpXc3Aym8F--he2iRRPePOdtEs3z%2BufSXg%3D%3D; path=/; secure; HttpOnly'], + 'x-request-id' => ['a0c0b8e7-cd60-4efa-b79b-cf1b0d5a0784'], + 'x-runtime' => ['0.049566'], + 'strict-transport-security' => ['max-age=31536000; includeSubDomains; preload'], + 'referrer-policy' => ['same-origin'], + 'content-security-policy' => ["frame-ancestors 'none'; script-src 'self'; object-src 'self'; img-src * data: blob:; media-src 'self' data:; font-src 'self' data: https://fonts.gstatic.com/; connect-src 'self' blob: wss://mastodonten.de"], ]; From 02bc99f67ba416455201ee6c249669fcbcca4e17 Mon Sep 17 00:00:00 2001 From: Philipp Date: Sat, 10 Oct 2020 18:58:45 +0200 Subject: [PATCH 10/15] Fix Content-Type for `CurlResult` class --- src/Network/CurlResult.php | 6 +++--- tests/src/Network/CurlResultTest.php | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Network/CurlResult.php b/src/Network/CurlResult.php index 4ecdd68d10..ca1ead91d1 100644 --- a/src/Network/CurlResult.php +++ b/src/Network/CurlResult.php @@ -37,7 +37,7 @@ class CurlResult implements IHTTPResult private $returnCode; /** - * @var string the content type of the Curl call + * @var string[] the content type of the Curl call */ private $contentType; @@ -223,9 +223,9 @@ class CurlResult implements IHTTPResult private function checkInfo() { if (isset($this->info['content_type'])) { - $this->contentType = $this->info['content_type']; + $this->contentType = [$this->info['content_type']]; } else { - $this->contentType = ''; + $this->contentType = []; } } diff --git a/tests/src/Network/CurlResultTest.php b/tests/src/Network/CurlResultTest.php index 03a5288124..4bc066b800 100644 --- a/tests/src/Network/CurlResultTest.php +++ b/tests/src/Network/CurlResultTest.php @@ -68,7 +68,7 @@ class CurlResultTest extends TestCase $this->assertFalse($curlResult->isRedirectUrl()); $this->assertSame($headerArray, $curlResult->getHeaders()); $this->assertSame($body, $curlResult->getBody()); - $this->assertSame('text/html; charset=utf-8', $curlResult->getContentType()); + $this->assertSame(['text/html; charset=utf-8'], $curlResult->getContentType()); $this->assertSame('https://test.local', $curlResult->getUrl()); $this->assertSame('https://test.local', $curlResult->getRedirectUrl()); } @@ -97,7 +97,7 @@ class CurlResultTest extends TestCase $this->assertTrue($curlResult->isRedirectUrl()); $this->assertSame($headerArray, $curlResult->getHeaders()); $this->assertSame($body, $curlResult->getBody()); - $this->assertSame('text/html; charset=utf-8', $curlResult->getContentType()); + $this->assertSame(['text/html; charset=utf-8'], $curlResult->getContentType()); $this->assertSame('https://test.local/test/it', $curlResult->getUrl()); $this->assertSame('https://test.other/test/it', $curlResult->getRedirectUrl()); } @@ -124,7 +124,7 @@ class CurlResultTest extends TestCase $this->assertFalse($curlResult->isRedirectUrl()); $this->assertSame($headerArray, $curlResult->getHeaders()); $this->assertSame($body, $curlResult->getBody()); - $this->assertSame('text/html; charset=utf-8', $curlResult->getContentType()); + $this->assertSame(['text/html; charset=utf-8'], $curlResult->getContentType()); $this->assertSame('https://test.local/test/it', $curlResult->getRedirectUrl()); $this->assertSame('Tested error', $curlResult->getError()); } @@ -152,7 +152,7 @@ class CurlResultTest extends TestCase $this->assertTrue($curlResult->isRedirectUrl()); $this->assertSame($headerArray, $curlResult->getHeaders()); $this->assertSame($body, $curlResult->getBody()); - $this->assertSame('text/html; charset=utf-8', $curlResult->getContentType()); + $this->assertSame(['text/html; charset=utf-8'], $curlResult->getContentType()); $this->assertSame('https://test.local/test/it?key=value', $curlResult->getUrl()); $this->assertSame('https://test.other/some/?key=value', $curlResult->getRedirectUrl()); } From 40b11442c2eefa104e4a229f48abad11998e1b49 Mon Sep 17 00:00:00 2001 From: Philipp Date: Sat, 10 Oct 2020 21:41:22 +0200 Subject: [PATCH 11/15] IHTTPResult::getContentType is now a string again --- src/Model/Photo.php | 2 +- src/Model/User.php | 2 +- src/Network/CurlResult.php | 6 +++--- src/Network/GuzzleResponse.php | 2 +- src/Network/IHTTPResult.php | 2 +- src/Network/Probe.php | 2 +- src/Util/Images.php | 18 ++++++++---------- src/Util/ParseUrl.php | 8 +++----- tests/src/Network/CurlResultTest.php | 8 ++++---- 9 files changed, 23 insertions(+), 27 deletions(-) diff --git a/src/Model/Photo.php b/src/Model/Photo.php index dc60f25f37..5f8551fa27 100644 --- a/src/Model/Photo.php +++ b/src/Model/Photo.php @@ -427,7 +427,7 @@ class Photo $contType = $ret->getContentType(); } else { $img_str = ''; - $contType = []; + $contType = ''; } if ($quit_on_error && ($img_str == "")) { diff --git a/src/Model/User.php b/src/Model/User.php index 48d9348961..e6f20f7387 100644 --- a/src/Model/User.php +++ b/src/Model/User.php @@ -1008,7 +1008,7 @@ class User $contType = $curlResult->getContentType(); } else { $img_str = ''; - $contType = []; + $contType = ''; } $type = Images::getMimeTypeByData($img_str, $photo, $contType); diff --git a/src/Network/CurlResult.php b/src/Network/CurlResult.php index ca1ead91d1..4ecdd68d10 100644 --- a/src/Network/CurlResult.php +++ b/src/Network/CurlResult.php @@ -37,7 +37,7 @@ class CurlResult implements IHTTPResult private $returnCode; /** - * @var string[] the content type of the Curl call + * @var string the content type of the Curl call */ private $contentType; @@ -223,9 +223,9 @@ class CurlResult implements IHTTPResult private function checkInfo() { if (isset($this->info['content_type'])) { - $this->contentType = [$this->info['content_type']]; + $this->contentType = $this->info['content_type']; } else { - $this->contentType = []; + $this->contentType = ''; } } diff --git a/src/Network/GuzzleResponse.php b/src/Network/GuzzleResponse.php index 69e88b4025..72d87ae9fe 100644 --- a/src/Network/GuzzleResponse.php +++ b/src/Network/GuzzleResponse.php @@ -88,7 +88,7 @@ class GuzzleResponse extends Response implements IHTTPResult, ResponseInterface /** {@inheritDoc} */ public function getContentType() { - return $this->getHeader('Content-Type'); + return implode($this->getHeader('Content-Type')); } /** {@inheritDoc} */ diff --git a/src/Network/IHTTPResult.php b/src/Network/IHTTPResult.php index acee2dde98..38a1176284 100644 --- a/src/Network/IHTTPResult.php +++ b/src/Network/IHTTPResult.php @@ -19,7 +19,7 @@ interface IHTTPResult /** * Returns the Content Type * - * @return string[] the Content Types + * @return string the Content Type */ public function getContentType(); diff --git a/src/Network/Probe.php b/src/Network/Probe.php index 14dcdea336..cfd0368439 100644 --- a/src/Network/Probe.php +++ b/src/Network/Probe.php @@ -429,7 +429,7 @@ class Probe } // If it isn't a HTML file then exit - if (!in_array('html', $curlResult->getContentType())) { + if (($curlResult->getContentType() != '') && !strstr(strtolower($curlResult->getContentType()), 'html')) { return false; } diff --git a/src/Util/Images.php b/src/Util/Images.php index f0aefb9f2c..e0375da3d7 100644 --- a/src/Util/Images.php +++ b/src/Util/Images.php @@ -75,25 +75,23 @@ class Images /** * Fetch image mimetype from the image data or guessing from the file name * - * @param string $image_data Image data - * @param string $filename File name (for guessing the type via the extension) - * @param string[] $mimeTypes possible mime types + * @param string $image_data Image data + * @param string $filename File name (for guessing the type via the extension) + * @param string $mimeTypes possible mime type * * @return string * @throws \Exception */ - public static function getMimeTypeByData(string $image_data, string $filename = '', array $mimeTypes = []) + public static function getMimeTypeByData(string $image_data, string $filename = '', string $mimeType = '') { - foreach ($mimeTypes as $mimeType) { - if (substr($mimeType, 0, 6) == 'image/') { - Logger::info('Using default mime type', ['filename' => $filename, 'mime' => $mimeTypes]); - return $mimeType; - } + if (substr($mimeType, 0, 6) == 'image/') { + Logger::info('Using default mime type', ['filename' => $filename, 'mime' => $mimeType]); + return $mimeType; } $image = @getimagesizefromstring($image_data); if (!empty($image['mime'])) { - Logger::info('Mime type detected via data', ['filename' => $filename, 'default' => $mimeTypes, 'mime' => $image['mime']]); + Logger::info('Mime type detected via data', ['filename' => $filename, 'default' => $mimeType, 'mime' => $image['mime']]); return $image['mime']; } diff --git a/src/Util/ParseUrl.php b/src/Util/ParseUrl.php index ee2bedd1ee..1a81a256ca 100644 --- a/src/Util/ParseUrl.php +++ b/src/Util/ParseUrl.php @@ -166,7 +166,7 @@ class ParseUrl } // If it isn't a HTML file then exit - if (!in_array('html', $curlResult->getContentType())) { + if (($curlResult->getContentType() != '') && !strstr(strtolower($curlResult->getContentType()), 'html')) { return $siteinfo; } @@ -198,10 +198,8 @@ class ParseUrl $charset = ''; // Look for a charset, first in headers // Expected form: Content-Type: text/html; charset=ISO-8859-4 - foreach ($curlResult->getContentType() as $type) { - if (preg_match('/charset=([a-z0-9-_.\/]+)/i', $type, $matches)) { - $charset = trim(trim(trim(array_pop($matches)), ';,')); - } + if (preg_match('/charset=([a-z0-9-_.\/]+)/i', $curlResult->getContentType(), $matches)) { + $charset = trim(trim(trim(array_pop($matches)), ';,')); } // Then in body that gets precedence diff --git a/tests/src/Network/CurlResultTest.php b/tests/src/Network/CurlResultTest.php index 4bc066b800..03a5288124 100644 --- a/tests/src/Network/CurlResultTest.php +++ b/tests/src/Network/CurlResultTest.php @@ -68,7 +68,7 @@ class CurlResultTest extends TestCase $this->assertFalse($curlResult->isRedirectUrl()); $this->assertSame($headerArray, $curlResult->getHeaders()); $this->assertSame($body, $curlResult->getBody()); - $this->assertSame(['text/html; charset=utf-8'], $curlResult->getContentType()); + $this->assertSame('text/html; charset=utf-8', $curlResult->getContentType()); $this->assertSame('https://test.local', $curlResult->getUrl()); $this->assertSame('https://test.local', $curlResult->getRedirectUrl()); } @@ -97,7 +97,7 @@ class CurlResultTest extends TestCase $this->assertTrue($curlResult->isRedirectUrl()); $this->assertSame($headerArray, $curlResult->getHeaders()); $this->assertSame($body, $curlResult->getBody()); - $this->assertSame(['text/html; charset=utf-8'], $curlResult->getContentType()); + $this->assertSame('text/html; charset=utf-8', $curlResult->getContentType()); $this->assertSame('https://test.local/test/it', $curlResult->getUrl()); $this->assertSame('https://test.other/test/it', $curlResult->getRedirectUrl()); } @@ -124,7 +124,7 @@ class CurlResultTest extends TestCase $this->assertFalse($curlResult->isRedirectUrl()); $this->assertSame($headerArray, $curlResult->getHeaders()); $this->assertSame($body, $curlResult->getBody()); - $this->assertSame(['text/html; charset=utf-8'], $curlResult->getContentType()); + $this->assertSame('text/html; charset=utf-8', $curlResult->getContentType()); $this->assertSame('https://test.local/test/it', $curlResult->getRedirectUrl()); $this->assertSame('Tested error', $curlResult->getError()); } @@ -152,7 +152,7 @@ class CurlResultTest extends TestCase $this->assertTrue($curlResult->isRedirectUrl()); $this->assertSame($headerArray, $curlResult->getHeaders()); $this->assertSame($body, $curlResult->getBody()); - $this->assertSame(['text/html; charset=utf-8'], $curlResult->getContentType()); + $this->assertSame('text/html; charset=utf-8', $curlResult->getContentType()); $this->assertSame('https://test.local/test/it?key=value', $curlResult->getUrl()); $this->assertSame('https://test.other/some/?key=value', $curlResult->getRedirectUrl()); } From b8314f0c301987b56dea540430b8268b862013c5 Mon Sep 17 00:00:00 2001 From: Philipp Date: Sat, 10 Oct 2020 21:47:47 +0200 Subject: [PATCH 12/15] Fix redirect logging --- src/Network/HTTPRequest.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Network/HTTPRequest.php b/src/Network/HTTPRequest.php index 3e34c01e2a..68cadee93d 100644 --- a/src/Network/HTTPRequest.php +++ b/src/Network/HTTPRequest.php @@ -170,12 +170,14 @@ class HTTPRequest implements IHTTPRequest $curlOptions[CURLOPT_BINARYTRANSFER] = 1; } + $logger = $this->logger; + $onRedirect = function( RequestInterface $request, ResponseInterface $response, UriInterface $uri - ) { - $this->logger->notice('Curl redirect.', ['url' => $request->getUri(), 'to' => $uri]); + ) use ($logger) { + $logger->notice('Curl redirect.', ['url' => $request->getUri(), 'to' => $uri]); }; $onHeaders = function (ResponseInterface $response) use ($opts) { From 8c7185154dfe4748758926c5fd958479a612feff Mon Sep 17 00:00:00 2001 From: Philipp Date: Sat, 10 Oct 2020 21:50:36 +0200 Subject: [PATCH 13/15] Remove unnecessary exception message (avoid log flooding) --- src/Network/HTTPRequest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Network/HTTPRequest.php b/src/Network/HTTPRequest.php index 68cadee93d..82eec80432 100644 --- a/src/Network/HTTPRequest.php +++ b/src/Network/HTTPRequest.php @@ -205,9 +205,9 @@ class HTTPRequest implements IHTTPRequest } catch (TransferException $exception) { if ($exception instanceof RequestException && $exception->hasResponse()) { - return new GuzzleResponse($exception->getResponse(), $url, $exception->getCode(), $exception->getMessage()); + return new GuzzleResponse($exception->getResponse(), $url, $exception->getCode(), ''); } else { - return new CurlResult($url, '', ['http_code' => $exception->getCode()], $exception->getCode(), $exception->getMessage()); + return new CurlResult($url, '', ['http_code' => $exception->getCode()], $exception->getCode(), ''); } } finally { $this->profiler->saveTimestamp($stamp1, 'network'); From e17befb7d604adfdb811551db456bddc57b8f923 Mon Sep 17 00:00:00 2001 From: Philipp Date: Sat, 10 Oct 2020 23:10:35 +0200 Subject: [PATCH 14/15] Use last entry for Content-Type --- src/Network/GuzzleResponse.php | 8 +++++++- src/Util/Images.php | 2 +- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Network/GuzzleResponse.php b/src/Network/GuzzleResponse.php index 72d87ae9fe..18155d1d29 100644 --- a/src/Network/GuzzleResponse.php +++ b/src/Network/GuzzleResponse.php @@ -88,7 +88,13 @@ class GuzzleResponse extends Response implements IHTTPResult, ResponseInterface /** {@inheritDoc} */ public function getContentType() { - return implode($this->getHeader('Content-Type')); + $contentTypes = $this->getHeader('Content-Type') ?? []; + $countTypes = count($contentTypes); + if ($countTypes > 0) { + return $contentTypes[$countTypes - 1]; + } else { + return ''; + } } /** {@inheritDoc} */ diff --git a/src/Util/Images.php b/src/Util/Images.php index e0375da3d7..1b52b91a13 100644 --- a/src/Util/Images.php +++ b/src/Util/Images.php @@ -77,7 +77,7 @@ class Images * * @param string $image_data Image data * @param string $filename File name (for guessing the type via the extension) - * @param string $mimeTypes possible mime type + * @param string $mimeType possible mime type * * @return string * @throws \Exception From 79e667b3d10c05e39dd081704ce54485f42ba095 Mon Sep 17 00:00:00 2001 From: Philipp Date: Sun, 11 Oct 2020 00:33:36 +0200 Subject: [PATCH 15/15] Update src/Network/GuzzleResponse.php Co-authored-by: Hypolite Petovan --- src/Network/GuzzleResponse.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Network/GuzzleResponse.php b/src/Network/GuzzleResponse.php index 18155d1d29..b1bbaa9653 100644 --- a/src/Network/GuzzleResponse.php +++ b/src/Network/GuzzleResponse.php @@ -89,12 +89,7 @@ class GuzzleResponse extends Response implements IHTTPResult, ResponseInterface public function getContentType() { $contentTypes = $this->getHeader('Content-Type') ?? []; - $countTypes = count($contentTypes); - if ($countTypes > 0) { - return $contentTypes[$countTypes - 1]; - } else { - return ''; - } + return array_pop($contentTypes) ?? ''; } /** {@inheritDoc} */