Skip to content

Commit

Permalink
Do not modify config.url when using a relative baseURL (resolves #1628)…
Browse files Browse the repository at this point in the history
… (#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
  • Loading branch information
multicolaure authored and felipewmartins committed Sep 5, 2019
1 parent 98e4acd commit 6fe506f
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 10 deletions.
4 changes: 3 additions & 1 deletion lib/adapters/http.js
Expand Up @@ -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');
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 4 additions & 2 deletions lib/adapters/xhr.js
Expand Up @@ -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');
Expand All @@ -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;
Expand Down Expand Up @@ -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;

Expand Down
20 changes: 20 additions & 0 deletions 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;
};
7 changes: 0 additions & 7 deletions lib/core/dispatchRequest.js
Expand Up @@ -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.
Expand All @@ -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 || {};

Expand Down
20 changes: 20 additions & 0 deletions 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');
});

});
24 changes: 24 additions & 0 deletions test/specs/requests.spec.js
Expand Up @@ -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';
Expand Down
14 changes: 14 additions & 0 deletions test/unit/adapters/http.js
Expand Up @@ -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();
});
});
});
});


0 comments on commit 6fe506f

Please sign in to comment.