Consistently resolve base URL according to HTTP specs

README.md

@@ -2282,16 +2282,16 @@ The `withBase(string|null $baseUrl): Browser` method can be used to
 change the base URL used to resolve relative URLs to.
 
 If you configure a base URL, any requests to relative URLs will be
-processed by first prepending this absolute base URL. Note that this
-merely prepends the base URL and does *not* resolve any relative path
-references (like `../` etc.). This is mostly useful for (RESTful) API
-calls where all endpoints (URLs) are located under a common base URL.
+processed by first resolving this relative to the given absolute base
+URL. This supports resolving relative path references (like `../` etc.).
+This is particularly useful for (RESTful) API calls where all endpoints
+(URLs) are located under a common base URL.
 
 ```php
-$browser = $browser->withBase('http://api.example.com/v3');
+$browser = $browser->withBase('http://api.example.com/v3/');
 
-// will request http://api.example.com/v3/example
-$browser->get('/example')->then(…);
+// will request http://api.example.com/v3/users
+$browser->get('users')->then(…);
 ```
 
 You can pass in a `null` base URL to return a new instance that does not

src/Browser.php

@@ -3,6 +3,7 @@
 namespace React\Http;
 
 use Psr\Http\Message\ResponseInterface;
+use RingCentral\Psr7\Uri;
 use React\EventLoop\LoopInterface;
 use React\Http\Io\MessageFactory;
 use React\Http\Io\Sender;
@@ -542,16 +543,16 @@ public function withRejectErrorResponse($obeySuccessCode)
      * Changes the base URL used to resolve relative URLs to.
      *
      * If you configure a base URL, any requests to relative URLs will be
-     * processed by first prepending this absolute base URL. Note that this
-     * merely prepends the base URL and does *not* resolve any relative path
-     * references (like `../` etc.). This is mostly useful for (RESTful) API
-     * calls where all endpoints (URLs) are located under a common base URL.
+     * processed by first resolving this relative to the given absolute base
+     * URL. This supports resolving relative path references (like `../` etc.).
+     * This is particularly useful for (RESTful) API calls where all endpoints
+     * (URLs) are located under a common base URL.
      *
      * ```php
-     * $browser = $browser->withBase('http://api.example.com/v3');
+     * $browser = $browser->withBase('http://api.example.com/v3/');
      *
-     * // will request http://api.example.com/v3/example
-     * $browser->get('/example')->then(…);
+     * // will request http://api.example.com/v3/users
+     * $browser->get('users')->then(…);
      * ```
      *
      * You can pass in a `null` base URL to return a new instance that does not
@@ -584,7 +585,7 @@ public function withBase($baseUrl)
             return $browser;
         }
 
-        $browser->baseUrl = $this->messageFactory->uri($baseUrl);
+        $browser->baseUrl = new Uri($baseUrl);
         if (!\in_array($browser->baseUrl->getScheme(), array('http', 'https')) || $browser->baseUrl->getHost() === '') {
             throw new \InvalidArgumentException('Base URL must be absolute');
         }
@@ -725,12 +726,13 @@ private function withOptions(array $options)
      */
     private function requestMayBeStreaming($method, $url, array $headers = array(), $contents = '')
     {
-        $request = $this->messageFactory->request($method, $url, $headers, $contents, $this->protocolVersion);
         if ($this->baseUrl !== null) {
             // ensure we're actually below the base URL
-            $request = $request->withUri($this->messageFactory->expandBase($request->getUri(), $this->baseUrl));
+            $url = Uri::resolve($this->baseUrl, $url);
         }
 
+        $request = $this->messageFactory->request($method, $url, $headers, $contents, $this->protocolVersion);
+
         return $this->transaction->send($request);
     }
 }

src/Io/MessageFactory.php

@@ -6,7 +6,6 @@
 use Psr\Http\Message\UriInterface;
 use RingCentral\Psr7\Request;
 use RingCentral\Psr7\Response;
-use RingCentral\Psr7\Uri;
 use React\Stream\ReadableStreamInterface;
 
 /**
@@ -76,64 +75,4 @@ public function body($body)
 
         return \RingCentral\Psr7\stream_for($body);
     }
-
-    /**
-     * Creates a new instance of UriInterface for the given URI string
-     *
-     * @param string $uri
-     * @return UriInterface
-     */
-    public function uri($uri)
-    {
-        return new Uri($uri);
-    }
-
-    /**
-     * Creates a new instance of UriInterface for the given URI string relative to the given base URI
-     *
-     * @param UriInterface $base
-     * @param string       $uri
-     * @return UriInterface
-     */
-    public function uriRelative(UriInterface $base, $uri)
-    {
-        return Uri::resolve($base, $uri);
-    }
-
-    /**
-     * Resolves the given relative or absolute $uri by appending it behind $this base URI
-     *
-     * The given $uri parameter can be either a relative or absolute URI and
-     * as such can not contain any URI template placeholders.
-     *
-     * As such, the outcome of this method represents a valid, absolute URI
-     * which will be returned as an instance implementing `UriInterface`.
-     *
-     * If the given $uri is a relative URI, it will simply be appended behind $base URI.
-     *
-     * If the given $uri is an absolute URI, it will simply be returned as-is.
-     *
-     * @param UriInterface $uri
-     * @param UriInterface $base
-     * @return UriInterface
-     */
-    public function expandBase(UriInterface $uri, UriInterface $base)
-    {
-        if ($uri->getScheme() !== '') {
-            return $uri;
-        }
-
-        $uri = (string)$uri;
-        $base = (string)$base;
-
-        if ($uri !== '' && substr($base, -1) !== '/' && substr($uri, 0, 1) !== '?') {
-            $base .= '/';
-        }
-
-        if (isset($uri[0]) && $uri[0] === '/') {
-            $uri = substr($uri, 1);
-        }
-
-        return $this->uri($base . $uri);
-    }
 }

