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

escape special HTML chars #8

Merged
merged 1 commit into from
Mar 26, 2019
Merged

escape special HTML chars #8

merged 1 commit into from
Mar 26, 2019

Conversation

agroszer
Copy link
Contributor

I'm very tempted to add escape also to GetAttrColumn.renderCell, GetItemColumn.renderCell and I18nGetAttrColumn.renderCell

opinions? anyone?

@mgedmin
Copy link
Member

mgedmin commented Mar 24, 2019

I'm woefully out of context, but IMnsHO all of zopefoundation stuff should start using MarkupSafe ASAP and make escaping mistakes impossible (or at least harmless).

@agroszer
Copy link
Contributor Author

Well the problem was simple, someone added a content object with the __name__ = '14" monitor' selecting with the checkbox silently did not work...
This is the 'quick' fix for it, I don't think switching all of zopefoundation stuff and our code is within easy reach.
OTOH most zopefoundation stuff that uses ZPT/TAL is much safer because there you need to do extra work to NOT get escaped strings. Maybe z3c.table gets it wrong by doing direct string operations?

Anyway, merge or change something?

Copy link
Member

@mgedmin mgedmin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm feeling a bit +0 for this. It's confusing which methods are expected to return raw HTML, and which methods can safely return user input without worrying about escaping. And almost none of that is documented in the interface docstrings.

(This is one of the reasons I like MarkupSafe so much: the escaped/raw decision belongs with the data, not in hidden assumptions about method calling conventions.)

src/z3c/table/column.py Show resolved Hide resolved
src/z3c/table/column.py Show resolved Hide resolved
src/z3c/table/column.py Outdated Show resolved Hide resolved
@mgedmin
Copy link
Member

mgedmin commented Mar 26, 2019

Thank you!

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

Successfully merging this pull request may close these issues.

None yet

2 participants