Consistently resolve base URL according to HTTP specs

Author: clue

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
@@ -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

@@ -99,41 +99,4 @@ 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);
-    }
 }

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',