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

Document j2_environment autoescape setting to control Jinja2 escaping #1380

Open
iainelder opened this issue Oct 30, 2023 · 4 comments
Open

Comments

@iainelder
Copy link
Contributor

iainelder commented Oct 30, 2023

The default Jinja2 esaping behavior depends on the template type. http behaves differently from file.

If an unsafe string value from sceptre_user_data contains a double quote character (") and the Jinja2 template is a:

  • file type then Jinja2 writes "
  • http type then Jinja2 writes &34;

The stack group config has a j2_environment property to control the behavior of Jinja2. The autoescape subproperty overrides the default escaping behavior. Set it to false to make all templates acts like the file type default. Set it to true to make all templates act like the http type default.

The autoescape feature is undocumented.

Expand the j2_environment documentation to describe the autoescape behavior with examples. Adapt examples from my demo repo that shows the behavior for different permutations of Sceptre settings.

image

Case: file-autoescape-false/test.yaml
---
Unsafe: "Hello, world!"
Safe: "Hello, world!"

Case: file-autoescape-true/test.yaml
---
Unsafe: "Hello, world!"
Safe: "Hello, world!"

Case: file/test.yaml
---
Unsafe: "Hello, world!"
Safe: "Hello, world!"

Case: http-autoescape-false/test.yaml
---
Unsafe: "Hello, world!"
Safe: "Hello, world!"

Case: http-autoescape-true/test.yaml
---
Unsafe: "Hello, world!"
Safe: "Hello, world!"

Case: http-github/test.yaml
---
Unsafe: "Hello, world!"
Safe: "Hello, world!"

Case: http-local/test.yaml
---
Unsafe: "Hello, world!"
Safe: "Hello, world!"

(The demo repo is named sceptre-http-jinja2-bug because I believe varying the default Jinja2 behavior by the template type to be a bug. I don't know enough yet about the internals to propose a fix, but we can at least document it and show how to work around it.)


Recently discused in Slack: https://og-aws.slack.com/archives/C01JNN8RGBB/p1697802872492689

First discussed here: https://github.com/orgs/Sceptre/discussions/1238

@jfalkenstein
Copy link
Contributor

Hey @iainelder, I think we'd all be happy to review a PR that clarifies our docs on this subject. I'd say put up a draft and we can engage with you about wording from there.

@alexharv074
Copy link
Contributor

alexharv074 commented Nov 30, 2023

These examples aren't helping me much, although I would welcome them as a PR that adds unit tests that illustrate the confusing behaviour. I did follow the links to @zaro0508 's comment here
https://github.com/orgs/Sceptre/discussions/1238#discussioncomment-2891480

Probably we also just need to make it clearer in the docs that there might be scenarios where you want to add

j2_environment:
   autoescape: False

?

@iainelder
Copy link
Contributor Author

iainelder commented Dec 16, 2023

Probably we also just need to make it clearer in the docs

In the v4.3.0 docs the StackGroup Config page has the only discussion of j2_environment. It doesn't mention autoescape at all.

I think Jinja2 templates deserve their own page. That seems like the right place to detail the j2_environment setting and give enough examples to make it clear. One of the examples should show where you would want to control the autoescape setting, We need to call all out any gotchas such as the different default behaviors of the file and http template types.

These examples aren't helping me much

I'll grant that the output from the examples isn't clear without reading the code. The point there was to have a way to reproduce the behavior and to rule out possible PEBKAC errors and check my own understanding. Some of them might be helpful in the documentation, and the rest can be thrown away.

I would welcome them as a PR that adds unit tests that illustrate the confusing behaviour

I wouldn't bother yet. Only a developer would think to look there. The main problem here is a general lack of documentation on the Jinja2 templating feature.

I think there's a bug in here somewhere, but the behavior might be deliberate. I'd like to understand the spec before nailing things down with tests.

@alexharv074
Copy link
Contributor

@iainelder What kind of docs do you propose in this project that do not overlap with what is already documented in https://jinja.palletsprojects.com/en/3.1.x/

Compare the Jinja2 docs in Ansible
https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_templating.html

It largely just redirects you to the official Jinja2 docs. I think that's similar here?
https://docs.sceptre-project.org/dev/docs/templates.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants