Safe HTML by default

[pupeno]

It would be nice if Hiccup escaped strings by default, only requiring something special to avoid escaping, as to make it very hard to make apps that are XSS exploitable.

[weavejester]

This is planned, but requires a rewrite of the entire library. The rewrite is currently in the 2.0 branch.

[pupeno]

I started playing around this, modifying hiccup to try to achieve this and I can see it's far from trivial.

[weavejester]

Hiccup 1.0 uses strings for performance. Unfortunately this means that there's no distinction between a safe and unsafe string.

[pupeno]

Selmer considers all string to be unsafe except does inside a vector like this: [:safe string]. Wouldn't something like this work?

[weavejester]

No, because Hiccup compiles into expressions of string concatenation. This makes it fast, but relies on the assumption that it's just dealing with strings. For example:

(html [:span {} (foo "bar")])

Compiles to:

(str "<span>" (foo "bar") "</span>")

[gyim]

What do you think of this solution? gyim@535282b2e98cdc8bc48314ade8cc181ea2815dd1 ?

The basic idea is to introduce a SafeString class that is not escaped, everything else is. This class can be easily plugged into the HtmlRendered protocol. I am not sure why this solution came up before: is it too much of a hack? Or did I overlook an important use case?

I think the most common use case for NOT escaping a string is when someone has multiple partial templates, each "pre-compiled" with the (html) macro for performance. So I introduced a (html {:partial? true}) option which returns a SafeString. This way the user rarely has to deal with enabling/disabling escaping directly, and the :partial? true option suggests that the return value of this function is "not the real thing", just something that is useful inside a "real" template. But this is just a syntactic sugar, using (safe-str (html ...)) would also work.

[weavejester]

@gyim, that solution isn't safe HTML by default, it's safe HTML if you set an option flag. I think if we're going to do this, we should do it properly, instead of trying to come up with partial implementations in an attempt to maintain backward compatibility.

We already know what safe HTML by default looks like:

(html [:p "<foo>"])
=> #hiccup.core/RawString["<p>&lt;foo&gt;</p>"]

(str (html [:p "<foo>"]))
=> "<p>&lt;foo&gt;</p>"

(html [:p (html [:span "foo"])])
=> #hiccup.core/RawString["<p><span>foo</span></p>"]

We just need to implement it. At this point I'd accept a patch that does just that and nothing else.

[gyim]

Makes perfect sense, I just did not want to introduce any breaking changes. If (html) returns a RawString instead of a String and the caller makes a string operation on it, it could cause a crash in existing code. But I agree that safe HTML by default is the right thing to do, so it could be a reasonable tradeoff.

I will rework the patch and send a PR.

[weavejester]

Fixed by #122.