Handle ?url query string for css-fixme, #847

[hallvors]

No description provided.

[miketaylr]

Looks great! Any feedback, @karlcow?

[hallvors]

Should be OK to merge, only failure on Travis is the one about wrong selector in Github test which is fixed in #851

[miketaylr]

This branch has conflicts that must be resolved

@hallvors can you rebase and fix conflicts before we pull this in, please?

[miketaylr]

The problem is that TEXTAREA will render those entities in the content - literally, so escaping everything will mangle any CSS code that contains <>&. Hence div > p would show up as div > p in the generated output.

Is that true? Every browser I tested does the "right" thing here, and if I copy and paste I get unescaped entities (which is what I would expect).

data:text/html,<textarea>&lt;script&gt;alert(0)&lt;/script&gt;</textarea>:

I'd feel a lot better if we just escape all instances of <, > and & -- XSS paranoia seems like a good policy.

[miketaylr]

(I guess this PR is out of sync with #850, which was just merged -- rebasing against master should help here. These patches still have the inline click handlers, etc.)

[miketaylr]

https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet#RULE_.231_-_HTML_Escape_Before_Inserting_Untrusted_Data_into_HTML_Element_Content recommends escaping ' and " as well.

[hallvors]

Is that true? Every browser I tested does the "right" thing here

Strange. I'm sure I saw the opposite while testing in Orlando - I'm even sure @karlcow was sitting next to me and saw it too. Had I somehow added a double escaping? :-o

[hallvors]

We're using the Flask templating engine and it's escaping input by default (unless you jump through some hoops to avoid that by passing in Markup class objects or something). So yes, we ended up with a double escape which confused me.

[hallvors]

@miketaylr should be fine to merge now - not sure what the two failures on Travis are about but they are definitely not related to this patch..

[miketaylr]

We're using the Flask templating engine and it's escaping input by default (unless you jump through some hoops to avoid that by passing in Markup class objects or something). So yes, we ended up with a double escape which confused me.

Ah yeah, that seems confusing, thanks for looking into it. 👍

not sure what the two failures on Travis are about but they are definitely not related to this patch

Looks like the PR failure was a pep8 failure. But I see you've fixed it so the push build passed. Let me just retrigger the PR build then we should be good to merge.

[miketaylr]

(oops, was looking at wrong build. yes, unrelated failures indeed)