Skip to content

Commit

Permalink
feat: validates HTTP message only once
Browse files Browse the repository at this point in the history
  • Loading branch information
@artem-zakharchenko
artem-zakharchenko committed May 24, 2019
1 parent b7cfb1b commit 0aaacbd
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 73 deletions.
136 changes: 65 additions & 71 deletions lib/TransactionRunner.js
View file
Expand Up @@ -528,102 +528,96 @@ Not performing HTTP request for '${transaction.name}'.\

validateTransaction(test, transaction, callback) {
logger.debug('Validating HTTP transaction by Gavel.js');
logger.debug('Determining whether HTTP transaction is valid (getting boolean verdict)');
gavel.isValid(transaction.real, transaction.expected, 'response', (isValidError, isValid) => {
if (isValidError) {
logger.debug('Gavel.js validation errored:', isValidError);
this.emitError(isValidError, test);

gavel.validate(transaction.real, transaction.expected, 'response', (validateError, gavelResult) => {
if (validateError) {
logger.debug('Gavel.js validation errored:', validateError);
this.emitError(validateError, test);
}

test.title = transaction.id;
test.actual = transaction.real;
test.expected = transaction.expected;
test.request = transaction.request;

const isValid = gavelResult ? gavelResult.isValid : false;

if (isValid) {
test.status = 'pass';
} else {
test.status = 'fail';
}

logger.debug('Validating HTTP transaction (getting verbose validation result)');
gavel.validate(transaction.real, transaction.expected, 'response', (validateError, gavelResult) => {
if (!isValidError && validateError) {
logger.debug('Gavel.js validation errored:', validateError);
this.emitError(validateError, test);
}

// Warn about empty responses
// Expected is as string, actual is as integer :facepalm:
const isExpectedResponseStatusCodeEmpty = ['204', '205'].includes(
test.expected.statusCode ? test.expected.statusCode.toString() : undefined
);
const isActualResponseStatusCodeEmpty = ['204', '205'].includes(
test.actual.statusCode ? test.actual.statusCode.toString() : undefined
);
const hasBody = (test.expected.body || test.actual.body);
if ((isExpectedResponseStatusCodeEmpty || isActualResponseStatusCodeEmpty) && hasBody) {
logger.warn(`\
// Warn about empty responses
// Expected is as string, actual is as integer :facepalm:
const isExpectedResponseStatusCodeEmpty = ['204', '205'].includes(
test.expected.statusCode ? test.expected.statusCode.toString() : undefined
);
const isActualResponseStatusCodeEmpty = ['204', '205'].includes(
test.actual.statusCode ? test.actual.statusCode.toString() : undefined
);
const hasBody = (test.expected.body || test.actual.body);
if ((isExpectedResponseStatusCodeEmpty || isActualResponseStatusCodeEmpty) && hasBody) {
logger.warn(`\
${test.title} HTTP 204 and 205 responses must not \
include a message body: https://tools.ietf.org/html/rfc7231#section-6.3\
`);
}
}

// Create test message from messages of all validation errors
let message = '';
const object = gavelResult || {};
let validatorOutput;
// Create test message from messages of all validation errors
let message = '';
const object = gavelResult || {};
let validatorOutput;

// Order-sensitive list of validation sections to output in the log
const resultSections = ['headers', 'body', 'statusCode']
.filter(sectionName => Object.prototype.hasOwnProperty.call(object, sectionName));
// Order-sensitive list of validation sections to output in the log
const resultSections = ['headers', 'body', 'statusCode']
.filter(sectionName => Object.prototype.hasOwnProperty.call(object, sectionName));

resultSections.forEach((sectionName) => {
validatorOutput = object[sectionName];
(validatorOutput.results || []).forEach((gavelError) => {
message += `${sectionName}: ${gavelError.message}\n`;
});
resultSections.forEach((sectionName) => {
validatorOutput = object[sectionName];
(validatorOutput.results || []).forEach((gavelError) => {
message += `${sectionName}: ${gavelError.message}\n`;
});
});

test.message = message;

// Record raw validation output to transaction results object
//
// It looks like the transaction object can already contain 'results'.
// (Needs to be prooved, the assumption is based just on previous
// version of the code.) In that case, we want to save the new validation
// output, but we want to keep at least the original array of Gavel errors.
const results = transaction.results || {};
for (const sectionName of Object.keys(gavelResult || {})) {
// Section names are 'statusCode', 'headers', 'body' (and 'version', which is irrelevant)
const rawValidatorOutput = gavelResult[sectionName];
if (sectionName !== 'version') {
if (!results[sectionName]) { results[sectionName] = {}; }

// We don't want to modify the object and we want to get rid of some
// custom Gavel.js types ('clone' will keep just plain JS objects).
validatorOutput = clone(rawValidatorOutput);

// If transaction already has the 'results' object, ...
if (results[sectionName].results) {
// ...then take all Gavel errors it contains and add them to the array
// of Gavel errors in the new validator output object...
validatorOutput.results = validatorOutput.results.concat(results[sectionName].results);
}
// ...and replace the original validator object with the new one.
results[sectionName] = validatorOutput;
test.message = message;

// Record raw validation output to transaction results object
//
// It looks like the transaction object can already contain 'results'.
// (Needs to be prooved, the assumption is based just on previous
// version of the code.) In that case, we want to save the new validation
// output, but we want to keep at least the original array of Gavel errors.
const results = transaction.results || {};
for (const sectionName of Object.keys(gavelResult || {})) {
// Section names are 'statusCode', 'headers', 'body' (and 'version', which is irrelevant)
const rawValidatorOutput = gavelResult[sectionName];
if (sectionName !== 'version') {
if (!results[sectionName]) { results[sectionName] = {}; }

// We don't want to modify the object and we want to get rid of some
// custom Gavel.js types ('clone' will keep just plain JS objects).
validatorOutput = clone(rawValidatorOutput);

// If transaction already has the 'results' object, ...
if (results[sectionName].results) {
// ...then take all Gavel errors it contains and add them to the array
// of Gavel errors in the new validator output object...
validatorOutput.results = validatorOutput.results.concat(results[sectionName].results);
}
// ...and replace the original validator object with the new one.
results[sectionName] = validatorOutput;
}
transaction.results = results;
}
transaction.results = results;

// Set the validation results and the boolean verdict to the test object
test.results = transaction.results;
test.valid = isValid;
// Set the validation results and the boolean verdict to the test object
test.results = transaction.results;
test.valid = isValid;

// Propagate test object so 'after' hooks can modify it
transaction.test = test;
callback();
});
// Propagate test object so 'after' hooks can modify it
transaction.test = test;
callback();
});
}

Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Expand Up @@ -41,7 +41,7 @@
"clone": "2.1.2",
"cross-spawn": "6.0.5",
"dredd-transactions": "8.1.3",
"gavel": "3.1.2",
"gavel": "3.2.0",
"glob": "7.1.4",
"html": "1.0.0",
"htmlencode": "0.0.4",
Expand Down
2 changes: 1 addition & 1 deletion test/integration/sanitation-test.js
View file
Expand Up @@ -494,7 +494,7 @@ describe('Sanitation of Reported Data', () => {
assert.equal(events[2].test.status, 'fail');
assert.include(events[2].test.results.general.results[0].message.toLowerCase(), 'fail');
});
it('emitted test data results contain all regular sections', () => assert.hasAllKeys(events[2].test.results, ['general', 'headers', 'body', 'statusCode']));
it('emitted test data results contain all regular sections', () => assert.hasAllKeys(events[2].test.results, ['isValid', 'general', 'headers', 'body', 'statusCode']));
it('sensitive data cannot be found anywhere in the emitted test data', () => {
const test = JSON.stringify(events);
assert.notInclude(test, sensitiveKey);
Expand Down

0 comments on commit 0aaacbd

Please sign in to comment.