Deprecate Network::unparseURL in favor of UriInterfact objects

- Added specific test for Probe::cleanURI

diff --git a/src/Network/Probe.php b/src/Network/Probe.php
index d8c1a91530e23cae1b7dcdf19b1e534763cc86ce..baae0d3d310d989bd2faa99e9772ab49a2e3fcb7 100644 (file)
--- a/src/Network/Probe.php
+++ b/src/Network/Probe.php
@@ -44,6 +44,7 @@ use Friendica\Util\DateTimeFormat;
 use Friendica\Util\Network;
 use Friendica\Util\Strings;
 use Friendica\Util\XML;
+use GuzzleHttp\Psr7\Uri;
 
 /**
  * This class contain functions for probing URL
@@ -58,26 +59,23 @@ class Probe
        /**
         * Remove stuff from an URI that doesn't belong there
         *
-        * @param string $URI
+        * @param string $rawUri
         * @return string Cleaned URI
         */
-       public static function cleanURI(string $URI)
+       public static function cleanURI(string $rawUri): string
        {
                // At first remove leading and trailing junk
-               $URI = trim($URI, "@#?:/ \t\n\r\0\x0B");
+               $rawUri = trim($rawUri, "@#?:/ \t\n\r\0\x0B");
 
-               $parts = parse_url($URI);
-
-               if (empty($parts['scheme'])) {
-                       return $URI;
+               $uri = new Uri($rawUri);
+               if (!$uri->getScheme()) {
+                       return $uri->__toString();
                }
 
                // Remove the URL fragment, since these shouldn't be part of any profile URL
-               unset($parts['fragment']);
-
-               $URI = Network::unparseURL($parts);
+               $uri = $uri->withFragment('');
 
-               return $URI;
+               return $uri->__toString();
        }
 
        /**

diff --git a/src/Util/Network.php b/src/Util/Network.php
index 2631fc75b09094d84b2547d448d6696e5df2e3c3..0d98cab2ed67a6095faa45d187724f3a28d69344 100644 (file)
--- a/src/Util/Network.php
+++ b/src/Util/Network.php
@@ -435,7 +435,8 @@ class Network
         *
         * @param array $parsed URL parts
         *
-        * @return string The glued URL
+        * @return string The glued URL.
+        * @deprecated since version 2021.12, use a UriInterface object like GuzzleHttp\Psr7\Uri instead
         */
        public static function unparseURL(array $parsed)
        {

diff --git a/tests/src/Network/ProbeTest.php b/tests/src/Network/ProbeTest.php
index 79c323adc202bbcd539078358d1ae8fe38c454d5..3b6b2e3a10578bddb723328b20429e893da06cb3 100644 (file)
--- a/tests/src/Network/ProbeTest.php
+++ b/tests/src/Network/ProbeTest.php
@@ -117,12 +117,48 @@ class ProbeTest extends FixtureTest
                }
        }
 
-       public function dataUri()
+       public function dataCleanUri(): array
        {
                return [
                        '@-first' => [
-                               'uri'         => '@Artists4Future_Muenchen@climatejustice.global',
-                               'assertUri'   => 'Artists4Future_Muenchen@climatejustice.global',
+                               'expected' => 'Artists4Future_Muenchen@climatejustice.global',
+                               'uri'      => '@Artists4Future_Muenchen@climatejustice.global',
+                       ],
+                       'no-scheme-no-fragment' => [
+                               'expected' => 'example.com/path?arg=value',
+                               'uri'      => 'example.com/path?arg=value',
+                       ],
+                       /* This case makes little sense, both in our expectation of receiving it in any context and in the way we
+                        * do not change it in Probe::cleanUri, but it doesn't seem to be the source of any terrible security hole.
+                        */
+                       'no-scheme-fragment' => [
+                               'expected' => 'example.com/path?arg=value#fragment',
+                               'uri'      => 'example.com/path?arg=value#fragment',
+                       ],
+                       'scheme-no-fragment' => [
+                               'expected' => 'https://example.com/path?arg=value',
+                               'uri'      => 'https://example.com/path?arg=value#fragment',
+                       ],
+                       'scheme-fragment' => [
+                               'expected' => 'https://example.com/path?arg=value',
+                               'uri'      => 'https://example.com/path?arg=value#fragment',
+                       ],
+               ];
+       }
+
+       /**
+        * @dataProvider dataCleanUri
+        */
+       public function testCleanUri(string $expected, string $uri)
+       {
+               self::assertEquals($expected, Probe::cleanURI($uri));
+       }
+
+       public function dataUri(): array
+       {
+               return [
+                       'Artists4Future_Muenchen@climatejustice.global' => [
+                               'uri'         => 'Artists4Future_Muenchen@climatejustice.global',
                                'assertInfos' => [
                                        'name'         => 'Artists4Future München',
                                        'nick'         => 'Artists4Future_Muenchen',
@@ -163,7 +199,7 @@ xQIDAQAB
        /**
         * @dataProvider dataUri
         */
-       public function testCleanUri(string $uri, string $assertUri, array $assertInfos)
+       public function testProbeUri(string $uri, array $assertInfos)
        {
                self::markTestIncomplete('hard work due mocking 19 different http-requests');
 
@@ -216,10 +252,7 @@ xQIDAQAB
 
                $this->httpRequestHandler->push($history);
 
-               $cleaned = Probe::cleanURI($uri);
-               self::assertEquals($assertUri, $cleaned);
-               self::assertArraySubset($assertInfos, Probe::uri($cleaned, '', 0));
-
+               self::assertArraySubset($assertInfos, Probe::uri($uri, '', 0));
 
                // Iterate over the requests and responses
                foreach ($container as $transaction) {