Skip to content

Commit

Permalink
Merge pull request #79 from massfords/master
Browse files Browse the repository at this point in the history
fix: detect invalid outbound transition from map/parallel state.
  • Loading branch information
ChristopheBougere authored Jun 25, 2022
2 parents ee13b90 + 9939422 commit 1a4c195
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 2 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ node_modules
coverage
reports
**/junit.xml
.idea
44 changes: 44 additions & 0 deletions src/__tests__/definitions/invalid-map-ob-link.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
{
"Comment": "The link to \"Final State\" from within the Iterator is invalid since the target state is defined outside of the Iterator",
"StartAt": "Map",
"States": {
"Map": {
"Type": "Map",
"Next": "Final State",
"InputPath": "$.input",
"ItemsPath": "$.items",
"MaxConcurrency": 0,
"Iterator": {
"StartAt": "ChoiceState",
"States": {
"ChoiceState": {
"Type": "Choice",
"Choices": [
{
"Variable": "$.foo",
"NumericEquals": 1,
"Next": "Wait 20s"
},
{
"Variable": "$.foo",
"NumericEquals": 2,
"Next": "Final State"
}
],
"Default": "Wait 20s"
},
"Wait 20s": {
"Type": "Wait",
"Seconds": 20,
"End": true
}
}
},
"ResultPath": "$.result"
},
"Final State": {
"Type": "Pass",
"End": true
}
}
}
56 changes: 56 additions & 0 deletions src/__tests__/definitions/invalid-parallel-ob-link.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
{
"Comment": "The link to the \"Final State\" within the Choice is invalid because it targets a state outside of its parent branch",
"StartAt": "Parallel",
"States": {
"Parallel": {
"Type": "Parallel",
"Next": "Final State",
"Branches": [
{
"StartAt": "Wait 20s",
"States": {
"Wait 20s": {
"Type": "Wait",
"Seconds": 20,
"End": true
}
}
},
{
"StartAt": "Pass",
"States": {
"Pass": {
"Type": "Pass",
"Next": "ChoiceState"
},
"ChoiceState": {
"Type": "Choice",
"Choices": [
{
"Variable": "$.foo",
"NumericEquals": 1,
"Next": "Wait 10s"
},
{
"Variable": "$.foo",
"NumericEquals": 2,
"Next": "Final State"
}
],
"Default": "Wait 10s"
},
"Wait 10s": {
"Type": "Wait",
"Seconds": 10,
"End": true
}
}
}
]
},
"Final State": {
"Type": "Pass",
"End": true
}
}
}
71 changes: 71 additions & 0 deletions src/lib/state-transitions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
const jp = require('jsonpath');

module.exports = (definition) => {
const errorMessages = [];

// given a nested state machine, this function will examine
// each state and record its `Next` or `Default` values
// to see what states are reachable.
// Avoids traversing into Map or Parallel states since the
// states defined within those containers are not valid
// targets for states outside the containers.
const nextAndDefaultTargets = (nestedStateMachine) => {
const states = [];
Object.keys(nestedStateMachine.States).forEach((stateName) => {
const nestedState = nestedStateMachine.States[stateName];
const isContainer = ['Map', 'Parallel'].indexOf(nestedState.Type) >= 0;
const path = isContainer ? '$.[\'Next\',\'Default\']' : '$..[\'Next\',\'Default\']';
states.push(...jp.query(nestedState, path));
});
return states;
};

// reports an error for each state that is found to be an invalid
// transition
const validateNestedStateMachine = (nestedStateMachine) => {
let availStateNames = [];
// don't traverse into any nested states. We only want to record the States
// that are immediately under the Branch.
// These are the only valid states to link to from within the branch
jp.query(nestedStateMachine, '$.States').forEach((branchStates) => {
availStateNames = availStateNames.concat(Object.keys(branchStates));
});

// check that there are no transitions outside this branch
const targetedStates = nextAndDefaultTargets(nestedStateMachine);

return targetedStates.filter((state) => availStateNames.indexOf(state) === -1);
};

// we know the step function is schema valid
// we know that every `Parallel` state has its expected `Branches` field
// we need to visit each Branch within a Parallel to ensure that it doesn't
// link outside its branch.
jp.query(definition, '$..[\'Branches\']')
.forEach((parallelBranches) => {
parallelBranches.forEach((nestedStateMachine) => {
const errs = validateNestedStateMachine(nestedStateMachine).map((state) => ({
'Error code': 'BRANCH_OUTBOUND_TRANSITION_TARGET',
Message: `Parallel branch state cannot transition to target: ${state}`,
}));

errorMessages.push(...errs);
});
});

// we know the step function is schema valid
// we know that every `Map` state has its expected `Iterator` field
// we need to visit the Iterator within a Map to ensure that it doesn't
// link outside its container.
jp.query(definition, '$..[\'Iterator\']')
.forEach((nestedStateMachine) => {
const errs = validateNestedStateMachine(nestedStateMachine).map((state) => ({
'Error code': 'MAP_OUTBOUND_TRANSITION_TARGET',
Message: `Map branch state cannot transition to target: ${state}`,
}));

errorMessages.push(...errs);
});

return errorMessages;
};
12 changes: 10 additions & 2 deletions src/validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const map = require('./schemas/map');
const errors = require('./schemas/errors');
const checkJsonPath = require('./lib/json-path-errors');
const missingTransitionTarget = require('./lib/missing-transition-target');
const stateTransitions = require('./lib/state-transitions');

function formatError(e) {
const code = e.Code ? e.Code : e['Error code'];
Expand Down Expand Up @@ -45,16 +46,23 @@ function validator(definition) {
// Validating JSON schemas
const isJsonSchemaValid = ajv.validate('http://asl-validator.cloud/state-machine.json#', definition);

// Check for Parallel states
const transitionErrors = isJsonSchemaValid ? stateTransitions(definition) : [];

return {
isValid: isJsonSchemaValid && !jsonPathErrors.length && !missingTransitionTargetErrors.length,
errors: jsonPathErrors.concat(ajv.errors || []).concat(missingTransitionTargetErrors || []),
isValid: isJsonSchemaValid && !jsonPathErrors.length && !missingTransitionTargetErrors.length
&& !transitionErrors.length,
errors: jsonPathErrors.concat(ajv.errors || [])
.concat(missingTransitionTargetErrors || [])
.concat(transitionErrors || []),
errorsText: (separator = '\n') => {
const errorList = [];
errorList.push(jsonPathErrors.map(formatError).join(separator));
if (ajv.errors) {
errorList.push(ajv.errorsText(ajv.errors, { separator }));
}
errorList.push(missingTransitionTargetErrors.map(formatError).join(separator));
errorList.push(transitionErrors.map(formatError).join(separator));
return errorList.join(separator);
},
};
Expand Down

0 comments on commit 1a4c195

Please sign in to comment.