Audit .html() calls take #2#5175
Conversation
|
@minrk I moved the todos into the |
There was a problem hiding this comment.
This suggests that this method should be made safe in a future fix, which I do not think is true. set_rendered ought to be treated as an unsafe method, and any user html should be cleaned before calling set_rendered.
|
@minrk the last commit treats |
There was a problem hiding this comment.
this block should go back to calling set_rendered
There was a problem hiding this comment.
Is there a possibility that e.toString() can have use injected html? Some sort of manipulation of the input such that it throws an error with user content (should I add a TODO here?)
There was a problem hiding this comment.
Don't revert the whole change, you can still use .text here, just use set_rendered instead of rendered.append (since rendered is undefined now).
|
I'm going to give this a test locally to make sure things still work. |
|
This works locally |
|
I think this looks good. Have you gone through all the |
Yes, there are quite a few |
There was a problem hiding this comment.
Is this missing the leading <br/> tag from above?
There was a problem hiding this comment.
don't need br because there are multiple divs, where there was one div with explicit br before.
|
A few comments, but probably ready to go. |
Audit .html() calls take ipython#2
3 participants
sequel to #5041
closes #5034