Very cool. I'll have to check this out. That profile tool looks interesting too. I've been using the rack-mini-profiler gem but it might be a good idea to do a deeper dive.
That Rails has an escape_once method is a big part of the reason I stopped using Rails. If you think "double escaping" is a problem to be solved by creating a helper method that won't escape what's already escaped, now you have two problems.
EDIT TO ELABORATE:
Problem 1 is the original problem, that parts of your code do not agree upon what a particular string represents. This is the "strings problem," the mother of XSS and injection vulnerabilities. [1]
Problem 2 is that the escape_once method papers over these problems, making them harder to detect, and preventing you from hunting down the logic errors that cause them. (Since these errors often occur in upstream code, you need to find them before they execute to be safe, which is why compile-time methods [2] work best.)
That's not the problem. The amount of work done scanning short strings is completely irrelevant. Using escape_once is a problem because it shows that you have no clue what strings in your codebase are escaped and what are not. If you knew, it would be completely pointless. Instead, people resort to cargo-cult like solutions "hey let's escape everything again here just to be sure, that's gotta catch them all".
I've now worked a while in Yesod (Haskell), and I now feel very strongly that this is something they got right. Unescaped strings and html strings should be two completely separate types that can't be cast to each other, except by using transforming functions that clearly define intent.
I could actually see an argument to subclass String for this kind of clarity. I don't know if it'd solve it (and I'm likely not to try it out since I don't have an active Ruby project at all right now, let alone an active Rails project), but someone might try that?
The article should have pointed out that Github is running Rails 2.3, and that this is where the problem exists.
Rails 3.x has reworked the entire escaping situation and now avoids the re-escaping trap (strings must be flagged as "HTML safe", otherwise they are escaped on final injection into a document). It still has escape_once, but it's not used by Rails itself.
You are of course right to blame 2.x for its flaws, but let's not blame entire projects for problems that have been fixed. (Not that Rails 3.x does not have other egregious inefficiencies, but that's another story.)
I stopped using Rails not so much because of the existence of the escape_once method but because of what its introduction in 2006, and its continued use in mainstream Rails methods (until 2010 when Rails 3.0 was released), said about the direction of the project.
What did it say about the direction of the project, in your opinion? I can see how one might conclude that the developers are idiots based on that method, but I have to say that what I get out of Rails vastly outweighs that stupidity.
Windows users only missed out on the first two games when they came out. EV Nova (the third game) is available for Windows and the first two games are available as free plugins for Nova.
"Just by replacing the escaping function with a more optimized one, we've reduced the average request time by 45ms, and we're allocating 20,000 less Ruby objects per request. That was a lot of escaped HTML right there!"
I generally stay away from web development so forgive me if this one is obvious, but why does so much text need to have HTML escaping performed in order to render the page? Also is there a way to quantify how much text that is? Like a few K per page or a few hundred K?
`phillmv has identified the biggest concern for sure, but more broadly speaking, you have issues whenever you cross one of two boundaries: 1) escaping context [characters get escaped differently in URLs, JavaScript, HTML, etc.] and 2) user-defined content.
The latter is the source of XSS attacks, but the former is a pain too. Let's say that you run a stock photo website, and your database has images and captions. You decide you want to use captions for the alt attribute of your images, and one of your captions is Cat saying "meow!". If you blindly insert that caption into your HTML without escaping, you could end up with
<img src="cat.jpg" alt="Cat saying "meow!"">
And now you've got crappy HTML on your hands. It's the same principle as XSS attacks, but you do it to yourself with less disastrous (but still unwanted) effects.
EDIT: Hm, seems like the comment I referenced is now gone? `phillmv pointed out that cross-site scripting (XSS) is an important reason to use escaping appropriately (and often widely).
Note that in the case of Rails 2.3 (which the article describes), Rails doesn't know if post.title, post_url() or "post_link" have been escaped or not. It originates at a time when the Rails people were fairly lax and ignorant about encodings and sanitization in general.
For the post.title bit, the link_to method just passes it along, assuming it's valid HTML. Which means it's an injection point if the post was user-submitted. The URL and the {class: "post_link"} are passed through escape_once, described in the article.
As you can see, the amount of data that must potentially be sanitized/encoded can be a lot; lots of small pieces of text that add up to a lot of overhead.
Rails 3.x fixes this by assuming that no strings contain sanitized HTML; therefore, when you insert data into a template, it will automatically escape everything. This is good. If you know that something is already sanitized, you can declare it as such:
= link_to post.title.html_safe
This flags the string as safe, and link_to will not need to escape it.
My guess is that it has something to do with the fact that Github is made up almost entirely of user generated content. Just load a Github project page. From top to bottom, you have:
* Username in the header (which is probably validated with a specific format,
so probably doesn't need escaping).
* Project summary entered when the project is created.
* Drop-down with list of branch and tag names.
* Latest commit message.
* List of names for all folders and files tracked in git for the project.
* Readme text.
That's just the homepage for a project, and each one of those things could potentially contain malicious HTML or JS, meaning it should be escaped before rendered on the page.
To people horrified by the Rails code, keep in mind that Github is running Rails 2.3, which is very old and near the end of LTS. Rails 3.x has reworked the entire escaping situation and now avoids the re-escaping trap (strings must be flagged as "HTML safe", otherwise they are escaped on final injection into a document).
So, what's their upgrade path situation? Are they going to try to hold out indefinitely on 2.3? Upgrade at the last possible moment? (Ick) Switch out of Rails when the situation becomes intolerable?
I'm always amazed by these startups doing what _seems_ like really simple optimizations and reaping these enormous benefits. If your tools support profiling, you should at least give it a shot once in a while. The fruit are hanging low indeed if Github can reap this kind of reward with this small a tweak.
40 comments
[ 3.0 ms ] story [ 76.5 ms ] threadAs always GH, thanks for sharing.
EDIT TO ELABORATE:
Problem 1 is the original problem, that parts of your code do not agree upon what a particular string represents. This is the "strings problem," the mother of XSS and injection vulnerabilities. [1]
Problem 2 is that the escape_once method papers over these problems, making them harder to detect, and preventing you from hunting down the logic errors that cause them. (Since these errors often occur in upstream code, you need to find them before they execute to be safe, which is why compile-time methods [2] work best.)
[1] http://blog.moertel.com/posts/2007-08-15-a-bright-future-sec...
[2] http://blog.moertel.com/posts/2006-10-18-a-type-based-soluti...
I've now worked a while in Yesod (Haskell), and I now feel very strongly that this is something they got right. Unescaped strings and html strings should be two completely separate types that can't be cast to each other, except by using transforming functions that clearly define intent.
Rails 3.x has reworked the entire escaping situation and now avoids the re-escaping trap (strings must be flagged as "HTML safe", otherwise they are escaped on final injection into a document). It still has escape_once, but it's not used by Rails itself.
You are of course right to blame 2.x for its flaws, but let's not blame entire projects for problems that have been fixed. (Not that Rails 3.x does not have other egregious inefficiencies, but that's another story.)
Small "let's play" video: https://www.youtube.com/watch?v=4VX6TZEfqf4
I generally stay away from web development so forgive me if this one is obvious, but why does so much text need to have HTML escaping performed in order to render the page? Also is there a way to quantify how much text that is? Like a few K per page or a few hundred K?
The latter is the source of XSS attacks, but the former is a pain too. Let's say that you run a stock photo website, and your database has images and captions. You decide you want to use captions for the alt attribute of your images, and one of your captions is Cat saying "meow!". If you blindly insert that caption into your HTML without escaping, you could end up with
And now you've got crappy HTML on your hands. It's the same principle as XSS attacks, but you do it to yourself with less disastrous (but still unwanted) effects.EDIT: Hm, seems like the comment I referenced is now gone? `phillmv pointed out that cross-site scripting (XSS) is an important reason to use escaping appropriately (and often widely).
For the post.title bit, the link_to method just passes it along, assuming it's valid HTML. Which means it's an injection point if the post was user-submitted. The URL and the {class: "post_link"} are passed through escape_once, described in the article.
As you can see, the amount of data that must potentially be sanitized/encoded can be a lot; lots of small pieces of text that add up to a lot of overhead.
Rails 3.x fixes this by assuming that no strings contain sanitized HTML; therefore, when you insert data into a template, it will automatically escape everything. This is good. If you know that something is already sanitized, you can declare it as such:
This flags the string as safe, and link_to will not need to escape it.Wow, that is...a lot...is that mostly rails or is that in user code?
http://meta.discourse.org/t/tuning-ruby-and-rails-for-discou...
Scroll down and you'll see that on an average request, Discourse eats up 230,000 strings [which are all objects in Ruby] per request.
The entire thread is about relaxing the garbage collector so that it won't run until the request is over.