Do not modify config.url when using a relative baseURL (resolves #1628) (#2391)

* Adding tests to show config.url mutation

Because config.url is modified while processing the request
when the baseURL is set,
it is impossible to perform a retry with the provided config object.

Ref #1628

* Fixing url combining without modifying config.url

As config.url is not modified anymore during the request processing.
The request can safely be retried after it failed with the provided
config.

resolves #1628

lib/adapters/http.js

@@ -2,6 +2,7 @@
 
 var utils = require('./../utils');
 var settle = require('./../core/settle');
+var buildFullPath = require('../core/buildFullPath');
 var buildURL = require('./../helpers/buildURL');
 var http = require('http');
 var https = require('https');
@@ -64,7 +65,8 @@ module.exports = function httpAdapter(config) {
     }
 
     // Parse url
-    var parsed = url.parse(config.url);
+    var fullPath = buildFullPath(config.baseURL, config.url);
+    var parsed = url.parse(fullPath);
     var protocol = parsed.protocol || 'http:';
 
     if (!auth && parsed.auth) {

lib/adapters/xhr.js

@@ -3,6 +3,7 @@
 var utils = require('./../utils');
 var settle = require('./../core/settle');
 var buildURL = require('./../helpers/buildURL');
+var buildFullPath = require('../core/buildFullPath');
 var parseHeaders = require('./../helpers/parseHeaders');
 var isURLSameOrigin = require('./../helpers/isURLSameOrigin');
 var createError = require('../core/createError');
@@ -25,7 +26,8 @@ module.exports = function xhrAdapter(config) {
       requestHeaders.Authorization = 'Basic ' + btoa(username + ':' + password);
     }
 
-    request.open(config.method.toUpperCase(), buildURL(config.url, config.params, config.paramsSerializer), true);
+    var fullPath = buildFullPath(config.baseURL, config.url);
+    request.open(config.method.toUpperCase(), buildURL(fullPath, config.params, config.paramsSerializer), true);
 
     // Set the request timeout in MS
     request.timeout = config.timeout;
@@ -100,7 +102,7 @@ module.exports = function xhrAdapter(config) {
       var cookies = require('./../helpers/cookies');
 
       // Add xsrf header
-      var xsrfValue = (config.withCredentials || isURLSameOrigin(config.url)) && config.xsrfCookieName ?
+      var xsrfValue = (config.withCredentials || isURLSameOrigin(fullPath)) && config.xsrfCookieName ?
         cookies.read(config.xsrfCookieName) :
         undefined;
 

lib/core/buildFullPath.js

@@ -0,0 +1,20 @@
+'use strict';
+
+var isAbsoluteURL = require('../helpers/isAbsoluteURL');
+var combineURLs = require('../helpers/combineURLs');
+
+/**
+ * Creates a new URL by combining the baseURL with the requestedURL,
+ * only when the requestedURL is not already an absolute URL.
+ * If the requestURL is absolute, this function returns the requestedURL untouched.
+ *
+ * @param {string} baseURL The base URL
+ * @param {string} requestedURL Absolute or relative URL to combine
+ * @returns {string} The combined full path
+ */
+module.exports = function buildFullPath(baseURL, requestedURL) {
+  if (baseURL && !isAbsoluteURL(requestedURL)) {
+    return combineURLs(baseURL, requestedURL);
+  }
+  return requestedURL;
+};

lib/core/dispatchRequest.js

@@ -4,8 +4,6 @@ var utils = require('./../utils');
 var transformData = require('./transformData');
 var isCancel = require('../cancel/isCancel');
 var defaults = require('../defaults');
-var isAbsoluteURL = require('./../helpers/isAbsoluteURL');
-var combineURLs = require('./../helpers/combineURLs');
 
 /**
  * Throws a `Cancel` if cancellation has been requested.
@@ -25,11 +23,6 @@ function throwIfCancellationRequested(config) {
 module.exports = function dispatchRequest(config) {
   throwIfCancellationRequested(config);
 
-  // Support baseURL config
-  if (config.baseURL && !isAbsoluteURL(config.url)) {
-    config.url = combineURLs(config.baseURL, config.url);
-  }
-
   // Ensure headers exist
   config.headers = config.headers || {};
 

test/specs/core/buildFullPath.spec.js

@@ -0,0 +1,20 @@
+var buildFullPath = require('../../../lib/core/buildFullPath');
+
+describe('helpers::buildFullPath', function () {
+  it('should combine URLs when the requestedURL is relative', function () {
+    expect(buildFullPath('https://api.github.com', '/users')).toBe('https://api.github.com/users');
+  });
+
+  it('should return the requestedURL when it is absolute', function () {
+    expect(buildFullPath('https://api.github.com', 'https://api.example.com/users')).toBe('https://api.example.com/users');
+  });
+
+  it('should not combine URLs when the baseURL is not configured', function () {
+    expect(buildFullPath(undefined, '/users')).toBe('/users');
+  });
+
+  it('should combine URLs when the baseURL and requestedURL are relative', function () {
+    expect(buildFullPath('/api', '/users')).toBe('/api/users');
+  });
+
+});

test/specs/requests.spec.js

@@ -245,6 +245,30 @@ describe('requests', function () {
     });
   });
 
+  it('should not modify the config url with relative baseURL', function (done) {
+    var config;
+
+    axios.get('/foo', {
+        baseURL: '/api'
+    }).catch(function (error) {
+        config = error.config;
+    });
+
+    getAjaxRequest().then(function (request) {
+      request.respondWith({
+        status: 404,
+        statusText: 'NOT FOUND',
+        responseText: 'Resource not found'
+      });
+
+      setTimeout(function () {
+        expect(config.baseURL).toEqual('/api');
+        expect(config.url).toEqual('/foo');
+        done();
+      }, 100);
+    });
+  });
+
   it('should allow overriding Content-Type header case-insensitive', function (done) {
     var response;
     var contentType = 'application/vnd.myapp.type+json';

test/unit/adapters/http.js

@@ -646,6 +646,20 @@ describe('supports http with nodejs', function () {
       });
     });
   });
+
+  it('should combine baseURL and url', function (done) {
+    server = http.createServer(function (req, res) {
+      res.end();
+    }).listen(4444, function () {
+      axios.get('/foo', {
+        baseURL: 'http://localhost:4444/',
+      }).then(function (res) {
+        assert.equal(res.config.baseURL, 'http://localhost:4444/');
+        assert.equal(res.config.url, '/foo');
+        done();
+      });
+    });
+  });
 });