Skip to content

Refactor mergeConfig without utils.deepMerge (#2844) #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MitchLewis930 wants to merge 1 commit into
base: pr_023_before
Choose a base branch
from
Open

Refactor mergeConfig without utils.deepMerge (#2844) #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/core/Axios.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ Axios.prototype.getUri = function getUri(config) {
utils.forEach(['delete', 'get', 'head', 'options'], function forEachMethodNoData(method) {
/*eslint func-names:0*/
Axios.prototype[method] = function(url, config) {
return this.request(utils.merge(config || {}, {
return this.request(mergeConfig(config || {}, {
method: method,
url: url
}));
Expand All @@ -83,7 +83,7 @@ utils.forEach(['delete', 'get', 'head', 'options'], function forEachMethodNoData
utils.forEach(['post', 'put', 'patch'], function forEachMethodWithData(method) {
/*eslint func-names:0*/
Axios.prototype[method] = function(url, data, config) {
return this.request(utils.merge(config || {}, {
return this.request(mergeConfig(config || {}, {
method: method,
url: url,
data: data
Expand Down
7 changes: 0 additions & 7 deletions lib/core/dispatchRequest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,6 @@ module.exports = function dispatchRequest(config) {
}
);

// Remove header where value is null
Copy link

@cursor cursor bot Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Null-valued headers no longer removed before sending requests

High Severity

The code that filtered out headers with null values was removed from dispatchRequest.js. Previously, setting a header to null would remove it from the request (useful for overriding default headers). Now, null-valued headers are passed to XMLHttpRequest.setRequestHeader(), which stringifies null to the string "null" and sends it in the request. This breaks the documented pattern of using null to unset default headers and will cause unexpected header values in production requests.

Fix in Cursor Fix in Web

utils.forEach(config.headers, function deleteNullValueHeaders(value, key) {
if (value === null) {
delete config.headers[key];
}
});

var adapter = config.adapter || defaults.adapter;

return adapter(config).then(function onAdapterResolution(response) {
Expand Down
72 changes: 43 additions & 29 deletions lib/core/mergeConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,56 +18,70 @@ module.exports = function mergeConfig(config1, config2) {
var valueFromConfig2Keys = ['url', 'method', 'data'];
var mergeDeepPropertiesKeys = ['headers', 'auth', 'proxy', 'params'];
var defaultToConfig2Keys = [
'baseURL', 'url', 'transformRequest', 'transformResponse', 'paramsSerializer',
'timeout', 'withCredentials', 'adapter', 'responseType', 'xsrfCookieName',
'xsrfHeaderName', 'onUploadProgress', 'onDownloadProgress',
'maxContentLength', 'maxBodyLength', 'validateStatus', 'maxRedirects', 'httpAgent',
'baseURL', 'transformRequest', 'transformResponse', 'paramsSerializer',
'timeout', 'timeoutMessage', 'withCredentials', 'adapter', 'responseType', 'xsrfCookieName',
'xsrfHeaderName', 'onUploadProgress', 'onDownloadProgress', 'decompress',
'maxContentLength', 'maxBodyLength', 'maxRedirects', 'transport', 'httpAgent',
'httpsAgent', 'cancelToken', 'socketPath', 'responseEncoding'
];
var directMergeKeys = ['validateStatus'];

function getMergedValue(target, source) {
if (utils.isPlainObject(target) && utils.isPlainObject(source)) {
return utils.merge(target, source);
} else if (utils.isPlainObject(source)) {
return utils.merge({}, source);
} else if (utils.isArray(source)) {
return source.slice();
}
return source;
}

function mergeDeepProperties(prop) {
if (!utils.isUndefined(config2[prop])) {
config[prop] = getMergedValue(config1[prop], config2[prop]);
} else if (!utils.isUndefined(config1[prop])) {
config[prop] = getMergedValue(undefined, config1[prop]);
}
}

utils.forEach(valueFromConfig2Keys, function valueFromConfig2(prop) {
if (typeof config2[prop] !== 'undefined') {
config[prop] = config2[prop];
if (!utils.isUndefined(config2[prop])) {
config[prop] = getMergedValue(undefined, config2[prop]);
}
});

utils.forEach(mergeDeepPropertiesKeys, function mergeDeepProperties(prop) {
if (utils.isObject(config2[prop])) {
config[prop] = utils.deepMerge(config1[prop], config2[prop]);
} else if (typeof config2[prop] !== 'undefined') {
config[prop] = config2[prop];
} else if (utils.isObject(config1[prop])) {
config[prop] = utils.deepMerge(config1[prop]);
} else if (typeof config1[prop] !== 'undefined') {
config[prop] = config1[prop];
utils.forEach(mergeDeepPropertiesKeys, mergeDeepProperties);

utils.forEach(defaultToConfig2Keys, function defaultToConfig2(prop) {
if (!utils.isUndefined(config2[prop])) {
config[prop] = getMergedValue(undefined, config2[prop]);
} else if (!utils.isUndefined(config1[prop])) {
config[prop] = getMergedValue(undefined, config1[prop]);
}
});

utils.forEach(defaultToConfig2Keys, function defaultToConfig2(prop) {
if (typeof config2[prop] !== 'undefined') {
config[prop] = config2[prop];
} else if (typeof config1[prop] !== 'undefined') {
config[prop] = config1[prop];
utils.forEach(directMergeKeys, function merge(prop) {
if (prop in config2) {
config[prop] = getMergedValue(config1[prop], config2[prop]);
} else if (prop in config1) {
config[prop] = getMergedValue(undefined, config1[prop]);
}
});

var axiosKeys = valueFromConfig2Keys
.concat(mergeDeepPropertiesKeys)
.concat(defaultToConfig2Keys);
.concat(defaultToConfig2Keys)
.concat(directMergeKeys);

var otherKeys = Object
.keys(config2)
.keys(config1)
.concat(Object.keys(config2))
.filter(function filterAxiosKeys(key) {
return axiosKeys.indexOf(key) === -1;
});

utils.forEach(otherKeys, function otherKeysDefaultToConfig2(prop) {
if (typeof config2[prop] !== 'undefined') {
config[prop] = config2[prop];
} else if (typeof config1[prop] !== 'undefined') {
config[prop] = config1[prop];
}
});
utils.forEach(otherKeys, mergeDeepProperties);
Copy link

@cursor cursor bot Jan 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate keys cause redundant processing in otherKeys loop

Low Severity

otherKeys is built by concatenating Object.keys(config1) and Object.keys(config2) without deduplication. When both configs have the same custom property, mergeDeepProperties will be called twice for that key. Consider deduplicating with Array.from(new Set(...)) or filtering duplicates.

Fix in Cursor Fix in Web


return config;
};
54 changes: 21 additions & 33 deletions lib/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,21 @@ function isObject(val) {
return val !== null && typeof val === 'object';
}

/**
* Determine if a value is a plain Object
*
* @param {Object} val The value to test
* @return {boolean} True if value is a plain Object, otherwise false
*/
function isPlainObject(val) {
if (toString.call(val) !== '[object Object]') {
return false;
}

var prototype = Object.getPrototypeOf(val);
return prototype === null || prototype === Object.prototype;
}

/**
* Determine if a value is a Date
*
Expand Down Expand Up @@ -261,8 +276,12 @@ function forEach(obj, fn) {
function merge(/* obj1, obj2, obj3, ... */) {
var result = {};
function assignValue(val, key) {
if (typeof result[key] === 'object' && typeof val === 'object') {
if (isPlainObject(result[key]) && isPlainObject(val)) {
result[key] = merge(result[key], val);
} else if (isPlainObject(val)) {
result[key] = merge({}, val);
} else if (isArray(val)) {
result[key] = val.slice();
} else {
result[key] = val;
}
Expand All @@ -274,37 +293,6 @@ function merge(/* obj1, obj2, obj3, ... */) {
return result;
}

/**
* Function equal to merge with the difference being that no reference
* to original objects is kept.
*
* @see merge
* @param {Object} obj1 Object to merge
* @returns {Object} Result of all merge properties
*/
function deepMerge(/* obj1, obj2, obj3, ... */) {
var result = {};
function assignValue(val, key) {
if (typeof result[key] === 'object' && typeof val === 'object') {
result[key] = deepMerge(result[key], val);
} else if (typeof val === 'object') {
result[key] = deepMerge({}, val);
} else {
result[key] = val;
}
}

var lastArgument = arguments[arguments.length - 1];
if (lastArgument === null || typeof lastArgument === 'undefined') {
return lastArgument;
}

for (var i = 0, l = arguments.length; i < l; i++) {
forEach(arguments[i], assignValue);
}
return result;
}

/**
* Extends object a by mutably adding to it the properties of object b.
*
Expand Down Expand Up @@ -333,6 +321,7 @@ module.exports = {
isString: isString,
isNumber: isNumber,
isObject: isObject,
isPlainObject: isPlainObject,
isUndefined: isUndefined,
isDate: isDate,
isFile: isFile,
Expand All @@ -343,7 +332,6 @@ module.exports = {
isStandardBrowserEnv: isStandardBrowserEnv,
forEach: forEach,
merge: merge,
deepMerge: deepMerge,
extend: extend,
trim: trim
};
Loading