I've submitted a small patch to make Rails behave properly with the
Erubis <%== %> construct. For some reason the current behaviour of
that tag in Rails 3 is to escape the contents _twice_ which is
probably a bug.
I offer three suggestions why this is a good idea:
- The syntax is cleaner. It can avoid a lot of .html_safe and raw in
your views. I especially like the conciseness of <%=== '<b>Alert</b>'
if level<0 %> better then the alternative with .html_safe.
- It performs slightly better since it saves a method call and we can
concat a String directly instead of coercing everything to a
SafeBuffer
- It re-enables the ability of Erubis to behave like Erb in Rails 2
which allows for easier upgrading (You can pass :escape => true to a
new Erubis instance or glabally replace the <%= with <%==)
I offer three suggestions why this is a good idea:
- The syntax is cleaner. It can avoid a lot of .html_safe and raw in
your views. I especially like the conciseness of <%=== '<b>Alert</b>'
if level<0 %> better then the alternative with .html_safe.
The only concern I have is that the syntax here has subtly different
isn't the same. In erubis strings get escaped irrespective of whether
they've been escaped before, in rails it's an idempotent escaping
function so <%= h(x) and <%= x are identical.
So if we override these additional operators we'll be giving them our
own meaning, not increasing compatibility with erubis. Having said
that, they're completely broken now so I think this is probably a good
idea.
- It performs slightly better since it saves a method call and we can
concat a String directly instead of coercing everything to a
SafeBuffer
We actually used to do something similar, but it was removed in this commit:
Was added here:
Not sure whether that was intentional though.
- It re-enables the ability of Erubis to behave like Erb in Rails 2
which allows for easier upgrading (You can pass :escape => true to a
new Erubis instance or glabally replace the <%= with <%==)
I don't actually consider this a feature to be honest. Yes it's
annoying having to mark strings as safe to prevent them being escaped,
but at least you notice it. Contrast this with the previous behaviour
where things 'mostly worked' until someone tried to attack your site.
It's annoying and fiddly, but compared to the alternative situation
where a single missing h can mean you give away root access to your
servers and all your customer's passwords[1], I think the pain is
justified.
It's currently a monkey patch but if there is a chance of this being
accepted I can turn it into a proper patch with tests. Comments
welcome.
I like the idea of making <%== work as a synonym for <%= raw, upload a
proper patch for it and I'll take a look.
Ok, I've uploaded a real patch (hope it's OK, it's my first attempt at
using git).
Do you know the reason why the commits you mentioned above were
removed?
With regards to the .html_safe being "annoying but good for you":
that's the same argument people use about static typing being
"annoying but good for you" and I don't buy that argument either. You
can still accidentally get non-escaped HTML in your code because of
string interpolation or because you marked an entire helper as safe.
The proper way to test for XSS scripting attacks is to put bad tags in
all fixtures and simulated user input and running the response body
though html tidy or similar. Similarly dynamic typing can be used by
responsible programmers by having appropriate tests.
Anyway, I've only changed the behaviour of the <%== tag and feeding an
":escape => :off" option to Erubis can only be done by monkey patching
Rails so that means a clueless programmer who needs to be protected
from himself won't run into it quickly.
Not really. Comparing html_safe to whitelisting is not correct because
whitelisting provides an adequate protection and html_safe does not.
You will still need additional security tests because insecure things
can still get through with string interpolations. You could even claim
there is a danger that people are lulled into a false sense of
security with html_safe. And that is how I meant the analogy of static
typing versus dynamic typing: people claim that you should use static
typing because that will allow you to find bugs before they go into
production. That's true to a point but it certainly does not mean that
you are somehow relieved of doing proper testing because you are using
a statically typed language. The analogy also works in reverse: if you
are doing proper testing than using a dynamically typed language is
not an issue, and similarly if you are testing the right way then it's
OK to do the escaping yourself.
Not really. Comparing html_safe to whitelisting is not correct because
whitelisting provides an adequate protection and html_safe does not.
Safe strings *are* whitelists, by calling html_safe on a string you
are instructing the erb handler that that string is to be considered
safe. Every string which isn't marked as safe, is considered unsafe
and is escaped. That's a whitelist.
You will still need additional security tests because insecure things
can still get through with string interpolations.
Newly interpolated strings aren't safe until you mark them as such by
calling html_safe.
and similarly if you are testing the right way then it's
OK to do the escaping yourself.
Similarly if you're testing the right way you'll notice that your
values are being escaped unexpectedly and there's no issue.
The erb handler is now secure by default, that's a feature and the
minor pain it imposes is nothing compared to the danger of an XSS hole
in your application because your forgot that one character.
That's fine in theory but in the last few weeks of using Rails 3 I've
still noticed unescaped HTML slipping though because so many html_safe
statements are needed that you make mistakes, or you mark an entire
helper safe that gets it content from somewhere else, or you marked a
string safe and then interpolated an unsafe item into it.
Anyway, it's all besides the point because as I started out saying: I
am NOT against on-by-default escaping, I just think there is no reason
not to provide an elegant way to use unescaped text. You still need
unescaped text, for example when you are generating a plain text e-
mail. My e-mail templates look much cleaner with <%== everywhere
instead of all the html_safe stuff.
Anyway, it's all besides the point because as I started out saying: I
am NOT against on-by-default escaping, I just think there is no reason
not to provide an elegant way to use unescaped text.
Agreed, I've applied it to master and 3-0-stable :).
