Fix a case where a random topic with null slug will be rendered instead of 404

This commit is contained in:
Neil Lalonde 2013-06-07 14:17:12 -04:00
parent 668a4a3042
commit 169125e96d
3 changed files with 19 additions and 1 deletions

View File

@ -29,7 +29,7 @@ class TopicsController < ApplicationController
begin begin
@topic_view = TopicView.new(params[:id] || params[:topic_id], current_user, opts) @topic_view = TopicView.new(params[:id] || params[:topic_id], current_user, opts)
rescue Discourse::NotFound 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 raise Discourse::NotFound unless topic
return redirect_to(topic.relative_url) return redirect_to(topic.relative_url)
end end

View File

@ -494,6 +494,7 @@ class Topic < ActiveRecord::Base
TopicUser.starred_since(sinceDaysAgo).by_date_starred.count TopicUser.starred_since(sinceDaysAgo).by_date_starred.count
end end
# Even if the slug column in the database is null, topic.slug will return something:
def slug def slug
unless slug = read_attribute(:slug) unless slug = read_attribute(:slug)
return '' unless title.present? return '' unless title.present?

View File

@ -383,6 +383,23 @@ describe TopicsController do
expect(response.status).to eq(404) expect(response.status).to eq(404)
end 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 it 'records a view' do
lambda { xhr :get, :show, topic_id: topic.id, slug: topic.slug }.should change(View, :count).by(1) lambda { xhr :get, :show, topic_id: topic.id, slug: topic.slug }.should change(View, :count).by(1)
end end