Skip to content
This repository was archived by the owner on Nov 8, 2024. It is now read-only.

Commit c2354ef

Browse files
fix: uses strict json parsing for actual body, weak for expected body
1 parent 48ecefe commit c2354ef

File tree

9 files changed

+160
-10
lines changed

9 files changed

+160
-10
lines changed

lib/units/validateBody.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,11 @@ function getBodyType(body, contentType, httpMessageOrigin) {
6969
const hasJsonContentType = isJsonContentType(contentType);
7070

7171
try {
72-
parseJson(body);
72+
// Use strict JSON parsing for actual body.
73+
// This prevents any kind of normalization of trailing commas
74+
// and other custom transformations applied by "jju".
75+
parseJson(body, httpMessageOrigin === 'actual');
76+
7377
const bodyMediaType = parseContentType(
7478
hasJsonContentType ? contentType : 'application/json'
7579
);

lib/utils/parseJson.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
11
const { parse } = require('jju/lib/parse');
22

3-
const parseJson = (json, revivew) => {
3+
const parseJson = (json, strict = false, revivew) => {
44
try {
5-
return parse(json, revivew);
5+
// Strict parsing uses native JSON parsing and implies
6+
// a given JSON is valid according to the spec.
7+
//
8+
// Non-strict parsing uses "jju" enhanced parsing
9+
// that allows trailing commas and potential other transformations.
10+
const jsonParser = strict ? JSON.parse : parse;
11+
return jsonParser(json, revivew);
612
} catch (error) {
713
throw error;
814
}

lib/utils/schema-v4-generator.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
const getType = require('./get-type');
2+
const parseJson = require('./parseJson');
23

34
const SCHEMA_VERSION = 'http://json-schema.org/draft-04/schema#';
45

@@ -30,7 +31,7 @@ class SchemaV4Generator {
3031
* @option {SchemaProperties} properties
3132
*/
3233
constructor({ json, properties }) {
33-
this.json = typeof json === 'string' ? JSON.parse(json) : json;
34+
this.json = typeof json === 'string' ? parseJson(json) : json;
3435

3536
this.schema = undefined;
3637
this.properties = properties || new SchemaV4Properties({});

lib/validators/json-schema.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ const deepEqual = require('deep-equal');
33
const tv4 = require('tv4');
44
const jsonPointer = require('json-pointer');
55

6+
const parseJson = require('../utils/parseJson');
67
const metaSchemaV3 = require('../meta-schema-v3');
78
const metaSchemaV4 = require('../meta-schema-v4');
89
const errors = require('../errors');
@@ -89,7 +90,7 @@ class JsonSchema {
8990

9091
if (typeof this.data === 'string') {
9192
try {
92-
this.data = JSON.parse(this.data);
93+
this.data = parseJson(this.data);
9394
} catch (error) {
9495
const outError = new errors.DataNotJsonParsableError(
9596
`JSON validator: body: ${error.message}`
@@ -101,7 +102,7 @@ class JsonSchema {
101102

102103
if (typeof this.schema === 'string') {
103104
try {
104-
this.schema = JSON.parse(this.schema);
105+
this.schema = parseJson(this.schema);
105106
} catch (error) {
106107
const outError = new errors.SchemaNotJsonParsableError(
107108
`JSON validator: schema: ${error.message}`

test/chai.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ chai.use(({ Assertion }, utils) => {
1313
const { currentError: error, currentErrorIndex } = this.__flags;
1414
const target = error[propName];
1515
const isRegExp = expectedValue instanceof RegExp;
16-
const matchWord = isRegExp ? 'matches' : 'equals';
16+
const matchWord = isRegExp ? 'matches the following RegExp' : 'equals';
1717

1818
new Assertion(error).to.be.instanceOf(Object);
1919
new Assertion(error).to.have.property(propName);
@@ -29,7 +29,7 @@ ${stringifiedObj}
2929
3030
to have an error at index ${currentErrorIndex} that includes property "${propName}" that ${matchWord}:
3131
32-
${JSON.stringify(expectedValue)}
32+
${isRegExp ? expectedValue.toString() : JSON.stringify(expectedValue)}
3333
3434
but got:
3535
@@ -45,7 +45,7 @@ to have an error at index ${currentErrorIndex} that includes property "${propNam
4545
${JSON.stringify(target)}
4646
`,
4747
JSON.stringify(target),
48-
JSON.stringify(expectedValue),
48+
isRegExp ? expectedValue.toString() : JSON.stringify(expectedValue),
4949
true
5050
);
5151
});
Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1,4 @@
1-
module.exports = `{"111!invalid string ... issue":''}`;
1+
// Note that Json Schema now uses "jju" for JSON parsing
2+
// which allows certain JSON abnormalities (i.e. trailing commas, single quotes).
3+
// Account on that to produce an invalid JSON.
4+
module.exports = `{"111!invalid string ... issue":malformed''}`;
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
const { expect } = require('../chai');
2+
const { validate } = require('../../lib/validate');
3+
4+
describe('Regression: Missing expected header', () => {
5+
const actualResponse = {
6+
method: 'POST',
7+
headers: {
8+
host: 'private-54408-mynotesapi.apiary-mock.test:8001',
9+
'content-length': '27',
10+
connection: 'keep-alive'
11+
},
12+
body: '{"id":2,"title":"Buy milk"}'
13+
};
14+
const result = validate(
15+
{
16+
method: 'POST',
17+
headers: {},
18+
body: '',
19+
headersSchema: null,
20+
bodySchema: null
21+
},
22+
actualResponse
23+
);
24+
25+
it('should contain "headers.values.expected" as empty Object', () => {
26+
expect(result.fields.headers.values.expected).to.deep.equal({});
27+
});
28+
29+
it('should contain "headers.values.actual"', () => {
30+
expect(result.fields.headers.values.actual).to.deep.equal(
31+
actualResponse.headers
32+
);
33+
});
34+
});
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
const { expect } = require('../chai');
2+
const { validateBody } = require('../../lib/units/validateBody');
3+
4+
describe('Regression: Unparseable JSON body', () => {
5+
describe('given unparseable JSON as expected body', () => {
6+
const result = validateBody(
7+
{
8+
headers: {
9+
'content-type': 'application/json'
10+
},
11+
body: `{ "username": "adm02", }`,
12+
bodySchema: null
13+
},
14+
{
15+
body: `{ "username": "adm03" }`,
16+
headers: {
17+
'content-type': 'application/json'
18+
}
19+
}
20+
);
21+
22+
it('marks field as invalid', () => {
23+
expect(result).to.be.valid;
24+
});
25+
26+
it('has "json" kind', () => {
27+
expect(result).to.have.kind('json');
28+
});
29+
30+
it('has values', () => {
31+
expect(result).to.have.property('values');
32+
expect(result.values).to.deep.equal({
33+
expected: `{ "username": "adm02", }`,
34+
actual: '{ "username": "adm03" }'
35+
});
36+
});
37+
38+
it('has no errors', () => {
39+
expect(result).to.not.have.errors;
40+
});
41+
});
42+
43+
describe('given unparseable JSON as actual body', () => {
44+
const result = validateBody(
45+
{
46+
headers: {
47+
'content-type': 'application/json'
48+
},
49+
body: `{ "firstName": "John" }`,
50+
bodySchema: null
51+
},
52+
{
53+
body: `{ "firstName": "Kyle", }`,
54+
headers: {
55+
'content-type': 'application/json'
56+
}
57+
}
58+
);
59+
60+
it('marks field as invalid', () => {
61+
expect(result).to.not.be.valid;
62+
});
63+
64+
it('has "null" kind', () => {
65+
expect(result).to.have.kind(null);
66+
});
67+
68+
it('has values', () => {
69+
expect(result).to.have.property('values');
70+
expect(result.values).to.deep.equal({
71+
expected: `{ "firstName": "John" }`,
72+
actual: `{ "firstName": "Kyle", }`
73+
});
74+
});
75+
76+
describe('produces an error', () => {
77+
it('exactly one error', () => {
78+
expect(result).to.have.errors.lengthOf(1);
79+
});
80+
81+
it('has error message about unparseable actual body', () => {
82+
expect(result)
83+
.to.have.errorAtIndex(0)
84+
.withMessage(
85+
/^Can't validate: actual body 'Content-Type' header is 'application\/json' but body is not a parseable JSON:/
86+
);
87+
});
88+
});
89+
});
90+
});

test/unit/validators/json-schema.test.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,17 @@ describe('JsonSchema', () => {
149149
assert.isDefined(validator.validateSchema);
150150
});
151151

152+
describe('when invalid, but jju-compliant, JSON-stringified-data are provided', () => {
153+
const invalidJjuCompliantSchema = `{"foo":'key',}`;
154+
155+
it('should not throw an error', () => {
156+
const fn = () => {
157+
new JsonSchema(invalidJjuCompliantSchema);
158+
};
159+
assert.doesNotThrow(fn);
160+
});
161+
});
162+
152163
describe('when invalid JSON-stringified-data are provided', () => {
153164
const invalidStringifiedSchema = require('../../fixtures/invalid-stringified-schema');
154165

0 commit comments

Comments
 (0)