[TASK] Deprecate alternative fetch methods for GeneralUtility::getUrl() 12/64012/7
authorBenni Mack <benni@typo3.org>
Mon, 30 Mar 2020 13:06:00 +0000 (15:06 +0200)
committerAndreas Fernandez <a.fernandez@scripting-base.de>
Mon, 6 Apr 2020 19:28:22 +0000 (21:28 +0200)
In times of PSR-7, TYPO3's RequestFactory API and the underlying
Guzzle HTTP library, the wrapper method GeneralUtility::getUrl() is merely
a lazy short-hand method for fetching the contents.

It is recommended to use TYPO3's RequestFactory API for fetching remote
URLs.

For now, however, the additional arguments (sending alternative headers,
only fetching HEAD requests and return headers or headers + content,
fetching a report of what happened) are deprecated, as they have
proven to be inflexible nowadays.

Resolves: #90956
Releases: master
Change-Id: Ic7631ccf61000d0853007cf9278db6e1bab09335
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/64012
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Georg Ringer <georg.ringer@gmail.com>
Tested-by: Andreas Fernandez <a.fernandez@scripting-base.de>
Reviewed-by: Georg Ringer <georg.ringer@gmail.com>
Reviewed-by: Andreas Fernandez <a.fernandez@scripting-base.de>
typo3/sysext/core/Classes/Error/PageErrorHandler/PageContentErrorHandler.php
typo3/sysext/core/Classes/Utility/GeneralUtility.php
typo3/sysext/core/Documentation/Changelog/master/Deprecation-90956-AlternativeFetchMethodsAndReportsForGeneralUtilitygetUrl.rst [new file with mode: 0644]
typo3/sysext/core/Tests/Unit/Utility/GeneralUtilityTest.php
typo3/sysext/core/Tests/UnitDeprecated/Utility/GeneralUtilityTest.php
typo3/sysext/indexed_search/Classes/Indexer.php
typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodArgumentDroppedStaticMatcher.php

index 3bf2e87..b2160da 100644 (file)
@@ -20,6 +20,7 @@ use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use TYPO3\CMS\Core\Exception\SiteNotFoundException;
 use TYPO3\CMS\Core\Http\HtmlResponse;
+use TYPO3\CMS\Core\Http\RequestFactory;
 use TYPO3\CMS\Core\LinkHandling\LinkService;
 use TYPO3\CMS\Core\Routing\InvalidRouteArgumentsException;
 use TYPO3\CMS\Core\Site\Entity\Site;
