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-html should escape apostrophe's #92

Closed msmith-techempower closed 10 years ago

msmith-techempower commented 10 years ago
(defn escape-html
  "Change special characters into HTML character entities."
  [text]
  (.. ^String (as-str text)
    (replace "&" "&")
    (replace "<" "&lt;")
    (replace ">" "&gt;")
    (replace "\"" "&quot;")
    (replace "'" "&apos;")))
weavejester commented 10 years ago

Why?

msmith-techempower commented 10 years ago

Most libraries that handle escaping do this; here is a simple mustache example to illustrate why:

<a href='{{untrustedData}}'>Click Me</a>

Apostrophes are technically valid replacements for quotes in this example, and if the untrusted data does not escape apostrophes and the untrusted data happens to be #' onclick='while(true){alert("hahahaha");} you have opened your page to an attack.

Basically, it is just considered best practice to escape apostrophes as well as quotes, amps, and lg/gt.

weavejester commented 10 years ago

So the use case is if Hiccup is used in conjunction with a library that uses single quotes?

I think we'll need to take into account the special case of HTML4, which doesn't have &apos;

msmith-techempower commented 10 years ago

Full disclosure - I am updating a test that uses Hiccup to produce HTML output, and I validate the output for our benchmarks; I am running into the problem that we consider the test to have failed validation if the apostrophe is not escaped (again, web's "best practices" is rule of law for our validations).

HTML4 should have &#x27;, &#39;, or &#039; would you be willing to use one of those?

weavejester commented 10 years ago

Yep, it's recommended to use &#39; by the w3, though of course all the examples you give are just the same character in different bases.

Something like:

(replace "'" (if (= *html-mode* :sgml) "&#39;" "&apos;"))
msmith-techempower commented 10 years ago

Sounds good to me.

weavejester commented 10 years ago

Fixed in 9d39730