Skip to content

Commit 413b643

Browse files
authored
fix(sfn): stop replacing JsonPath.DISCARD with null (#24717)
Follow-up to #24593. The `renderJsonPath` function is subsituting a literal `null` for `JsonPath.DISCARD`, which results in the key being dropped if the value is sent across a language boundary, which effectively changes semantics. The `JsonPath.DISCARD` value is a `Token` that ultimately resolves to `null`, and it must be preserved as such so that it is safe to exchange across languages. Thanks to @beck3905 for reporting & diagnosing this. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent f511000 commit 413b643

File tree

5 files changed

+36
-16
lines changed

5 files changed

+36
-16
lines changed

packages/@aws-cdk/aws-stepfunctions/lib/states/state.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { Token } from '@aws-cdk/core';
22
import { IConstruct, Construct, Node } from 'constructs';
33
import { Condition } from '../condition';
4-
import { FieldUtils, JsonPath } from '../fields';
4+
import { FieldUtils } from '../fields';
55
import { StateGraph } from '../state-graph';
66
import { CatchProps, Errors, IChainable, INextable, RetryProps } from '../types';
77

@@ -578,7 +578,6 @@ export function renderList<T>(xs: T[], mapFn: (x: T) => any, sortFn?: (a: T, b:
578578
*/
579579
export function renderJsonPath(jsonPath?: string): undefined | null | string {
580580
if (jsonPath === undefined) { return undefined; }
581-
if (jsonPath === JsonPath.DISCARD) { return null; }
582581

583582
if (!Token.isUnresolved(jsonPath) && !jsonPath.startsWith('$')) {
584583
throw new Error(`Expected JSON path to start with '$', got: ${jsonPath}`);

packages/@aws-cdk/aws-stepfunctions/test/state.test.ts

+18-11
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,33 @@
1+
import * as assert from '@aws-cdk/assertions';
12
import * as cdk from '@aws-cdk/core';
23
import { FakeTask } from './fake-task';
3-
import { renderGraph } from './private/render-util';
4-
import { JsonPath } from '../lib';
4+
import { JsonPath, StateMachine } from '../lib';
55

66
test('JsonPath.DISCARD can be used to discard a state\'s output', () => {
7-
const stack = new cdk.Stack();
8-
7+
// GIVEN
8+
const app = new cdk.App();
9+
const stack = new cdk.Stack(app, 'TestStack');
910
const task = new FakeTask(stack, 'my-state', {
1011
inputPath: JsonPath.DISCARD,
1112
outputPath: JsonPath.DISCARD,
1213
resultPath: JsonPath.DISCARD,
1314
});
15+
new StateMachine(stack, 'state-machine', {
16+
definition: task,
17+
});
18+
19+
// WHEN
20+
const definitionString = new assert.Capture();
21+
assert.Template.fromStack(stack).hasResourceProperties('AWS::StepFunctions::StateMachine', {
22+
DefinitionString: definitionString,
23+
});
24+
25+
// THEN
26+
const definition = JSON.parse(definitionString.asString());
1427

15-
expect(renderGraph(task)).toEqual({
16-
StartAt: 'my-state',
28+
expect(definition).toMatchObject({
1729
States: {
1830
'my-state': {
19-
End: true,
20-
Type: 'Task',
21-
Resource: expect.any(String),
22-
Parameters: expect.any(Object),
23-
// The important bits:
2431
InputPath: null,
2532
OutputPath: null,
2633
ResultPath: null,

packages/@aws-cdk/core/lib/private/cloudformation-lang.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -452,4 +452,4 @@ class ScopedCache<O extends object, K, V> {
452452
}
453453
}
454454

455-
const stringifyCache = new ScopedCache<Stack, string, string>();
455+
const stringifyCache = new ScopedCache<Stack, string, string>();

packages/@aws-cdk/core/lib/private/resolve.ts

+8
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,14 @@ export function resolve(obj: any, options: IResolveOptions): any {
192192
return arr;
193193
}
194194

195+
//
196+
// literal null -- from JsonNull resolution, preserved as-is (semantically meaningful)
197+
//
198+
199+
if (obj === null) {
200+
return obj;
201+
}
202+
195203
//
196204
// tokens - invoke 'resolve' and continue to resolve recursively
197205
//

packages/@aws-cdk/core/lib/token.ts

+8-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { unresolved } from './private/encoding';
44
import { Intrinsic } from './private/intrinsic';
55
import { resolve } from './private/resolve';
66
import { TokenMap } from './private/token-map';
7-
import { IResolvable, ITokenResolver } from './resolvable';
7+
import { IResolvable, ITokenResolver, IResolveContext } from './resolvable';
88
import { TokenizedStringFragments } from './string-fragments';
99

1010
/**
@@ -236,12 +236,18 @@ export class Tokenization {
236236
* An object which serializes to the JSON `null` literal, and which can safely
237237
* be passed across languages where `undefined` and `null` are not different.
238238
*/
239-
export class JsonNull {
239+
export class JsonNull implements IResolvable {
240240
/** The canonical instance of `JsonNull`. */
241241
public static readonly INSTANCE = new JsonNull();
242242

243+
public readonly creationStack: string[] = [];
244+
243245
private constructor() { }
244246

247+
public resolve(_ctx: IResolveContext): any {
248+
return null;
249+
}
250+
245251
/** Obtains the JSON representation of this object (`null`) */
246252
public toJSON(): any {
247253
return null;

0 commit comments

Comments
 (0)