@@ -71,14 +72,18 @@ class PageContentErrorHandler implements PageErrorHandlerInterface
         try {
             $resolvedUrl = $this->resolveUrl($request, $this->errorHandlerConfiguration['errorContentSource']);
             $content = null;
-            $report = [];
-
             if ($resolvedUrl !== (string)$request->getUri()) {
-                $content = GeneralUtility::getUrl($resolvedUrl, 0, null, $report);
-                if ($content === false && ((int)$report['error'] === -1 || (int)$report['error'] > 200)) {
-                    throw new \RuntimeException('Error handler could not fetch error page "' . $resolvedUrl . '", reason: ' . $report['message'], 1544172838);
+                try {
+                    $response = GeneralUtility::makeInstance(RequestFactory::class)->request($resolvedUrl, 'GET');
+                } catch (\Exception $e) {
+                    throw new \RuntimeException('Error handler could not fetch error page "' . $resolvedUrl . '", reason: ' . $e->getMessage(), 1544172838);
+                }
+                if ($response->getStatusCode() >= 300) {
+                    throw new \RuntimeException('Error handler could not fetch error page "' . $resolvedUrl . '", status code: ' . $response->getStatusCode(), 1544172839);
                 }
+                return $response->withStatus($this->statusCode);
             }
+            $content = 'The error page could not be resolved, as the error page itself is not accessible';
         } catch (InvalidRouteArgumentsException | SiteNotFoundException $e) {
             $content = 'Invalid error handler configuration: ' . $this->errorHandlerConfiguration['errorContentSource'];
         }
index d366510..9c6f0c1 100644 (file)
@@ -1712,13 +1712,16 @@ class GeneralUtility
      * If you are having trouble with proxies when reading URLs you can configure your way out of that with settings within $GLOBALS['TYPO3_CONF_VARS']['HTTP'].
      *
      * @param string $url File/URL to read
-     * @param int $includeHeader Whether the HTTP header should be fetched or not. 0=disable, 1=fetch header+content, 2=fetch header only
-     * @param array $requestHeaders HTTP headers to be used in the request
-     * @param array $report Error code/message and, if $includeHeader is 1, response meta data (HTTP status and content type)
+     * @param int $includeHeader Whether the HTTP header should be fetched or not. 0=disable, 1=fetch header+content, 2=fetch header only - deprecated and will be removed in TYPO3 v11.
+     * @param array $requestHeaders HTTP headers to be used in the request - deprecated and will be removed in TYPO3 v11.
+     * @param array $report Error code/message and, if $includeHeader is 1, response meta data (HTTP status and content type) - deprecated and will be removed in TYPO3 v11.
      * @return mixed The content from the resource given as input. FALSE if an error has occurred.
      */
     public static function getUrl($url, $includeHeader = 0, $requestHeaders = null, &$report = null)
     {
+        if (func_num_args() > 1) {
+            trigger_error('Calling GeneralUtility::getUrl() with more than one argument will not be supported anymore in TYPO3 v11.0. Use RequestFactory and PSR-7 Requests and Response objects to evaluate the results in detail. For local files, use file_get_contents directly.', E_USER_DEPRECATED);
+        }
         if (isset($report)) {
             $report['error'] = 0;
             $report['message'] = '';
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Deprecation-90956-AlternativeFetchMethodsAndReportsForGeneralUtilitygetUrl.rst b/typo3/sysext/core/Documentation/Changelog/master/Deprecation-90956-AlternativeFetchMethodsAndReportsForGeneralUtilitygetUrl.rst
new file mode 100644 (file)
index 0000000..b9d6a12
--- /dev/null
@@ -0,0 +1,73 @@
+.. include:: ../../Includes.txt
+
+========================================================================================
+Deprecation: #90956 - Alternative fetch methods and reports for GeneralUtility::getUrl()
+========================================================================================
+
+See :issue:`90956`
+
+Description
+===========
+
+The short-hand method :php:`GeneralUtility::getUrl()` provides a
+fast way to fetch the contents of a local file or remote URL.
+
+For Remote URLs, TYPO3 v8 provides a object-oriented (PSR-7 compatible) way by using the :php:`RequestFactory->request($url, $method)` API. Under the hood, the PHP library GuzzleHTTP is used,
+which evaluates what best option (e.g. curl library) should handle
+the download to TYPO3.
+
+In general, it is recommended for any third-party extension developer to use either PHP's native :php:`file_get_contents($file)` method or the `RequestFactory->request()` method to fetch a PSR-7 ResponseInterface object.
+
+The additional arguments in `GeneralUtility::getUrl()` which allowed
+to send headers to the content or just do a HEAD request, or find
+ reports on why the request did not succeed are deprecated.
+
+PHP's native Exception Handling and the response object give enough insights already to load the HTTP headers as well, or even do HTTP `POST` requests.
+
+
+Impact
+======
+
+Calling the method `GeneralUtility::getUrl()` with more than one
+method argument will trigger a deprecation warning.
+
+
+Affected Installations
+======================
+
+TYPO3 installations using a third-party extension with the method
+described above.
+
+
+Migration
+=========
+
+Depending on the use-case of using the additional method parameters,
+certain alternatives exist since TYPO3 v8 already:
+
+Fetching the headers (as array) from a HTTP response:
+
+.. code-block:: php
+
+   $response = GeneralUtility::makeInstance(RequestFactory::class)->request($url);
+   $allHeaders = $response->getHeaders();
+   // Also see $response->getHeader($headerName) and $response->getHeaderLine($headerName)
+
+Sending additional headers with the HTTP request:
+
+.. code-block:: php
+
+   $response = GeneralUtility::makeInstance(RequestFactory::class)->request($url, [$headers => ['accept' => 'application/json']);
+
+Finding additional information about the response:
+
+.. code-block:: php
+
+   $response = GeneralUtility::makeInstance(RequestFactory::class)->request($url, [$headers => ['accept' => 'application/json']);
+   if ($response->getStatusCode() >= 300) {
+      $content = $response->getReasonPhrase();
+   } else {
+      $content = $response->getBody()->getContents();
+   }
+
+.. index:: PHP-API, FullyScanned, ext:core
\ No newline at end of file
index 73b7ea9..cc2d97f 100644 (file)
@@ -18,13 +18,10 @@ use org\bovigo\vfs\vfsStream;
 use org\bovigo\vfs\vfsStreamDirectory;
 use org\bovigo\vfs\vfsStreamWrapper;
 use Prophecy\Argument;
-use Psr\Http\Message\ResponseInterface;
-use Psr\Http\Message\StreamInterface;
 use Psr\Log\LoggerInterface;
 use TYPO3\CMS\Core\Cache\CacheManager;
 use TYPO3\CMS\Core\Cache\Frontend\FrontendInterface;
 use TYPO3\CMS\Core\Core\Environment;
-use TYPO3\CMS\Core\Http\RequestFactory;
 use TYPO3\CMS\Core\Package\Package;
 use TYPO3\CMS\Core\Package\PackageManager;
 use TYPO3\CMS\Core\Tests\Unit\Utility\AccessibleProxies\ExtensionManagementUtilityAccessibleProxy;
@@ -4577,36 +4574,6 @@ class GeneralUtilityTest extends UnitTestCase
         self::assertSame($expected, $result['index']['vDEF']);
     }
 
-    public function splitHeaderLinesDataProvider(): array
-    {
-        return [
-            'multi-line headers' => [
-                ['Content-Type' => 'multipart/form-data; boundary=something', 'Content-Language' => 'de-DE, en-CA'],
-                ['Content-Type' => 'multipart/form-data; boundary=something', 'Content-Language' => 'de-DE, en-CA'],
-            ]
-        ];
-    }
-
-    /**
-     * @test
-     * @dataProvider splitHeaderLinesDataProvider
-     * @param array $headers
-     * @param array $expectedHeaders
-     */
-    public function splitHeaderLines(array $headers, array $expectedHeaders): void
-    {
-        $stream = $this->prophesize(StreamInterface::class);
-        $response = $this->prophesize(ResponseInterface::class);
-        $response->getBody()->willReturn($stream);
-        $requestFactory = $this->prophesize(RequestFactory::class);
-        $requestFactory->request(Argument::cetera())->willReturn($response);
-
-        GeneralUtility::addInstance(RequestFactory::class, $requestFactory->reveal());
-        GeneralUtility::getUrl('http://example.com', 0, $headers);
-
-        $requestFactory->request(Argument::any(), Argument::any(), ['headers' => $expectedHeaders])->shouldHaveBeenCalled();
-    }
-
     public function locationHeaderUrlDataProvider(): array
     {
         return [
index 9477411..0ea1a51 100644 (file)
@@ -14,6 +14,10 @@ namespace TYPO3\CMS\Core\Tests\UnitDeprecated\Utility;
  * The TYPO3 project - inspiring people to share!
  */
 
+use Prophecy\Argument;
+use Psr\Http\Message\ResponseInterface;
+use Psr\Http\Message\StreamInterface;
+use TYPO3\CMS\Core\Http\RequestFactory;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
 
@@ -217,4 +221,34 @@ class GeneralUtilityTest extends UnitTestCase
     {
         self::assertTrue(GeneralUtility::verifyFilenameAgainstDenyPattern($allowedFile));
     }
+
+    public function splitHeaderLinesDataProvider(): array
+    {
+        return [
+            'multi-line headers' => [
+                ['Content-Type' => 'multipart/form-data; boundary=something', 'Content-Language' => 'de-DE, en-CA'],
+                ['Content-Type' => 'multipart/form-data; boundary=something', 'Content-Language' => 'de-DE, en-CA'],
+            ]
+        ];
+    }
+
+    /**
+     * @test
+     * @dataProvider splitHeaderLinesDataProvider
+     * @param array $headers
+     * @param array $expectedHeaders
+     */
+    public function splitHeaderLines(array $headers, array $expectedHeaders): void
+    {
+        $stream = $this->prophesize(StreamInterface::class);
+        $response = $this->prophesize(ResponseInterface::class);
+        $response->getBody()->willReturn($stream);
+        $requestFactory = $this->prophesize(RequestFactory::class);
+        $requestFactory->request(Argument::cetera())->willReturn($response);
+
+        GeneralUtility::addInstance(RequestFactory::class, $requestFactory->reveal());
+        GeneralUtility::getUrl('http://example.com', 0, $headers);
+
+        $requestFactory->request(Argument::any(), Argument::any(), ['headers' => $expectedHeaders])->shouldHaveBeenCalled();
+    }
 }
index f955918..cd97341 100644 (file)
@@ -18,6 +18,7 @@ use TYPO3\CMS\Core\Configuration\ExtensionConfiguration;
 use TYPO3\CMS\Core\Core\Environment;
 use TYPO3\CMS\Core\Database\Connection;
 use TYPO3\CMS\Core\Database\ConnectionPool;
+use TYPO3\CMS\Core\Http\RequestFactory;
 use TYPO3\CMS\Core\TimeTracker\TimeTracker;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Core\Utility\MathUtility;
@@ -671,7 +672,7 @@ class Indexer
     {
         // Get headers:
         $urlHeaders = $this->getUrlHeaders($externalUrl);
-        if (stripos($urlHeaders['Content-Type'], 'text/html') !== false) {
+        if (is_array($urlHeaders) && stripos($urlHeaders['Content-Type'], 'text/html') !== false) {
             $content = ($this->indexExternalUrl_content = GeneralUtility::getUrl($externalUrl));
             if ((string)$content !== '') {
                 // Create temporary file:
@@ -695,20 +696,17 @@ class Indexer
      */
     public function getUrlHeaders($url)
     {
-        // Try to get the headers only
-        $content = GeneralUtility::getUrl($url, 2);
-        if ((string)$content !== '') {
-            // Compile headers:
-            $headers = GeneralUtility::trimExplode(LF, $content, true);
+        try {
+            $response = GeneralUtility::makeInstance(RequestFactory::class)->request($url, 'HEAD');
+            $headers = $response->getHeaders();
             $retVal = [];
-            foreach ($headers as $line) {
-                if (trim($line) === '') {
-                    break;
-                }
-                [$headKey, $headValue] = explode(':', $line, 2);
-                $retVal[$headKey] = $headValue;
+            foreach ($headers as $key => $value) {
+                $retVal[$key] = implode('', $value);
             }
             return $retVal;
+        } catch (\Exception $e) {
+            // fail silently if the HTTP request failed
+            return false;
         }
     }
 
index 950dffc..325906a 100644 (file)
@@ -65,4 +65,10 @@ return [
             'Deprecation-87332-AvoidRuntimeReflectionCallsInObjectAccess.rst',
         ],
     ],
+    'TYPO3\CMS\Core\Utility\GeneralUtility::getUrl' => [
+        'maximumNumberOfArguments' => 1,
+        'restFiles' => [
+            'Deprecation-90956-AlternativeFetchMethodsAndReportsForGeneralUtilitygetUrl.rst',
+        ],
+    ],
 ];