From 169125e96d90259d2eb8a019ec7500ab0a58a757 Mon Sep 17 00:00:00 2001
From: Neil Lalonde <neillalonde@gmail.com>
Date: Fri, 7 Jun 2013 14:17:12 -0400
Subject: [PATCH] Fix a case where a random topic with null slug will be
 rendered instead of 404

---
 app/controllers/topics_controller.rb       |  2 +-
 app/models/topic.rb                        |  1 +
 spec/controllers/topics_controller_spec.rb | 17 +++++++++++++++++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb
index 7e8f63da58a..055215bfcbf 100644
--- a/app/controllers/topics_controller.rb
+++ b/app/controllers/topics_controller.rb
@@ -29,7 +29,7 @@ class TopicsController < ApplicationController
     begin
       @topic_view = TopicView.new(params[:id] || params[:topic_id], current_user, opts)
     rescue Discourse::NotFound
-      topic = Topic.where(slug: params[:id]).first
+      topic = Topic.where(slug: params[:id]).first if params[:id]
       raise Discourse::NotFound unless topic
       return redirect_to(topic.relative_url)
     end
diff --git a/app/models/topic.rb b/app/models/topic.rb
index 1d632527a6d..cc8f1568b81 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -494,6 +494,7 @@ class Topic < ActiveRecord::Base
     TopicUser.starred_since(sinceDaysAgo).by_date_starred.count
   end
 
+  # Even if the slug column in the database is null, topic.slug will return something:
   def slug
     unless slug = read_attribute(:slug)
       return '' unless title.present?
diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb
index 59447e69dcf..d78bb3d23f5 100644
--- a/spec/controllers/topics_controller_spec.rb
+++ b/spec/controllers/topics_controller_spec.rb
@@ -383,6 +383,23 @@ describe TopicsController do
       expect(response.status).to eq(404)
     end
 
+    it 'returns a 404 when slug and topic id do not match a topic' do
+      xhr :get, :show, topic_id: 123123, slug: 'topic-that-is-made-up'
+      expect(response.status).to eq(404)
+    end
+
+    context 'a topic with nil slug exists' do
+      before do
+        @nil_slug_topic = Fabricate(:topic)
+        Topic.connection.execute("update topics set slug=null where id = #{@nil_slug_topic.id}") # can't find a way to set slug column to null using the model
+      end
+
+      it 'returns a 404 when slug and topic id do not match a topic' do
+        xhr :get, :show, topic_id: 123123, slug: 'topic-that-is-made-up'
+        expect(response.status).to eq(404)
+      end
+    end
+
     it 'records a view' do
       lambda { xhr :get, :show, topic_id: topic.id, slug: topic.slug }.should change(View, :count).by(1)
     end