Skip to content

Commit

Permalink
Do not modify config.url when using a relative baseURL (resolves axio…
Browse files Browse the repository at this point in the history
…s#1628) (axios#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 axios#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 axios#1628
  • Loading branch information
multicolaure authored and genie-youn committed Sep 27, 2019
1 parent 98e4acd commit ba66bd0
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 ba66bd0

Please sign in to comment.