src/Io/Transaction.php

@@ -5,6 +5,7 @@
 use Psr\Http\Message\RequestInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\UriInterface;
+use RingCentral\Psr7\Uri;
 use React\EventLoop\LoopInterface;
 use React\Http\Message\ResponseException;
 use React\Promise\Deferred;
@@ -246,7 +247,7 @@ public function onResponse(ResponseInterface $response, RequestInterface $reques
     private function onResponseRedirect(ResponseInterface $response, RequestInterface $request, Deferred $deferred)
     {
         // resolve location relative to last request URI
-        $location = $this->messageFactory->uriRelative($request->getUri(), $response->getHeaderLine('Location'));
+        $location = Uri::resolve($request->getUri(), $response->getHeaderLine('Location'));
 
         $request = $this->makeRedirectRequest($request, $location);
         $this->progress('redirect', array($request));

tests/BrowserTest.php

@@ -223,35 +223,30 @@ public function provideOtherUris()
                 'http://example.com/base/another',
                 'http://example.com/base/another',
             ),
-            'slash returns added slash' => array(
+            'slash returns base without path' => array(
                 'http://example.com/base',
                 '/',
-                'http://example.com/base/',
-            ),
-            'slash does not add duplicate slash if base already ends with slash' => array(
-                'http://example.com/base/',
-                '/',
-                'http://example.com/base/',
+                'http://example.com/',
             ),
             'relative is added behind base' => array(
                 'http://example.com/base/',
                 'test',
                 'http://example.com/base/test',
             ),
-            'relative with slash is added behind base without duplicate slashes' => array(
-                'http://example.com/base/',
-                '/test',
-                'http://example.com/base/test',
-            ),
-            'relative is added behind base with automatic slash inbetween' => array(
+            'relative is added behind base without path' => array(
                 'http://example.com/base',
                 'test',
-                'http://example.com/base/test',
+                'http://example.com/test',
             ),
-            'relative with slash is added behind base' => array(
+            'relative level up is added behind parent path' => array(
+                'http://example.com/base/foo/',
+                '../bar',
+                'http://example.com/base/bar',
+            ),
+            'absolute with slash is added behind base without path' => array(
                 'http://example.com/base',
                 '/test',
-                'http://example.com/base/test',
+                'http://example.com/test',
             ),
             'query string is added behind base' => array(
                 'http://example.com/base',
@@ -263,10 +258,10 @@ public function provideOtherUris()
                 '?key=value',
                 'http://example.com/base/?key=value',
             ),
-            'query string with slash is added behind base' => array(
+            'query string with slash is added behind base without path' => array(
                 'http://example.com/base',
                 '/?key=value',
-                'http://example.com/base/?key=value',
+                'http://example.com/?key=value',
             ),
             'absolute with query string below base is returned as-is' => array(
                 'http://example.com/base',

tests/Io/MessageFactoryTest.php

@@ -17,68 +17,6 @@ public function setUpMessageFactory()
         $this->messageFactory = new MessageFactory();
     }
 
-    public function testUriSimple()
-    {
-        $uri = $this->messageFactory->uri('http://www.lueck.tv/');
-
-        $this->assertEquals('http', $uri->getScheme());
-        $this->assertEquals('www.lueck.tv', $uri->getHost());
-        $this->assertEquals('/', $uri->getPath());
-
-        $this->assertEquals(null, $uri->getPort());
-        $this->assertEquals('', $uri->getQuery());
-    }
-
-    public function testUriComplete()
-    {
-        $uri = $this->messageFactory->uri('https://example.com:8080/?just=testing');
-
-        $this->assertEquals('https', $uri->getScheme());
-        $this->assertEquals('example.com', $uri->getHost());
-        $this->assertEquals(8080, $uri->getPort());
-        $this->assertEquals('/', $uri->getPath());
-        $this->assertEquals('just=testing', $uri->getQuery());
-    }
-
-    public function testPlaceholdersInUriWillBeEscaped()
-    {
-        $uri = $this->messageFactory->uri('http://example.com/{version}');
-
-        $this->assertEquals('/%7Bversion%7D', $uri->getPath());
-    }
-
-    public function testEscapedPlaceholdersInUriWillStayEscaped()
-    {
-        $uri = $this->messageFactory->uri('http://example.com/%7Bversion%7D');
-
-        $this->assertEquals('/%7Bversion%7D', $uri->getPath());
-    }
-
-    public function testResolveRelative()
-    {
-        $base = $this->messageFactory->uri('http://example.com/base/');
-
-        $this->assertEquals('http://example.com/base/', $this->messageFactory->uriRelative($base, ''));
-        $this->assertEquals('http://example.com/', $this->messageFactory->uriRelative($base, '/'));
-
-        $this->assertEquals('http://example.com/base/a', $this->messageFactory->uriRelative($base, 'a'));
-        $this->assertEquals('http://example.com/a', $this->messageFactory->uriRelative($base, '../a'));
-    }
-
-    public function testResolveAbsolute()
-    {
-        $base = $this->messageFactory->uri('http://example.org/');
-
-        $this->assertEquals('http://www.example.com/', $this->messageFactory->uriRelative($base, 'http://www.example.com/'));
-    }
-
-    public function testResolveUri()
-    {
-        $base = $this->messageFactory->uri('http://example.org/');
-
-        $this->assertEquals('http://www.example.com/', $this->messageFactory->uriRelative($base, $this->messageFactory->uri('http://www.example.com/')));
-    }
-
     public function testBodyString()
     {
         $body = $this->messageFactory->body('hi');