Skip to content
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.

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

JSON improvements: throw if JSON parsing failed; number, boolean can be passed directly as payload for encoding to JSON #2613, #61, #907 #3688

Merged
merged 14 commits into from Apr 19, 2021
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions README.md
Expand Up @@ -458,6 +458,16 @@ These are the available config options for making requests. Only the `url` is re
// - Node only (XHR cannot turn off decompression)
decompress: true // default

// transitional options for backward compatibility that may be removed in the newer versions
transitional: {
// silent JSON parsing mode
// `true` - ignore JSON parsing errors and set response.data to null if parsing failed (old behaviour)
// `false` - throw SyntaxError if JSON parsing failed (Note: responseType must be set to 'json')
silentJSONParsing: true; // default value for the current Axios version

// try to parse the response string as JSON even if `resposeType` is not 'json'
forcedJSONParsing: true;
}
}
```

Expand Down
6 changes: 6 additions & 0 deletions index.d.ts
Expand Up @@ -41,6 +41,11 @@ export type ResponseType =
| 'text'
| 'stream'

export interface TransitionalOptions{
silentJSONParsing: boolean;
forcedJSONParsing: boolean;
}

export interface AxiosRequestConfig {
url?: string;
method?: Method;
Expand Down Expand Up @@ -71,6 +76,7 @@ export interface AxiosRequestConfig {
proxy?: AxiosProxyConfig | false;
cancelToken?: CancelToken;
decompress?: boolean;
transitional?: TransitionalOptions
}

export interface AxiosResponse<T = any> {
Expand Down
16 changes: 5 additions & 11 deletions lib/adapters/xhr.js
Expand Up @@ -13,6 +13,7 @@ module.exports = function xhrAdapter(config) {
return new Promise(function dispatchXhrRequest(resolve, reject) {
var requestData = config.data;
var requestHeaders = config.headers;
var responseType = config.responseType;

if (utils.isFormData(requestData)) {
delete requestHeaders['Content-Type']; // Let the browser set it
Expand Down Expand Up @@ -49,7 +50,8 @@ module.exports = function xhrAdapter(config) {

// Prepare the response
var responseHeaders = 'getAllResponseHeaders' in request ? parseHeaders(request.getAllResponseHeaders()) : null;
var responseData = !config.responseType || config.responseType === 'text' ? request.responseText : request.response;
var responseData = !responseType || responseType === 'text' || responseType === 'json' ?
request.responseText : request.response;
var response = {
data: responseData,
status: request.status,
Expand Down Expand Up @@ -133,16 +135,8 @@ module.exports = function xhrAdapter(config) {
}

// Add responseType to request if needed
if (config.responseType) {
try {
request.responseType = config.responseType;
} catch (e) {
// Expected DOMException thrown by browsers not compatible XMLHttpRequest Level 2.
// But, this can be suppressed for 'json' type as it can be parsed by default 'transformResponse' function.
if (config.responseType !== 'json') {
throw e;
}
}
if (responseType && responseType !== 'json') {
request.responseType = config.responseType;
}

// Handle progress if needed
Expand Down
11 changes: 11 additions & 0 deletions lib/core/Axios.js
Expand Up @@ -5,7 +5,9 @@ var buildURL = require('../helpers/buildURL');
var InterceptorManager = require('./InterceptorManager');
var dispatchRequest = require('./dispatchRequest');
var mergeConfig = require('./mergeConfig');
var validator = require('../helpers/validator');

var validators = validator.validators;
/**
* Create a new instance of Axios
*
Expand Down Expand Up @@ -45,6 +47,15 @@ Axios.prototype.request = function request(config) {
config.method = 'get';
}

var transitional = config.transitional;

if (transitional !== undefined) {
validator.assertOptions(transitional, {
silentJSONParsing: validators.transitional(validators.boolean, '1.0.0'),
forcedJSONParsing: validators.transitional(validators.boolean, '1.0.0')
});
}

// filter out skipped interceptors
var requestInterceptorChain = [];
var synchronousRequestInterceptors = true;
Expand Down
9 changes: 6 additions & 3 deletions lib/core/dispatchRequest.js
Expand Up @@ -27,7 +27,8 @@ module.exports = function dispatchRequest(config) {
config.headers = config.headers || {};

// Transform request data
config.data = transformData(
config.data = transformData.call(
config,
config.data,
config.headers,
config.transformRequest
Expand All @@ -53,7 +54,8 @@ module.exports = function dispatchRequest(config) {
throwIfCancellationRequested(config);

// Transform response data
response.data = transformData(
response.data = transformData.call(
config,
response.data,
response.headers,
config.transformResponse
Expand All @@ -66,7 +68,8 @@ module.exports = function dispatchRequest(config) {

// Transform response data
if (reason && reason.response) {
reason.response.data = transformData(
reason.response.data = transformData.call(
config,
reason.response.data,
reason.response.headers,
config.transformResponse
Expand Down
4 changes: 3 additions & 1 deletion lib/core/transformData.js
@@ -1,6 +1,7 @@
'use strict';

var utils = require('./../utils');
var defaults = require('./../defaults');

/**
* Transform the data for a request or a response
Expand All @@ -11,9 +12,10 @@ var utils = require('./../utils');
* @returns {*} The resulting transformed data
*/
module.exports = function transformData(data, headers, fns) {
var context = this || defaults;
/*eslint no-param-reassign:0*/
utils.forEach(fns, function transform(fn) {
data = fn(data, headers);
data = fn.call(context, data, headers);
});

return data;
Expand Down
32 changes: 26 additions & 6 deletions lib/defaults.js
Expand Up @@ -2,6 +2,7 @@

var utils = require('./utils');
var normalizeHeaderName = require('./helpers/normalizeHeaderName');
var enhanceError = require('./core/enhanceError');

var DEFAULT_CONTENT_TYPE = {
'Content-Type': 'application/x-www-form-urlencoded'
Expand All @@ -26,11 +27,18 @@ function getDefaultAdapter() {
}

var defaults = {

transitional: {
silentJSONParsing: true,
forcedJSONParsing: true
},

adapter: getDefaultAdapter(),

transformRequest: [function transformRequest(data, headers) {
normalizeHeaderName(headers, 'Accept');
normalizeHeaderName(headers, 'Content-Type');

if (utils.isFormData(data) ||
utils.isArrayBuffer(data) ||
utils.isBuffer(data) ||
Expand All @@ -47,21 +55,33 @@ var defaults = {
setContentTypeIfUnset(headers, 'application/x-www-form-urlencoded;charset=utf-8');
return data.toString();
}
if (utils.isObject(data)) {
if (utils.isObject(data) || (headers && headers['Content-Type'] === 'application/json')) {
setContentTypeIfUnset(headers, 'application/json;charset=utf-8');
return JSON.stringify(data);
}
return data;
}],

transformResponse: [function transformResponse(data) {
var result = data;
if (utils.isString(result) && result.length) {
var transitional = this.transitional;
var silentJSONParsing = transitional && transitional.silentJSONParsing;
var forcedJSONParsing = transitional && transitional.forcedJSONParsing;
var strictJSONParsing = !silentJSONParsing && this.responseType === 'json';

if (strictJSONParsing || (forcedJSONParsing && utils.isString(data) && data.length)) {
try {
result = JSON.parse(result);
} catch (e) { /* Ignore */ }
return JSON.parse(data);
} catch (e) {
if (strictJSONParsing) {
if (e.name === 'SyntaxError') {
throw enhanceError(e, this, 'E_JSON_PARSE');
}
throw e;
}
}
}
return result;

return data;
}],

/**
Expand Down
105 changes: 105 additions & 0 deletions lib/helpers/validator.js
@@ -0,0 +1,105 @@
'use strict';

var pkg = require('./../../package.json');

var validators = {};

// eslint-disable-next-line func-names
['object', 'boolean', 'number', 'function', 'string', 'symbol'].forEach(function(type, i) {
validators[type] = function validator(thing) {
return typeof thing === type || 'a' + (i < 1 ? 'n ' : ' ') + type;
};
});

var deprecatedWarnings = {};
var currentVerArr = pkg.version.split('.');
Copy link

@luchaos luchaos Sep 7, 2021

Choose a reason for hiding this comment

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

var pkg = require('./../../package.json'); tries to load my project's package.json which has no version key set, causing the application to fail on boot in the browser.
what's the purpose of this validator and why does it need my project's package.json and a potentially non-existing version? (it was brought to my attention that that's the culprit after all - for the version is a required package.json field https://docs.npmjs.com/creating-a-package-json-file#required-name-and-version-fields)
it's a vue3/webpack project.
edit: seems to be related? #3499

Copy link
Member

Choose a reason for hiding this comment

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

Will have a look as to why this is happening

Copy link

@luchaos luchaos Sep 7, 2021

Choose a reason for hiding this comment

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

it's an edge case, really - people do the weirdest things in their project's package.json (as my own example showed). the fix is simple enough, the package.json just has to be valid and meet the minimum requirements (the version should be there at all times, technically - til).

yet, this issue might highlight a completely different problem. relying on the validity of / bundling the package.json might not be the best solution to use a version for negotiation (as seen in this PR) nor to be used for the user agent (as seen in the linked issue above where it is imported in the http adapter).

is this actually a path/loader issue and the version to be read is the one from axios' own package.json, not the project's? looking at the http adapter code it looks like it wants axios' version (headers['User-Agent'] = 'axios/' + pkg.version;)
if that's the case then i'd assume that axios is potentially sending the wrong version (the project's it's used in) in the user agent header, too?

Choose a reason for hiding this comment

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

I meet the same question here, so do you know how to solve it?

Copy link

Choose a reason for hiding this comment

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

I meet the same question here, so do you know how to solve it?

For me the solution was to add a version to package.json. "version": "0.0.0" works just fine.


/**
* Compare package versions
* @param {string} version
* @param {string?} thanVersion
* @returns {boolean}
*/
function isOlderVersion(version, thanVersion) {
var pkgVersionArr = thanVersion ? thanVersion.split('.') : currentVerArr;
var destVer = version.split('.');
for (var i = 0; i < 3; i++) {
if (pkgVersionArr[i] > destVer[i]) {
return true;
} else if (pkgVersionArr[i] < destVer[i]) {
return false;
}
}
return false;
}

/**
* Transitional option validator
* @param {function|boolean?} validator
* @param {string?} version
* @param {string} message
* @returns {function}
*/
validators.transitional = function transitional(validator, version, message) {
var isDeprecated = version && isOlderVersion(version);

function formatMessage(opt, desc) {
return '[Axios v' + pkg.version + '] Transitional option \'' + opt + '\'' + desc + (message ? '. ' + message : '');
}

// eslint-disable-next-line func-names
return function(value, opt, opts) {
if (validator === false) {
throw new Error(formatMessage(opt, ' has been removed in ' + version));
}

if (isDeprecated && !deprecatedWarnings[opt]) {
deprecatedWarnings[opt] = true;
// eslint-disable-next-line no-console
console.warn(
formatMessage(
opt,
' has been deprecated since v' + version + ' and will be removed in the near future'
)
);
}

return validator ? validator(value, opt, opts) : true;
};
};

/**
* Assert object's properties type
* @param {object} options
* @param {object} schema
* @param {boolean} allowUnknown
*/

function assertOptions(options, schema, allowUnknown) {
if (typeof options !== 'object') {
throw new TypeError('options must be an object');
}
var keys = Object.keys(options);
var i = keys.length;
while (i-- > 0) {
var opt = keys[i];
var validator = schema[opt];
if (validator) {
var value = options[opt];
var result = value === undefined || validator(value, opt, options);
if (result !== true) {
throw new TypeError('option ' + opt + ' must be ' + result);
}
continue;
}
if (allowUnknown !== true) {
throw Error('Unknown option ' + opt);
}
}
}

module.exports = {
isOlderVersion: isOlderVersion,
assertOptions: assertOptions,
validators: validators
};
2 changes: 1 addition & 1 deletion test/specs/defaults.spec.js
Expand Up @@ -25,7 +25,7 @@ describe('defaults', function () {
});

it('should transform response json', function () {
var data = defaults.transformResponse[0]('{"foo":"bar"}');
var data = defaults.transformResponse[0].call(defaults, '{"foo":"bar"}');

expect(typeof data).toEqual('object');
expect(data.foo).toEqual('bar');
Expand Down
58 changes: 58 additions & 0 deletions test/specs/helpers/validator.spec.js
@@ -0,0 +1,58 @@
'use strict';

var validator = require('../../../lib/helpers/validator');

describe('validator::isOlderVersion', function () {
it('should return true if dest version is older than the package version', function () {
expect(validator.isOlderVersion('0.0.1', '1.0.0')).toEqual(true);
expect(validator.isOlderVersion('0.0.1', '0.1.0')).toEqual(true);
expect(validator.isOlderVersion('0.0.1', '0.0.1')).toEqual(false);


expect(validator.isOlderVersion('100.0.0', '1.0.0')).toEqual(false);
expect(validator.isOlderVersion('100.0.0', '0.1.0')).toEqual(false);
expect(validator.isOlderVersion('100.0.0', '0.0.1')).toEqual(false);

expect(validator.isOlderVersion('0.10000.0', '1000.0.1')).toEqual(true);
});
});

describe('validator::assertOptions', function () {
it('should throw only if unknown an option was passed', function () {
expect(function() {
validator.assertOptions({
x: true
}, {
y: validator.validators.boolean
});
}).toThrow(new Error('Unknown option x'));

expect(function() {
validator.assertOptions({
x: true
}, {
x: validator.validators.boolean,
y: validator.validators.boolean
});
}).not.toThrow(new Error('Unknown option x'));
});

it('should throw TypeError only if option type doesn\'t match', function () {
expect(function() {
validator.assertOptions({
x: 123
}, {
x: validator.validators.boolean
});
}).toThrow(new TypeError('option x must be a boolean'));

expect(function() {
validator.assertOptions({
x: true
}, {
x: validator.validators.boolean,
y: validator.validators.boolean
});
}).not.toThrow();
});
});