Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

token-aware-jsonify: Stringify resolved tokens #406

Merged
merged 2 commits into from
Jul 25, 2018

Conversation

eladb
Copy link
Contributor

@eladb eladb commented Jul 25, 2018

token-aware-jsonify, which is currently used to
serialize CloudWatch dashboard configuration into
a JSON string is expected to return a stringified JSON object.
This means that any string within the object must be escaped (e.g. not
include "\n" or quotes).

The function currently stringifies the primary string but this still
leaves room for non-allowed characters in the resolved tokens. For example,
if a token resolves to { "Fn::Join": [ "", [ "Hello,\nWorld!" ] ] } then
the deploy-time value will be "Hello,\nWorld!" which must be represented
in JSON as "Hello,\nWorld!".

This change eagerly stringifies all string values in the resolved tokens.
Theoretically this might cause trouble in the case where token strings
have special characters AND not emitted, but this seems like a long shot
and we optimize for the common case.

`token-aware-jsonify` is expected to return a stringified JSON object.
This means that any string within the object must be escaped (e.g. not
include "\n" or quotes).

The function currently stringifies the primary string but this still
leaves room for non-allowed characters in the resolved tokens. For example,
if a token resolves to `{ "Fn::Join": [ "", [ "Hello,\nWorld!" ] ] }` then
the deploy-time value will be "Hello,\nWorld!" which must be represented
in JSON as "Hello,\\nWorld!".

This change eagerly stringifies all string values in the resolved tokens.
Theoretically this might cause trouble in the case where token strings
have special characters AND not emitted, but this seems like a long shot
and we optimize for the common case.
@eladb eladb requested a review from RomainMuller July 25, 2018 13:14
@eladb eladb merged commit 264e6b5 into master Jul 25, 2018
@eladb eladb deleted the benisrae/stringify-strings branch July 25, 2018 13:47
@NGL321 NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants