Merge pull request #3814 from evazion/fix-3813

Fix #3813: Favorite limit can be bypassed.
This commit is contained in:
Albert Yi
2018-08-13 18:07:41 -07:00
committed by GitHub
8 changed files with 65 additions and 98 deletions

View File

@@ -1,6 +1,6 @@
class FavoritesController < ApplicationController
before_action :member_only, except: [:index]
respond_to :html, :xml, :json
respond_to :html, :xml, :json, :js
skip_before_action :api_check
def index
@@ -24,23 +24,11 @@ class FavoritesController < ApplicationController
end
def create
if CurrentUser.favorite_limit.nil? || CurrentUser.favorite_count < CurrentUser.favorite_limit
@post = Post.find(params[:post_id])
@post.add_favorite!(CurrentUser.user)
else
@error_msg = "You can only keep up to #{CurrentUser.favorite_limit} favorites. Upgrade your account to save more."
end
@post = Post.find(params[:post_id])
@post.add_favorite!(CurrentUser.user)
flash[:notice] = "You have favorited this post"
respond_with(@post) do |format|
format.js
format.json do
if @post
render :json => {:success => true}.to_json
else
render :json => {:success => false, :reason => @error_msg}.to_json, :status => 422
end
end
end
respond_with(@post)
end
def destroy
@@ -52,11 +40,7 @@ class FavoritesController < ApplicationController
Favorite.remove(post_id: params[:id], user: CurrentUser.user)
end
respond_with(@post) do |format|
format.js
format.json do
render :json => {:success => true}.to_json
end
end
flash[:notice] = "You have unfavorited this post"
respond_with(@post)
end
end

View File

@@ -1,4 +1,6 @@
class Favorite < ApplicationRecord
class Error < Exception ; end
belongs_to :post
belongs_to :user
scope :for_user, ->(user_id) {where("user_id % 100 = #{user_id.to_i % 100} and user_id = #{user_id.to_i}")}
@@ -7,7 +9,12 @@ class Favorite < ApplicationRecord
Favorite.transaction do
User.where(:id => user.id).select("id").lock("FOR UPDATE NOWAIT").first
return if Favorite.for_user(user.id).where(:user_id => user.id, :post_id => post.id).exists?
if user.favorite_count >= user.favorite_limit
raise Error, "You can only keep up to #{user.favorite_limit} favorites. Upgrade your account to save more."
elsif Favorite.for_user(user.id).where(:user_id => user.id, :post_id => post.id).exists?
raise Error, "You have already favorited this post"
end
Favorite.create!(:user_id => user.id, :post_id => post.id)
Post.where(:id => post.id).update_all("fav_count = fav_count + 1")
post.append_user_to_fav_string(user.id)

View File

@@ -108,6 +108,7 @@ class User < ApplicationRecord
has_many :forum_posts, -> {order("forum_posts.created_at, forum_posts.id")}, :foreign_key => "creator_id"
has_many :user_name_change_requests, -> {visible.order("user_name_change_requests.created_at desc")}
has_many :favorite_groups, -> {order(name: :asc)}, foreign_key: :creator_id
has_many :favorites, ->(rec) {where("user_id % 100 = #{rec.id % 100} and user_id = #{rec.id}").order("id desc")}
belongs_to :inviter, class_name: "User", optional: true
after_update :create_mod_action
accepts_nested_attributes_for :dmail_filter
@@ -297,20 +298,6 @@ class User < ApplicationRecord
end
end
module FavoriteMethods
def favorites
Favorite.where("user_id % 100 = #{id % 100} and user_id = #{id}").order("id desc")
end
def add_favorite!(post)
Favorite.add(post: post, user: self)
end
def remove_favorite!(post)
Favorite.remove(post: post, user: self)
end
end
module LevelMethods
extend ActiveSupport::Concern
@@ -618,7 +605,7 @@ class User < ApplicationRecord
def favorite_limit
if is_platinum?
nil
Float::INFINITY
elsif is_gold?
20_000
else
@@ -917,7 +904,6 @@ class User < ApplicationRecord
include NameMethods
include PasswordMethods
include AuthenticationMethods
include FavoriteMethods
include LevelMethods
include EmailMethods
include BlacklistMethods

View File

@@ -0,0 +1,16 @@
$("#add-to-favorites, #add-fav-button, #remove-from-favorites, #remove-fav-button").toggle();
$("#score-for-post-<%= @post.id %>").text(<%= @post.score %>);
$("#favcount-for-post-<%= @post.id %>").text(<%= @post.fav_count %>);
<% if CurrentUser.is_gold? %>
var fav_count = <%= @post.fav_count %>;
$("#favlist").html("<%= j post_favlist(@post) %>");
if (fav_count === 0) {
$("#show-favlist-link, #hide-favlist-link, #favlist").hide();
} else if (!$("#favlist").is(":visible")) {
$("#show-favlist-link").show();
}
<% end %>
Danbooru.Utility.notice("<%= j flash[:notice] %>");

View File

@@ -1,17 +0,0 @@
<% if @error_msg %>
$(window).trigger("danbooru:error", "<%= @error_msg %>");
<% else %>
$("#add-to-favorites").hide();
$("#add-fav-button").hide();
$("#remove-from-favorites").show();
$("#remove-fav-button").show();
$("#score-for-post-<%= @post.id %>").html(<%= @post.score %>);
$("#favcount-for-post-<%= @post.id %>").html(<%= @post.fav_count %>);
<% if CurrentUser.is_gold? %>
$("#favlist").html("<%= escape_javascript(post_favlist(@post)) %>");
if (!$("#favlist").is(":visible")) {
$("#show-favlist-link").show();
}
<% end %>
$(window).trigger("danbooru:notice", "You have favorited this post");
<% end %>

View File

@@ -0,0 +1 @@
_update.js.erb

View File

@@ -1,13 +0,0 @@
$("#add-to-favorites").show();
$("#add-fav-button").show();
$("#remove-from-favorites").hide();
$("#remove-fav-button").hide();
$("#score-for-post-<%= @post.id %>").html(<%= @post.score %>);
$("#favcount-for-post-<%= @post.id %>").html(<%= @post.fav_count %>);
<% if CurrentUser.is_gold? %>
$("#favlist").html("<%= escape_javascript(post_favlist(@post)) %>");
<% if @post.fav_count == 0 %>
$("#show-favlist-link, #hide-favlist-link, #favlist").hide();
<% end %>
<% end %>
$(window).trigger("danbooru:notice", "You have unfavorited this post");

View File

@@ -0,0 +1 @@
_update.js.erb

View File

@@ -0,0 +1,2 @@
var message = <%= raw @error_message.try(:to_json) || @exception.message.to_json %>;
Danbooru.Utility.error(message);

View File

@@ -2,10 +2,13 @@ require 'test_helper'
class FavoriteTest < ActiveSupport::TestCase
setup do
user = FactoryBot.create(:user)
CurrentUser.user = user
@user1 = FactoryBot.create(:user)
@user2 = FactoryBot.create(:user)
@p1 = FactoryBot.create(:post)
@p2 = FactoryBot.create(:post)
CurrentUser.user = @user1
CurrentUser.ip_addr = "127.0.0.1"
Favorite # need to force loading the favorite model
end
teardown do
@@ -15,44 +18,41 @@ class FavoriteTest < ActiveSupport::TestCase
context "A favorite" do
should "delete from all tables" do
user1 = FactoryBot.create(:user)
p1 = FactoryBot.create(:post)
@p1.add_favorite!(@user1)
assert_equal(1, @user1.favorite_count)
user1.add_favorite!(p1)
assert_equal(1, Favorite.count)
Favorite.where(:user_id => user1.id, :post_id => p1.id).delete_all
Favorite.where(:user_id => @user1.id, :post_id => @p1.id).delete_all
assert_equal(0, Favorite.count)
end
should "know which table it belongs to" do
user1 = FactoryBot.create(:user)
user2 = FactoryBot.create(:user)
p1 = FactoryBot.create(:post)
p2 = FactoryBot.create(:post)
@p1.add_favorite!(@user1)
@p2.add_favorite!(@user1)
@p1.add_favorite!(@user2)
user1.add_favorite!(p1)
user1.add_favorite!(p2)
user2.add_favorite!(p1)
favorites = user1.favorites.order("id desc")
favorites = @user1.favorites.order("id desc")
assert_equal(2, favorites.count)
assert_equal(p2.id, favorites[0].post_id)
assert_equal(p1.id, favorites[1].post_id)
assert_equal(@p2.id, favorites[0].post_id)
assert_equal(@p1.id, favorites[1].post_id)
favorites = user2.favorites.order("id desc")
favorites = @user2.favorites.order("id desc")
assert_equal(1, favorites.count)
assert_equal(p1.id, favorites[0].post_id)
assert_equal(@p1.id, favorites[0].post_id)
end
should "not allow duplicates" do
user1 = FactoryBot.create(:user)
p1 = FactoryBot.create(:post)
p2 = FactoryBot.create(:post)
user1.add_favorite!(p1)
user1.add_favorite!(p1)
@p1.add_favorite!(@user1)
error = assert_raises(Favorite::Error) { @p1.add_favorite!(@user1) }
assert_equal(1, user1.favorites.count)
assert_equal("You have already favorited this post", error.message)
assert_equal(1, @user1.favorite_count)
end
should "not allow exceeding the user's favorite limit" do
@user1.stubs(:favorite_limit).returns(0)
error = assert_raises(Favorite::Error) { @p1.add_favorite!(@user1) }
assert_equal("You can only keep up to 0 favorites. Upgrade your account to save more.", error.message)
end
end
end