weavejester / hiccup

Fast library for rendering HTML in Clojure
http://weavejester.github.io/hiccup
Eclipse Public License 1.0
2.68k stars 174 forks source link

Escape special characters when generating HTML strings #120

Closed EwenG closed 8 years ago

EwenG commented 8 years ago

Hi,

This is my attempt at making hiccup to automatically escape strings.

I added a dynamic var: hiccup.util/*no-escape-strings*. The dynamic var is bound to an empty set when using a top level html macro. The set compares its content using identical? instead of equal. When a html macro is called it wraps its result into a new String object and add it to the dynamic var. Wrapping in a new String object is done because two literal strings may have the same identity due to jvm caching. When a string is emitted, it is escaped only if it is not in the dynamic var. It is not escaped otherwise.

Note that a string generated by a html macro is escaped if it is not used dynamically, although that can easily be worked around manually:

(def s (html [:div]))
(html s) => "<div></div>"
(html (without-escape-html s)) => "<div></div>"
weavejester commented 8 years ago

Thanks for the patch. However, there are a couple of problems that mean that I can't merge it.

The first problem is that the way to optimize code in Hiccup is to wrap it in html, which precompiles where possible. With your patch you'd have to wrap every function that produces precompiled output with without-escape-html. This would quickly become very verbose.

The second problem is that this is a breaking change, so we'd need to go to version 2.0. However, there are already plans in progress for version 2.0 that solve this problem in a different way. I just haven't had the time to write the new compiler.

EwenG commented 8 years ago

No you don't need to wrap every output of html in without-escape-html.

(defhtml foo [] [:div])
(html (foo)) => "<div></div>"
weavejester commented 8 years ago

Ah, I see. You're using local bindings to distinguish between strings produced by the html macro from strings produced in other ways. However, that's still a leaky abstraction:

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

The only way to solve this is to introduce some way of distinguishing strings and compiled HTML. That necessitates a new type, for instance:

(deftype HtmlString [string]
  Object
  (toString [_] string))

(html [:div])
=> #hiccup.core.HtmlString["<div></div>"]
(str (html [:div]))
=> "<div></div>"

The plan was for Hiccup 2.0 to solve the main problems with 1.0, so:

  1. Automatically escaping strings
  2. Splitting the project up into compiler and support libraries
  3. ClojureScript support for support library
  4. Pretty printing HTML option

However, now my feeling is that this is too much to tackle in one go. My inclination is therefore to go for just 1 and 2 for version 2.0, and sort out the rest in a later release.

The problem for me is that Hiccup doesn't currently intersect with any of the work I'm doing right now, so I haven't had time to implement it.

EwenG commented 8 years ago

I personally don't think a rewrite is worth the trouble.

When writing (let [foo (html [:div])] (html [:div foo])), you can't even use a parameter in the foo template without wrapping it in a function.

What about removing the html macro from the public API in favor of defhtml? (let [foo (fn-html [] [:div])] (defhtml x [] [:div (foo)])) would be correct.

weavejester commented 8 years ago

Your suggestion is a workaround, but it doesn't remove the fundamental leakiness from the abstraction, and adds some additional complexity. It's also a breaking change, so we'd need to move up to version 2.0 anyway, and if we're going to do that, we might as well fix things properly.

EwenG commented 8 years ago

All right, I close the pull request then.