From e3bbb2c8bb7847447ba79a36a290d78dab8f35c3 Mon Sep 17 00:00:00 2001
From: Robin Ward <robin.ward@gmail.com>
Date: Thu, 18 Jul 2013 12:03:09 -0400
Subject: [PATCH] Never render an avatar img if we know the username is
 invalid.

---
 .../discourse/components/utilities.js         | 23 +++++++------
 .../javascripts/discourse/models/composer.js  |  1 -
 .../javascripts/discourse/views/post_view.js  |  4 +--
 test/javascripts/components/utilities_test.js | 32 +++++++++++++++++++
 test/javascripts/test_helper.js               |  1 +
 5 files changed, 48 insertions(+), 13 deletions(-)

diff --git a/app/assets/javascripts/discourse/components/utilities.js b/app/assets/javascripts/discourse/components/utilities.js
index a2eff614b..f13533865 100644
--- a/app/assets/javascripts/discourse/components/utilities.js
+++ b/app/assets/javascripts/discourse/components/utilities.js
@@ -49,20 +49,23 @@ Discourse.Utilities = {
 
   avatarUrl: function(username, size, template) {
     if (!username) return "";
-    size = Discourse.Utilities.translateSize(size);
-    var rawSize = (size * (window.devicePixelRatio || 1)).toFixed();
+    var rawSize = (Discourse.Utilities.translateSize(size) * (window.devicePixelRatio || 1)).toFixed();
+
+    if (username.match(/[^A-Za-z0-9_]/)) { return ""; }
     if (template) return template.replace(/\{size\}/g, rawSize);
-    return Discourse.getURL("/users/") + (username.toLowerCase()) + "/avatar/" + rawSize + "?__ws=" + (encodeURIComponent(Discourse.BaseUrl || ""));
+    return Discourse.getURL("/users/") + username.toLowerCase() + "/avatar/" + rawSize + "?__ws=" + encodeURIComponent(Discourse.BaseUrl || "");
   },
 
   avatarImg: function(options) {
-    var extraClasses, size, title, url;
-    size = Discourse.Utilities.translateSize(options.size);
-    title = options.title || "";
-    extraClasses = options.extraClasses || "";
-    url = Discourse.Utilities.avatarUrl(options.username, options.size, options.avatarTemplate);
-    return "<img width='" + size + "' height='" + size + "' src='" + url + "' class='avatar " +
-            (extraClasses || "") + "' title='" + (Handlebars.Utils.escapeExpression(title || "")) + "'>";
+    var size = Discourse.Utilities.translateSize(options.size);
+    var url = Discourse.Utilities.avatarUrl(options.username, options.size, options.avatarTemplate);
+
+    // We won't render an invalid url
+    if (Em.isEmpty(url)) { return ""; }
+
+    var classes = "avatar" + (options.extraClasses ? " " + options.extraClasses : "");
+    var title = (options.title) ? " title='" + Handlebars.Utils.escapeExpression(options.title || "") + "'" : "";
+    return "<img width='" + size + "' height='" + size + "' src='" + url + "' class='" + classes + "'" + title + ">";
   },
 
   tinyAvatar: function(username) {
diff --git a/app/assets/javascripts/discourse/models/composer.js b/app/assets/javascripts/discourse/models/composer.js
index be48bd408..8f471d9ce 100644
--- a/app/assets/javascripts/discourse/models/composer.js
+++ b/app/assets/javascripts/discourse/models/composer.js
@@ -469,7 +469,6 @@ Discourse.Composer = Discourse.Model.extend({
       }
     }
 
-    // Save callback
     var composer = this;
     return Ember.Deferred.promise(function(promise) {
       createdPost.save(function(result) {
diff --git a/app/assets/javascripts/discourse/views/post_view.js b/app/assets/javascripts/discourse/views/post_view.js
index 6c05a8f6d..961d5fad1 100644
--- a/app/assets/javascripts/discourse/views/post_view.js
+++ b/app/assets/javascripts/discourse/views/post_view.js
@@ -115,7 +115,7 @@ Discourse.PostView = Discourse.View.extend({
 
         // If it's the same topic as ours, build the URL from the topic object
         if (topic && topic.get('id') === topicId) {
-          navLink = "<a href='" + (topic.urlForPostNumber(postNumber)) + "' title='" + quoteTitle + "' class='back'></a>";
+          navLink = "<a href='" + topic.urlForPostNumber(postNumber) + "' title='" + quoteTitle + "' class='back'></a>";
         } else {
           // Made up slug should be replaced with canonical URL
           navLink = "<a href='" + Discourse.getURL("/t/via-quote/") + topicId + "/" + postNumber + "' title='" + quoteTitle + "' class='quote-other-topic'></a>";
@@ -123,7 +123,7 @@ Discourse.PostView = Discourse.View.extend({
 
       } else if (topic = this.get('controller.content')) {
         // assume the same topic
-        navLink = "<a href='" + (topic.urlForPostNumber(postNumber)) + "' title='" + quoteTitle + "' class='back'></a>";
+        navLink = "<a href='" + topic.urlForPostNumber(postNumber) + "' title='" + quoteTitle + "' class='back'></a>";
       }
     }
     // Only add the expand/contract control if it's not a full post
diff --git a/test/javascripts/components/utilities_test.js b/test/javascripts/components/utilities_test.js
index a7c3b1afb..20b4b1f2a 100644
--- a/test/javascripts/components/utilities_test.js
+++ b/test/javascripts/components/utilities_test.js
@@ -110,3 +110,35 @@ test("isAnImage", function() {
   ok(!utils.isAnImage("http://foo.bar/path/to/file.txt"));
   ok(!utils.isAnImage(""));
 });
+
+test("avatarUrl", function() {
+  blank(Discourse.Utilities.avatarUrl('', 'tiny'), "no avatar url returns blank");
+  blank(Discourse.Utilities.avatarUrl('this is not a username', 'tiny'), "invalid username returns blank");
+
+  equal(Discourse.Utilities.avatarUrl('eviltrout', 'tiny'), "/users/eviltrout/avatar/20?__ws=", "simple avatar url");
+  equal(Discourse.Utilities.avatarUrl('eviltrout', 'large'), "/users/eviltrout/avatar/45?__ws=", "different size");
+  equal(Discourse.Utilities.avatarUrl('EvilTrout', 'tiny'), "/users/eviltrout/avatar/20?__ws=", "lowercases username");
+  equal(Discourse.Utilities.avatarUrl('eviltrout', 'tiny', 'test{size}'), "test20", "replaces the size in a template");
+});
+
+test("avatarUrl with a baseUrl", function() {
+  Discourse.BaseUrl = "http://try.discourse.org";
+  equal(Discourse.Utilities.avatarUrl('eviltrout', 'tiny'), "/users/eviltrout/avatar/20?__ws=http%3A%2F%2Ftry.discourse.org", "simple avatar url");
+});
+
+test("avatarImg", function() {
+  equal(Discourse.Utilities.avatarImg({username: 'eviltrout', size: 'tiny'}),
+        "<img width='20' height='20' src='/users/eviltrout/avatar/20?__ws=' class='avatar'>",
+        "it returns the avatar html");
+
+  equal(Discourse.Utilities.avatarImg({username: 'eviltrout', size: 'tiny', title: 'evilest trout'}),
+        "<img width='20' height='20' src='/users/eviltrout/avatar/20?__ws=' class='avatar' title='evilest trout'>",
+        "it adds a title if supplied");
+
+  equal(Discourse.Utilities.avatarImg({username: 'eviltrout', size: 'tiny', extraClasses: 'evil fish'}),
+        "<img width='20' height='20' src='/users/eviltrout/avatar/20?__ws=' class='avatar evil fish'>",
+        "it adds extra classes if supplied");
+
+  blank(Discourse.Utilities.avatarImg({username: 'weird*username', size: 'tiny'}),
+        "it doesn't render avatars for invalid usernames");
+});
\ No newline at end of file
diff --git a/test/javascripts/test_helper.js b/test/javascripts/test_helper.js
index 075110661..376d89d7f 100644
--- a/test/javascripts/test_helper.js
+++ b/test/javascripts/test_helper.js
@@ -81,5 +81,6 @@ QUnit.testStart(function() {
   // Allow our tests to change site settings and have them reset before the next test
   Discourse.SiteSettings = jQuery.extend(true, {}, Discourse.SiteSettingsOriginal);
   Discourse.BaseUri = "/";
+  Discourse.BaseUrl = "";
 })