diff --git a/lib/package/plugins/PO026-assignVariableUsage.js b/lib/package/plugins/PO026-assignVariableUsage.js index 30ab669e..26db4683 100644 --- a/lib/package/plugins/PO026-assignVariableUsage.js +++ b/lib/package/plugins/PO026-assignVariableUsage.js @@ -107,13 +107,17 @@ const onPolicy = function(policy, cb) { addMessage(textnode[0].lineNumber, textnode[0].columnNumber, "The text of the PropertySetRef element must not be empty"); } - if ( ! pluginUtil.isValidTemplate(psrText)) { + let r = pluginUtil.validateTemplate(psrText); + if ( r) { addMessage(textnode[0].lineNumber, textnode[0].columnNumber, - "The text of the PropertySetRef element must be a valid message template"); + `The text of the PropertySetRef element must be a valid message template (${r})`); } - else if ( !pluginUtil.isValidPropertySetRef(psrText)) { + else { + r = pluginUtil.validatePropertySetRef(psrText); + if (r) { addMessage(textnode[0].lineNumber, textnode[0].columnNumber, - "The text of the PropertySetRef element must resolve to a string that contains exactly one dot"); + `Error in the text of the PropertySetRef element: ${r}`); + } } } @@ -124,9 +128,10 @@ const onPolicy = function(policy, cb) { const textnode = xpath.select("Template/text()", node), templateText = textnode[0] && textnode[0].data; if (templateText) { - if ( ! pluginUtil.isValidTemplate(templateText)) { + const r = pluginUtil.validateTemplate(templateText); + if ( r ) { addMessage(textnode[0].lineNumber, textnode[0].columnNumber, - "The text of the Template element must be a valid message template"); + r.error); } } } diff --git a/lib/package/plugins/_pluginUtil.js b/lib/package/plugins/_pluginUtil.js index 8845bbdb..1cfc7866 100644 --- a/lib/package/plugins/_pluginUtil.js +++ b/lib/package/plugins/_pluginUtil.js @@ -1,91 +1,196 @@ +const debug = require("debug")("apigeelint:template"); const STATES = { - OUTSIDE_CURLY: 0, - INSIDE_CURLY_NO_TEXT: 1, - INSIDE_CURLY: 2, - INSIDE_FUNCTION_NO_TEXT: 3, - INSIDE_FUNCTION: 4 + UNDEFINED : -1, + OUTSIDE_CURLY : 0, + INSIDE_CURLY_NO_TEXT : 1, + INSIDE_CURLY : 2, + INSIDE_VARREF : 3, + INSIDE_FNARGS_NO_TEXT : 4, + INSIDE_FNARGS : 5 }; -const isValidTemplate = (expr) => { +const CODES = { + UPPER_A : 65, + UPPER_Z : 90, + LOWER_a : 97, + LOWER_z : 122, + ZERO : 48, + NINE : 57 + }; + +const ZERO_ARG_FUNCTIONS = [ 'createUuid', 'randomLong' ]; + +const FUNCTION_NAMES = [ + "sha1Hex", "sha1Base64", "sha256Hex", "sha256Base64", "sha384Hex", "sha384Base64", + "sha512Hex", "sha512Base64", "md5Hex", "md5Base64", "xPath", "xpath", "jsonPath", + "substring", "firstnonnull", "replaceAll", "replaceFirst", "createUuid", "randomLong", + "hmacMd5", "hmacSha1", "hmacSha224", "hmacSha256", "hmacSha384", "hmacSha512", + "timeFormat", "timeFormatMs", "timeFormatUTC", "timeFormatUTCMs", "xeger", + "encodeHTML", "escapeJSON", "escapeXML", "escapeXML11", "toUpperCase", "toLowerCase", + "encodeBase64", "decodeBase64" + ]; + +const validateFunction = (expr, start, end) => { + const fn = expr.substring(start, end); + if (FUNCTION_NAMES.find( f => fn == f)) { + return { fn }; + } + return {fn, error: 'unknown function'}; + }; + +const validateFunctionArgs = (expr, fnStart, argsStart, end) => { + const fn = expr.substring(fnStart, argsStart - 1); + const argsString = expr.substring(argsStart, end); + if (argsString.includes(' ')) { + return {fn, error: 'spaces in argument string'}; + } + if (argsString == '' && !ZERO_ARG_FUNCTIONS.includes(fn)) { + return {fn, error: `empty arg string`}; + } + return { fn }; + }; + +const stateToString = s => + Object.keys(STATES).find( key => STATES[key] == s); + + +/* + * returns undefined for valid template. Returns an explanatory string if invalid. + **/ +const validateTemplate = (expr) => { let state = STATES.OUTSIDE_CURLY; - for (const ch of expr) { - switch (state) { - case STATES.OUTSIDE_CURLY: - if (ch == '{') { - state = STATES.INSIDE_CURLY_NO_TEXT; - } - if (ch == '}') { - return false; - } - break; + const states = []; + let code, fnStart = -1, argsStart = -1; + let ix = -1; + try { + for (const ch of expr) { + ix++; + debug(`state(${stateToString(state)}) ch(${ch})`); + switch (state) { + case STATES.OUTSIDE_CURLY: + case STATES.INSIDE_CURLY: + if (ch == '{') { + states.push(state); + state = STATES.INSIDE_CURLY_NO_TEXT; + fnStart = ix; + } + if (ch == '}') { + if (state == STATES.OUTSIDE_CURLY) { + return `unexpected close curly at position ${ix}`; + } + state = states.pop(); + } + break; - case STATES.INSIDE_CURLY_NO_TEXT: - if (ch == '}' || ch == '{' || ch == '[' || ch == '(') { - return false; - } - state = STATES.INSIDE_CURLY; - break; + case STATES.INSIDE_CURLY_NO_TEXT: + if (ch == '}') { + return `empty function name at position ${ix}`; + } + if (ch == '{' || ch == '[' || ch == '(') { + return `unexpected character at position ${ix}: ${ch}`; + } + code = ch.charCodeAt(0); + // examine first character. A numeric is invalid. + if (code >= CODES.ZERO && code <= CODES.NINE) { + return `unexpected character at position ${ix}: ${ch}`; + } + // a non-alphabetic means it's not a variable or function. + else if (!(code >= CODES.UPPER_A && code <= CODES.UPPER_Z) && !(code >= CODES.LOWER_a && code <= CODES.LOWER_z)) { + state = STATES.INSIDE_CURLY; + } + else { + state = STATES.INSIDE_VARREF; + } + break; - case STATES.INSIDE_CURLY: - if (ch == '{' || ch == '[' || ch == ')') { - return false; - } - if (ch == '(') { - state = STATES.INSIDE_FUNCTION_NO_TEXT; - } - if (ch == '}') { - state = STATES.OUTSIDE_CURLY; - } - break; + case STATES.INSIDE_VARREF: + if (ch == '{' || ch == '[' || ch == ')') { + return `unexpected character at position ${ix}: ${ch}`; + } + if (ch == '(') { + const r = validateFunction(expr, fnStart + 1, ix); + if ( r.error ) { + return `unsupported function name (${r.fn})`; + } + argsStart = ix; + state = STATES.INSIDE_FNARGS_NO_TEXT; + } + if (ch == '}') { + state = states.pop(); + } + break; - case STATES.INSIDE_FUNCTION_NO_TEXT: - if (ch == '{' || ch == '[' || ch == '(' || ch == ')' || ch == '}' || ch == ']') { - return false; - } - state = STATES.INSIDE_FUNCTION; - break; + case STATES.INSIDE_FNARGS_NO_TEXT: + if (ch == '{' || ch == '[' || ch == '(' || ch == '}' || ch == ']') { + return `unexpected character at position ${ix}: ${ch}`; + } + if (ch == ')') { + const r = validateFunctionArgs(expr, fnStart + 1, argsStart + 1, ix); + if ( r.error ) { + return `${r.error} for function ${r.fn}`; + } + state = STATES.AWAITING_CLOSE_CURLY; + } + else { + state = STATES.INSIDE_FNARGS; + } + break; - case STATES.INSIDE_FUNCTION: - if (ch == '{' || ch == '[' || ch == '(') { - return false; - } - if (ch == ')') { - state = STATES.AWAITING_CLOSE_CURLY; - } - break; + case STATES.INSIDE_FNARGS: + if (ch == '{' || ch == '[' || ch == '(') { + return `unexpected character at position ${ix}: ${ch}`; + } + if (ch == ')') { + const r = validateFunctionArgs(expr, fnStart + 1, argsStart + 1, ix); + if ( r.error ) { + return `${r.error} for function ${r.fn}`; + } + state = STATES.AWAITING_CLOSE_CURLY; + } + break; - case STATES.AWAITING_CLOSE_CURLY: - if (ch != '}') { - return false; - } - state = STATES.OUTSIDE_CURLY; - break; + case STATES.AWAITING_CLOSE_CURLY: + if (ch != '}') { + return `unexpected character at position ${ix}: ${ch}`; + } + state = states.pop(); + break; - default: - // should not happen - return false; + default: + // should not happen + debug(`unknown state (${state})`); + return 'parse faillure'; + } } + } catch(e) { + // stack underflow + debug(`stack underflow? exception (${e})`); + return 'extraneous close-brace at position ${ix}'; } - return state == STATES.OUTSIDE_CURLY; + debug(`final state(${stateToString(state)})`); + return state == STATES.OUTSIDE_CURLY ? undefined : 'unterminated curly brace'; }; -const isValidPropertySetRef = (expr) => { - if ( ! isValidTemplate(expr)) { - return false; +const validatePropertySetRef = (expr) => { + let r = validateTemplate(expr); + if ( r ) { + return r; } if (expr.includes('{')) { // There is a variable reference. - // Simulate a variable substitution; the result must have AT MOST one dot. - const r = new RegExp('{[^}]+}', 'g'), - result = expr.replaceAll(r, 'xxx'); - return (result.match(/\./g) || []).length <= 1; + // Simulate a variable substitution; the result must have at most one dot. + const re = new RegExp('{[^}]+}', 'g'), + result = expr.replaceAll(re, 'xxx'); + return ((result.match(/\./g) || []).length > 1) ? + 'there is more than one dot in the template result' : undefined; } // No variable reference; the expression must have exactly one dot. - return (expr.match(/\./g) || []).length == 1; + r = (expr.match(/\./g) || []).length; + return (r == 1) ? undefined : `there are ${r} dots ( !=1 ) in the template result`; }; module.exports = { - isValidTemplate, - isValidPropertySetRef + validateTemplate, + validatePropertySetRef }; diff --git a/test/fixtures/resources/PO026-assignVariable-policies/Template-1.xml b/test/fixtures/resources/PO026-assignVariable-policies/Template-1.xml new file mode 100644 index 00000000..1860953e --- /dev/null +++ b/test/fixtures/resources/PO026-assignVariable-policies/Template-1.xml @@ -0,0 +1,14 @@ + + + assigned + + + diff --git a/test/fixtures/resources/PO026-assignVariable-policies/Template-2.xml b/test/fixtures/resources/PO026-assignVariable-policies/Template-2.xml new file mode 100644 index 00000000..724b2dcb --- /dev/null +++ b/test/fixtures/resources/PO026-assignVariable-policies/Template-2.xml @@ -0,0 +1,14 @@ + + + assigned + + + diff --git a/test/specs/PO026-profile.js b/test/specs/PO026-profile.js index 3fd5a49d..f57e4fcb 100644 --- a/test/specs/PO026-profile.js +++ b/test/specs/PO026-profile.js @@ -156,4 +156,16 @@ describe(`PO026 - AssignVariable with PropertySetRef`, () => { assert.equal(p.getReport().messages.length, 0, JSON.stringify(p.getReport().messages)); }); + po026Test(`Template-1.xml`, 'apigeex', (p, foundIssues) => { + assert.equal(foundIssues, false); + assert.ok(p.getReport().messages, "messages undefined"); + assert.equal(p.getReport().messages.length, 0, JSON.stringify(p.getReport().messages)); + }); + + po026Test(`Template-2.xml`, 'apigeex', (p, foundIssues) => { + assert.equal(foundIssues, true); + assert.ok(p.getReport().messages, "messages undefined"); + assert.equal(p.getReport().messages.length, 1, JSON.stringify(p.getReport().messages)); + }); + }); diff --git a/test/specs/testTemplateCheck.js b/test/specs/testTemplateCheck.js index 9d0cfbea..3c239bda 100644 --- a/test/specs/testTemplateCheck.js +++ b/test/specs/testTemplateCheck.js @@ -21,43 +21,59 @@ const assert = require("assert"), describe("TemplateCheck", function() { const testCases = [ - ["{}", false], - ["{a}", true], - ["{{a}", false], - ["{{a}}", false], - ["{[]}", false], - ["{a[]}", false], - ["{a", false], - ["}a", false], - ["{a}}", false], - ["}a{", false], - ["{a}b", true], - ["{a}{b}", true], - ["{timeFormatUTCMs(propertyset.set1.timeformat,system.timestamp)}", true], - ["{timeFormatUTCMs(propertyset.set1.timeformat,system.timestamp) }", false], + ["{}", "empty function name at position 1"], + ["{a}", undefined], + ["{{a}", 'unexpected character at position 1: {'], + ["{{a}}", 'unexpected character at position 1: {'], + ["{[]}", 'unexpected character at position 1: ['], + ["{a[]}", 'unexpected character at position 2: ['], + ["{a", 'unterminated curly brace'], + ["}a", 'unexpected close curly at position 0'], + ["{a}}", 'unexpected close curly at position 3'], + ["}a{", 'unexpected close curly at position 0'], + ["{a}b", undefined], + ["{a}{b}", undefined], + ["{a}.{b}", undefined], + ["{a.b}", undefined], + ["{3}", 'unexpected character at position 1: 3'], + ["{timeFormatUTCMs(propertyset.set1.timeformat,system.timestamp)}", undefined], + ["{timeFormatUTCMs(propertyset.set1.timeformat,system.timestamp) }", + 'unexpected character at position 62: '], + [`{ "organization": "{organization.name}", "environment": "{environment.name}" } `, undefined], + [`{ "organization": "{organization.name}", "environment": "{environment.{name}}" } `, + 'unexpected character at position 71: {'], + [`{ "organization": "{organization.name}", "other": {"environment": "{environment.name}" } } `, + undefined], + ["{createUuid()}", undefined], + ["{createUuid( )}", 'spaces in argument string for function createUuid'], + ["{timeFormatUTCMs()}", 'empty arg string for function timeFormatUTCMs'], + ["{notARealfunction()}", 'unsupported function name (notARealfunction)'], + ["{createUuid[]}", 'unexpected character at position 11: ['], + ["{ createUuid() }", undefined], // but ineffective ]; testCases.forEach( (item, _ix) => { - const [expression, expectedResult] = item; - it(`Check template (${expression}) should return ${expectedResult}`, function() { - const actualResult = pu.isValidTemplate(expression); - assert.equal(actualResult, expectedResult); + const [expression, expectedError] = item; + it(`validateTemplate (${expression}) should return ${expectedError}`, function() { + const actualResult = pu.validateTemplate(expression); + assert.equal(actualResult, expectedError); }); }); }); describe("PropertySetRefCheck", function() { const testCases = [ - ["{foo.bar}", true], - ["{foo.bar}.{var2}.setting", false], - ["{foo.bar}.setting", true], - ["{foo.bar}.{flow.var2}", true], + ["{foo.bar}", undefined], + ["{foo.bar}.{var2}.setting", 'there is more than one dot in the template result'], + ["foo.bar.var2.setting", 'there are 3 dots ( !=1 ) in the template result'], + ["{foo.bar}.setting", undefined], + ["{foo.bar}.{flow.var2}", undefined], ]; testCases.forEach( (item, _ix) => { const [expression, expectedResult] = item; it(`PropertySetRef (${expression}) should be ${expectedResult?'valid':'not valid'}`, function() { - const actualResult = pu.isValidPropertySetRef(expression); + const actualResult = pu.validatePropertySetRef(expression); assert.equal(actualResult, expectedResult); }); });