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

turn on autoescape for flask.templating.render_template_string #1515

Closed
wants to merge 2 commits into from

Conversation

@alanhamlett
Copy link
Contributor

commented Jun 24, 2015

Previous pull request was #1176

@ThiefMaster

This comment has been minimized.

Copy link
Member

commented Jun 24, 2015

@alanhamlett alanhamlett changed the title turn on autoescape by default for flask.templating.render_template_string turn on autoescape for flask.templating.render_template_string Jun 24, 2015

@alanhamlett

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2015

@ThiefMaster sure, done. What's the eta on this getting merged, I have other pull requests I would like to submit as well.

@@ -1460,7 +1460,7 @@ def handle_http_exception(self, e):
# those unchanged as errors
if e.code is None:
return e

This comment has been minimized.

Copy link
@untitaker

untitaker Jun 30, 2015

Member

I apprechiate this, but could you make it a separate commit?

This comment has been minimized.

Copy link
@alanhamlett

alanhamlett Jun 30, 2015

Author Contributor

Sorry, my Vim automatically does that. Fixing it asap.

This comment has been minimized.

Copy link
@alanhamlett

alanhamlett Jun 30, 2015

Author Contributor

@untitaker separated the changes.

@untitaker

This comment has been minimized.

Copy link
Member

commented Jun 30, 2015

I'm not sure how, but I think the current behavior should be better documented.

@untitaker

This comment has been minimized.

Copy link
Member

commented Jun 30, 2015

In addition to some docs, could you add a changelog and a note in the Upgrading document?

@alanhamlett

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2015

I've updated the docstring which will show up in this doc, but I should regenerate docs or add more info somewhere?

For changelog, should I add to version 0.10.2 or 1.0?

@untitaker

This comment has been minimized.

Copy link
Member

commented Jun 30, 2015

1.0

@alanhamlett

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2015

Finished all the changes, everything looks good?

@ThiefMaster

This comment has been minimized.

Copy link
Member

commented Jun 30, 2015

I would squash all the commits (besides the whitespace fixes) into a single one

for the message i'd use e.g. this:

Enable autoescape for `render_template_string

that way you stay below 51 chars and it's still clear what it changes

@untitaker

This comment has been minimized.

Copy link
Member

commented Jun 30, 2015

I've pinged mitsuhiko on IRC if there's a good reason for the current behavior. I assume not and will merge this if he doesn't respond.

@alanhamlett

This comment has been minimized.

Copy link
Contributor Author

commented Jun 30, 2015

@ThiefMaster squashed.

@ThiefMaster

This comment has been minimized.

Copy link
Member

commented Jun 30, 2015

thanks! (and oops, looks like i forgot the trailing backtick in the commit message suggestion)

@untitaker

This comment has been minimized.

Copy link
Member

commented Jul 4, 2015

Thanks, rebased and merged!

@untitaker untitaker closed this in db09c67 Jul 4, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.