Skip to content

Commit

Permalink
fix(stepfunctions): repeated object references not allowed even if no…
Browse files Browse the repository at this point in the history
…t a circular reference (#14628)

Popping the current node from `visited` array after finishing visiting it in `recurseObject` method. This allows another reference to the same object to happen on a different object path (currently being swallowed).

Created unit test before making any code changes to verify the failure scenario on the expected behavior before fixing it.

fixes #14596 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
marciocarmona authored Jun 7, 2021
1 parent b22da80 commit 486990f
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 5 deletions.
15 changes: 10 additions & 5 deletions packages/@aws-cdk/aws-stepfunctions/lib/json-path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,12 @@ interface FieldHandlers {

export function recurseObject(obj: object | undefined, handlers: FieldHandlers, visited: object[] = []): object | undefined {
if (obj === undefined) { return undefined; }
if (visited.includes(obj)) {
return {};
} else {
visited.push(obj);
}

// Avoiding infinite recursion
if (visited.includes(obj)) { return {}; }

// Marking current object as visited for the current recursion path
visited.push(obj);

const ret: any = {};
for (const [key, value] of Object.entries(obj)) {
Expand All @@ -106,6 +107,10 @@ export function recurseObject(obj: object | undefined, handlers: FieldHandlers,
}
}

// Removing from visited after leaving the current recursion path
// Allowing it to be visited again if it's not causing a recursion (circular reference)
visited.pop();

return ret;
}

Expand Down
22 changes: 22 additions & 0 deletions packages/@aws-cdk/aws-stepfunctions/test/fields.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,4 +152,26 @@ describe('Fields', () => {
expect(FieldUtils.findReferencedPaths(paths))
.toStrictEqual(['$.listField', '$.numField', '$.stringField']);
});

test('repeated object references at different tree paths should not be considered as recursions', () => {
const repeatedObject = {
field: JsonPath.stringAt('$.stringField'),
numField: JsonPath.numberAt('$.numField'),
};
expect(FieldUtils.renderObject(
{
reference1: repeatedObject,
reference2: repeatedObject,
},
)).toStrictEqual({
reference1: {
'field.$': '$.stringField',
'numField.$': '$.numField',
},
reference2: {
'field.$': '$.stringField',
'numField.$': '$.numField',
},
});
});
});

0 comments on commit 486990f

Please sign in to comment.