awslabs / aws-cfn-template-flip

Tool for converting AWS CloudFormation templates between JSON and YAML formats.
Apache License 2.0
994 stars 142 forks source link

Fix Json string literal in resource property #80

Closed koiker closed 5 years ago

koiker commented 5 years ago

Fix: This fix solves the issue with JSON payload in resource AWS::StepFunctions::StateMachine and property DefinitionString that has a JSON string that must remain a json string when converted to YAML.

Details:

When the script load a json file all key/values are converted to objects and those objects and later converted to yaml. This behavior made some payloads like DefinitionString to become Yaml but the property is a json and in a Yaml document this property must be converted to a string literal. The way to solve this is to run a parser that will check for Key == AWS::StepFunctions::StateMachine and when found this resource we try to find the property DefinitionString. If found the property we convert the object into a string using json.dumps and create a python object to be specifically parsed in the yaml representer to use the style |. Here is where the things get interesting.
Just adding the style | to generate a literal didn't work as expected as some simple strings got converted correctly and others don't.

You can reproduce the behavior with this script:

import yaml

class Literal(str):
    pass

def representer_literal(dumper, value):
    return dumper.represent_scalar(u"tag:yaml.org,2002:str", value, style='|')

if __name__ == '__main__':
    var = Literal(u"{\n  line1\n  line2\n  line3\n}")
    yaml.add_representer(Literal, representer_literal)
    print(yaml.dump({'a': var}))
    # Output:
    # a: |-
    #   {
    #     line1
    #     line2
    #     line3
    #   }
    var = Literal(u"{\n \"texto\"   \n line1\n  line2\n  line3\n}")
    print(yaml.dump({'a': var}))
    # Output:
    # a: "{\n \"texto\"   \n line1\n  line2\n  line3\n}"

After analysing the problem I found that pyYaml in the Emitter step there is a string validation to check against leading and trailing whitespaces. The code is here: https://github.com/yaml/pyyaml/blob/master/lib/yaml/emitter.py

The way to circumvent this characteristic is to override the method analyze_scalarfrom class Emitter and when we find a scalar that is a instance of our object type LiteralString we return the ScalarAnalisys with the parameters that we want (In this case to allow_block_plain=True

Other scalar types just get his normal flow using the super()

This solution allow an easy way to add more resource properties as the code has an array of tuples of (<Resource_Type>, <Property_Name>)


Fix: Updated the PyYaml requirement in setup.py to >= 4.1 to avoid remote execution vulnerability in CVE-2017-18342 By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

stilvoid commented 5 years ago

Hi @koiker, thanks for this! Happy with the changes but following merging your other PRs, there are now merge conflicts. If you fix those, I'll merge this :)

codecov-io commented 5 years ago

Codecov Report

Merging #80 into master will decrease coverage by 0.23%. The diff coverage is 97.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #80      +/-   ##
==========================================
- Coverage   99.31%   99.08%   -0.24%     
==========================================
  Files          10       11       +1     
  Lines         293      327      +34     
==========================================
+ Hits          291      324      +33     
- Misses          2        3       +1
Impacted Files Coverage Δ
cfn_clean/__init__.py 100% <100%> (ø) :arrow_up:
cfn_tools/literal.py 100% <100%> (ø)
cfn_flip/yaml_dumper.py 100% <100%> (ø) :arrow_up:
cfn_flip/__init__.py 100% <100%> (ø) :arrow_up:
cfn_tools/yaml_dumper.py 97.22% <92.3%> (-2.78%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3107ee2...23e7a8b. Read the comment docs.

stilvoid commented 5 years ago

Awesome :)