Skip to content

Commit

Permalink
Fixing url combining without modifying config.url
Browse files Browse the repository at this point in the history
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 committed Sep 5, 2019
1 parent 958512c commit aece92f
Show file tree
Hide file tree
Showing 5 changed files with 47 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');
});

});

0 comments on commit aece92f

Please sign in to comment.