diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index 1c8745dae..5069221a2 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -23,7 +23,7 @@ class CommentsController < ApplicationController def update @comment = Comment.find(params[:id]) check_privilege(@comment) - @comment.update_attributes(params[:comment]) + @comment.update_attributes(params[:comment].permit(:body)) respond_with(@comment, :location => post_path(@comment.post_id)) end diff --git a/app/models/comment.rb b/app/models/comment.rb index 30afa6ff1..088580b07 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -1,6 +1,7 @@ class Comment < ActiveRecord::Base include Mentionable + validate :validate_post_exists, :on => :create validate :validate_creator_is_not_limited, :on => :create validates_format_of :body, :with => /\S/, :message => 'has no content' belongs_to :post @@ -148,6 +149,10 @@ class Comment < ActiveRecord::Base User.id_to_name(updater_id) end + def validate_post_exists + errors.add(:post, "must exist") unless Post.exists?(post_id) + end + def validate_creator_is_not_limited if creator.is_comment_limited? && !do_not_bump_post? errors.add(:base, "You can only post #{Danbooru.config.member_comment_limit} comments per hour") diff --git a/app/models/post.rb b/app/models/post.rb index 78f7fb17c..26ad1232d 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -45,7 +45,7 @@ class Post < ActiveRecord::Base has_many :favorites, :dependent => :destroy validates_uniqueness_of :md5 validate :post_is_not_its_own_parent - attr_accessible :source, :rating, :tag_string, :old_tag_string, :old_parent_id, :old_source, :old_rating, :last_noted_at, :parent_id, :has_embedded_notes, :as => [:member, :builder, :gold, :platinum, :janitor, :moderator, :admin, :default] + attr_accessible :source, :rating, :tag_string, :old_tag_string, :old_parent_id, :old_source, :old_rating, :parent_id, :has_embedded_notes, :as => [:member, :builder, :gold, :platinum, :janitor, :moderator, :admin, :default] attr_accessible :is_rating_locked, :is_note_locked, :as => [:builder, :janitor, :moderator, :admin] attr_accessible :is_status_locked, :as => [:admin] diff --git a/test/functional/comments_controller_test.rb b/test/functional/comments_controller_test.rb index 41fbeceb8..a98ef8d85 100644 --- a/test/functional/comments_controller_test.rb +++ b/test/functional/comments_controller_test.rb @@ -33,6 +33,28 @@ class CommentsControllerTest < ActionController::TestCase post :update, {:id => @comment.id, :comment => {:body => "abc"}}, {:user_id => @comment.creator_id} assert_redirected_to post_path(@comment.post) end + + should "only allow changing the body" do + params = { + id: @comment.id, + comment: { + body: "herp derp", + do_not_bump_post: true, + is_deleted: true, + post_id: FactoryGirl.create(:post).id, + } + } + + post :update, params, { :user_id => @comment.creator_id } + @comment.reload + + assert_equal("herp derp", @comment.body) + assert_equal(false, @comment.do_not_bump_post) + assert_equal(false, @comment.is_deleted) + assert_equal(@post.id, @comment.post_id) + + assert_redirected_to post_path(@post) + end end context "create action"do @@ -43,6 +65,11 @@ class CommentsControllerTest < ActionController::TestCase comment = Comment.last assert_redirected_to post_path(comment.post) end + + should "not allow commenting on nonexistent posts" do + post :create, {:comment => FactoryGirl.attributes_for(:comment, :post_id => -1)}, {:user_id => @user.id} + assert_response :error + end end end end diff --git a/test/functional/posts_controller_test.rb b/test/functional/posts_controller_test.rb index 7eb538932..5800b2689 100644 --- a/test/functional/posts_controller_test.rb +++ b/test/functional/posts_controller_test.rb @@ -103,6 +103,14 @@ class PostsControllerTest < ActionController::TestCase @post.reload assert_equal("bbb", @post.tag_string) end + + should "ignore restricted params" do + post :update, {:id => @post.id, :post => {:last_noted_at => 1.minute.ago}}, {:user_id => @user.id} + assert_redirected_to post_path(@post) + + @post.reload + assert_nil(@post.last_noted_at) + end end context "revert action" do diff --git a/test/unit/comment_test.rb b/test/unit/comment_test.rb index 46ec5f858..616462044 100644 --- a/test/unit/comment_test.rb +++ b/test/unit/comment_test.rb @@ -101,6 +101,13 @@ class CommentTest < ActiveSupport::TestCase assert(comment.errors.empty?, comment.errors.full_messages.join(", ")) end + should "not validate if the post does not exist" do + comment = FactoryGirl.build(:comment, :post_id => -1) + + assert_not(comment.valid?) + assert_equal(["must exist"], comment.errors[:post]) + end + should "not bump the parent post" do post = FactoryGirl.create(:post) comment = FactoryGirl.create(:comment, :do_not_bump_post => true, :post => post) diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 92ab10f0c..36aac9934 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -1723,5 +1723,9 @@ class PostTest < ActiveSupport::TestCase end end end + + context "Mass assignment: " do + should_not allow_mass_assignment_of(:last_noted_at).as(:member) + end end