From 438186a75a6e1f04e05228a2b69abdbf9761a328 Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 7 May 2020 20:48:50 -0500 Subject: [PATCH] search: fix user-dependent searches showing incorrect paginators. Some searches, such as searches for private favorites or for the status:unmoderated tag, return different results for different users. These searches need to have their counts cached separately for each user so that we don't return incorrect page counts when two different users perform the same search. This can also potentially leak private information, such as the number of posts flagged, downvoted, or disapproved by a given user. Partial fix for #4280. --- app/controllers/modqueue_controller.rb | 2 +- app/logical/post_query_builder.rb | 16 ++++++++++++++-- app/models/post.rb | 2 +- test/unit/post_disapproval_test.rb | 8 ++++---- test/unit/post_query_builder_test.rb | 10 ++++++++++ 5 files changed, 30 insertions(+), 8 deletions(-) diff --git a/app/controllers/modqueue_controller.rb b/app/controllers/modqueue_controller.rb index d4a09104f..264f44b06 100644 --- a/app/controllers/modqueue_controller.rb +++ b/app/controllers/modqueue_controller.rb @@ -4,7 +4,7 @@ class ModqueueController < ApplicationController def index authorize :modqueue - @posts = Post.includes(:appeals, :disapprovals, :uploader, flags: [:creator]).pending_or_flagged.available_for_moderation(search_params[:hidden]) + @posts = Post.includes(:appeals, :disapprovals, :uploader, flags: [:creator]).pending_or_flagged.available_for_moderation(CurrentUser.user, hidden: search_params[:hidden]) @posts = @posts.paginated_search(params, order: "modqueue", count_pages: true) @modqueue_posts = @posts.except(:offset, :limit, :order) diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index 6c4556c3c..6c98545d4 100644 --- a/app/logical/post_query_builder.rb +++ b/app/logical/post_query_builder.rb @@ -288,7 +288,7 @@ class PostQueryBuilder when "active" Post.active when "unmoderated" - Post.pending_or_flagged.available_for_moderation + Post.pending_or_flagged.available_for_moderation(current_user, hidden: false) when "all", "any" Post.all else @@ -868,7 +868,19 @@ class PostQueryBuilder end def count_cache_key - "pfc:#{to_s}" + if is_user_dependent_search? + "pfc[#{current_user.id.to_i}]:#{to_s}" + else + "pfc:#{to_s}" + end + end + + def is_user_dependent_search? + metatags.any? do |metatag| + metatag.name.in?(%w[upvoter upvote downvoter downvote search flagger fav ordfav favgroup ordfavgroup]) || + metatag.name == "status" && metatag.value == "unmoderated" || + metatag.name == "disapproved" && User.normalize_name(metatag.value) == current_user.name + end end end diff --git a/app/models/post.rb b/app/models/post.rb index dbee7b602..4c8d800e5 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -1453,7 +1453,7 @@ class Post < ApplicationRecord from(relation.arel.as("posts")) end - def available_for_moderation(hidden = false, user = CurrentUser.user) + def available_for_moderation(user, hidden: false) return none if user.is_anonymous? approved_posts = user.post_approvals.select(:post_id) diff --git a/test/unit/post_disapproval_test.rb b/test/unit/post_disapproval_test.rb index 879679c05..8def746e1 100644 --- a/test/unit/post_disapproval_test.rb +++ b/test/unit/post_disapproval_test.rb @@ -35,8 +35,8 @@ class PostDisapprovalTest < ActiveSupport::TestCase end should "remove the associated post from alice's moderation queue" do - assert(!Post.available_for_moderation(false).map(&:id).include?(@post_1.id)) - assert(Post.available_for_moderation(false).map(&:id).include?(@post_2.id)) + assert(!Post.available_for_moderation(CurrentUser.user, hidden: false).map(&:id).include?(@post_1.id)) + assert(Post.available_for_moderation(CurrentUser.user, hidden: false).map(&:id).include?(@post_2.id)) end end @@ -47,8 +47,8 @@ class PostDisapprovalTest < ActiveSupport::TestCase end should "not remove the associated post from brittony's moderation queue" do - assert(Post.available_for_moderation(false).map(&:id).include?(@post_1.id)) - assert(Post.available_for_moderation(false).map(&:id).include?(@post_2.id)) + assert(Post.available_for_moderation(CurrentUser.user, hidden: false).map(&:id).include?(@post_1.id)) + assert(Post.available_for_moderation(CurrentUser.user, hidden: false).map(&:id).include?(@post_2.id)) end end end diff --git a/test/unit/post_query_builder_test.rb b/test/unit/post_query_builder_test.rb index 8878c3afb..e3eb07629 100644 --- a/test/unit/post_query_builder_test.rb +++ b/test/unit/post_query_builder_test.rb @@ -1153,5 +1153,15 @@ class PostQueryBuilderTest < ActiveSupport::TestCase end end end + + context "for a user-dependent metatag" do + should "cache the count separately for different users" do + @user = create(:user, enable_private_favorites: true) + @post = as(@user) { create(:post, tag_string: "fav:#{@user.name}") } + + assert_equal(1, PostQueryBuilder.new("fav:#{@user.name}", @user).fast_count) + assert_equal(0, PostQueryBuilder.new("fav:#{@user.name}").fast_count) + end + end end end