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 entities in html() output #221

Merged
merged 2 commits into from
Aug 6, 2021
Merged

Conversation

jcushman
Copy link
Contributor

@jcushman jcushman commented Aug 2, 2021

Currently the .html() method does not escape the tag.text portion of the response:

In [1]: pq('<div>encoded &lt;script&gt; tag</div>').html()
Out[1]: 'encoded <script> tag'

This PR updates .html() to call the builtin html.escape() function on tag.text. We can set quote=False because we're escaping element text rather than attribute text.

Fixes #205 #218 #200 #178

@jcushman jcushman force-pushed the escape-html branch 2 times, most recently from 1bcf107 to 5eabedb Compare August 2, 2021 20:51
@jcushman
Copy link
Contributor Author

jcushman commented Aug 2, 2021

Addressing a question @gawel asked on #205, we don't need any additional escaping for the children, because that's handled by etree.tostring(). I made sure the new test covers that, though.

I also had to tweak the textarea test since calling.val(text) sets .text, so fetching that value now comes back escaped.

(I messed a little bit with changing how .val() and .text() work in general with textareas, to make it more like jQuery where angle brackets seem to be escaped separately from ampersands, but decided that was too invasive for this PR.)

@gawel
Copy link
Owner

gawel commented Aug 5, 2021

Seems fine. Can you just add an entry to the changelog ? thanks

@jcushman
Copy link
Contributor Author

jcushman commented Aug 5, 2021

Changelog updated.

@gawel gawel merged commit e92b8b7 into gawel:master Aug 6, 2021
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.

.html() fails to escape initial html entities
2 participants