From eca0ab04f7c1de3ae7e22f5425130848d45bd217 Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 9 Apr 2022 01:36:44 -0500 Subject: [PATCH] post queries: raise error on invalid searches. Raise an error if the search is invalid for one of the following reasons: * It contains multiple conflicting order: metatags (e.g. `order:score order:favcount` or `ordfav:a ordfav:b`). * It contains a metatag that can't be used more than once: (e.g. `limit:5 limit:10`, `random:5 random:10`). * It contains a metatag that can't be negated (e.g. `-order:score`, `-limit:20`, or `-random:20`). * It contains a metatag that can't be used in an OR clause (e.g. ` touhou or order:score`, `touhou or limit:20`, `touhou or random:20`). --- app/controllers/application_controller.rb | 2 +- app/logical/post_query.rb | 54 ++++++++++++++++++++++- app/logical/post_query/ast.rb | 31 +++++++++++-- app/logical/post_query_builder.rb | 19 -------- test/unit/post_query_builder_test.rb | 44 ++++++++++++++++-- test/unit/post_sets/post_test.rb | 2 +- 6 files changed, 123 insertions(+), 29 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index cd8ea2578..796f0175f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -130,7 +130,7 @@ class ApplicationController < ActionController::Base render_error_page(406, exception, message: "#{request.format} is not a supported format for this page") when PaginationExtension::PaginationError render_error_page(410, exception, template: "static/pagination_error", message: "You cannot go beyond page #{CurrentUser.user.page_limit}.") - when PostQueryBuilder::TagLimitError + when PostQuery::TagLimitError render_error_page(422, exception, template: "static/tag_limit_error", message: "You cannot search for more than #{CurrentUser.tag_query_limit} tags at a time.") when RateLimiter::RateLimitError render_error_page(429, exception, message: "Rate limit exceeded. You're doing that too fast") diff --git a/app/logical/post_query.rb b/app/logical/post_query.rb index 9e2607c2a..6d15927d4 100644 --- a/app/logical/post_query.rb +++ b/app/logical/post_query.rb @@ -3,10 +3,22 @@ class PostQuery extend Memoist + class Error < StandardError; end + class TagLimitError < Error; end + + # Metatags that don't count against the user's tag limit. + UNLIMITED_METATAGS = %w[status rating limit] + + # Metatags that define the order of search results. These metatags can't be used more than once per query. + ORDER_METATAGS = %w[order ordfav ordfavgroup ordpool] + + # Metatags that can't be used more than once per query, and that can't be used with OR or NOT operators. + SINGLETON_METATAGS = ORDER_METATAGS + %w[limit random] + attr_reader :current_user private attr_reader :tag_limit, :safe_mode, :hide_deleted_posts, :builder - delegate :tag?, :metatag?, :wildcard?, :metatags, :wildcards, :tag_names, :metatags, :to_infix, to: :ast + delegate :tag?, :metatag?, :wildcard?, :metatags, :wildcards, :tag_names, :to_infix, to: :ast alias_method :safe_mode?, :safe_mode alias_method :hide_deleted_posts?, :hide_deleted_posts alias_method :to_s, :to_infix @@ -16,6 +28,11 @@ class PostQuery PostQuery.new(...).replace_aliases.rewrite_opts.trim end + # Perform a search and return the resulting posts + def self.search(search, ...) + PostQuery.normalize(search, ...).with_implicit_metatags.posts + end + def initialize(search_or_ast, current_user: User.anonymous, tag_limit: nil, safe_mode: false, hide_deleted_posts: false) if search_or_ast.is_a?(AST) @ast = search_or_ast @@ -47,10 +64,12 @@ class PostQuery end def posts + validate! builder.posts(to_cnf) end def paginated_posts(...) + validate! builder.paginated_posts(to_cnf, ...) end @@ -241,5 +260,36 @@ class PostQuery end end - memoize :tags, :replace_aliases, :with_implicit_metatags, :to_cnf, :aliases, :implicit_metatags, :hide_deleted? + concerning :ValidationMethods do + def validate! + return if is_empty_search? || is_simple_tag? + + validate_tag_limit! + validate_metatags! + end + + def validate_tag_limit! + raise TagLimitError if tag_limit.present? && term_count > tag_limit + end + + def validate_metatags! + return if metatags.empty? + + raise Error, "Can't have multiple order metatags" if select_metatags(*ORDER_METATAGS).size > 1 + + SINGLETON_METATAGS.each do |name| + metatag = select_metatags(name).first + raise Error, "'#{name}:' can't be used more than once" if select_metatags(name).size > 1 + raise Error, "#{metatag} can't be negated" if metatag&.parents&.any?(&:not?) + raise Error, "#{metatag} can't be used in an 'or' clause" if metatag&.parents&.any?(&:or?) + end + end + + # The number of unique tags, wildcards, and metatags in the search, excluding metatags that don't count against the user's tag limit. + def term_count + tag_names.size + wildcards.size + metatags.count { !_1.name.in?(UNLIMITED_METATAGS) } + end + end + + memoize :tags, :replace_aliases, :with_implicit_metatags, :to_cnf, :aliases, :implicit_metatags, :hide_deleted?, :term_count end diff --git a/app/logical/post_query/ast.rb b/app/logical/post_query/ast.rb index 4e5eefda5..2d3ad020b 100644 --- a/app/logical/post_query/ast.rb +++ b/app/logical/post_query/ast.rb @@ -41,7 +41,8 @@ class PostQuery include Comparable include Enumerable - attr_reader :type, :args + attr_reader :type, :args, :parent + protected attr_writer :parent delegate :all?, :none?, :and?, :or?, :not?, :opt?, :tag?, :metatag?, :wildcard?, to: :inquirer # Create an AST node. @@ -51,7 +52,14 @@ class PostQuery # AND/OR/NOT/OPT nodes, or the name and/or value for tag, metatag, or wildcard nodes). def initialize(type, args) @type = type - @args = args + @parent = nil + + if term? + @args = args + else + @args = args.deep_dup + @args.each { _1.parent = self } + end end concerning :ConstructorMethods do @@ -198,6 +206,10 @@ class PostQuery end concerning :OutputMethods do + def to_s + to_infix + end + def inspect to_sexp end @@ -339,6 +351,19 @@ class PostQuery tags.map(&:name) end + # @return [Array] The list of all parent nodes of this node. + def parents + parents = [] + + node = self + until node.parent.nil? + parents << node.parent + node = node.parent + end + + parents + end + # True if the AST is a simple node, that is a leaf node with no child nodes. def term? type.in?(%i[tag metatag wildcard all none]) @@ -390,6 +415,6 @@ class PostQuery end end - memoize :to_cnf, :simplify, :simplify_once, :rewrite_opts, :trim, :trim_once, :sort, :inquirer, :deconstruct, :inspect, :to_sexp, :to_infix, :to_tree, :nodes, :tags, :metatags, :tag_names + memoize :to_cnf, :simplify, :simplify_once, :rewrite_opts, :trim, :trim_once, :sort, :inquirer, :deconstruct, :inspect, :to_sexp, :to_infix, :to_tree, :nodes, :tags, :metatags, :tag_names, :parents end end diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index b5bdcdb1a..3199b1e10 100644 --- a/app/logical/post_query_builder.rb +++ b/app/logical/post_query_builder.rb @@ -12,8 +12,6 @@ require "strscan" class PostQueryBuilder extend Memoist - # Raised when the number of tags exceeds the user's tag limit. - class TagLimitError < StandardError; end class ParseError < StandardError; end # How many tags a `blah*` search should match. @@ -72,9 +70,6 @@ class PostQueryBuilder COUNT_METATAG_SYNONYMS.flat_map { |str| [str, "#{str}_asc"] } + CATEGORY_COUNT_METATAGS.flat_map { |str| [str, "#{str}_asc"] } - # Tags that don't count against the user's tag limit. - UNLIMITED_METATAGS = %w[status rating limit] - attr_reader :query_string, :current_user, :tag_limit, :safe_mode, :hide_deleted_posts alias_method :safe_mode?, :safe_mode alias_method :hide_deleted_posts?, :hide_deleted_posts @@ -497,20 +492,6 @@ class PostQueryBuilder relation.in_order_of(:id, ids) end - # @raise [TagLimitError] if the number of tags exceeds the user's tag limit - def validate! - tag_count = terms.count { |term| !is_unlimited_tag?(term) } - - if tag_limit.present? && tag_count > tag_limit - raise TagLimitError - end - end - - # @return [Boolean] true if the metatag doesn't count against the user's tag limit - def is_unlimited_tag?(term) - term.type == :metatag && term.name.in?(UNLIMITED_METATAGS) - end - concerning :ParseMethods do # Parse the search into a list of search terms. A search term is a tag or a metatag. # @return [Array] a list of terms diff --git a/test/unit/post_query_builder_test.rb b/test/unit/post_query_builder_test.rb index addd09ae5..62362aaac 100644 --- a/test/unit/post_query_builder_test.rb +++ b/test/unit/post_query_builder_test.rb @@ -5,6 +5,10 @@ class PostQueryBuilderTest < ActiveSupport::TestCase assert_equal(posts.map(&:id), Post.user_tag_match(query, current_user, **options).pluck(:id)) end + def assert_search_error(query, current_user: CurrentUser.user, **options) + assert_raises(PostQuery::Error) { PostQuery.search(query, current_user: current_user, **options) } + end + def assert_fast_count(count, query, query_options = {}, fast_count_options = {}) assert_equal(count, PostQuery.normalize(query, **query_options).with_implicit_metatags.fast_count(**fast_count_options)) end @@ -1285,11 +1289,11 @@ class PostQueryBuilderTest < ActiveSupport::TestCase assert_tag_match([post1], "-/hr") end - should "fail for more than 6 tags" do + should "fail if the search exceeds the tag limit" do post1 = create(:post, rating: "s") - assert_raise(PostQueryBuilder::TagLimitError) do - Post.user_tag_match("a b c rating:s width:10 height:10 user:bob") + assert_raise(PostQuery::TagLimitError) do + PostQuery.search("a b c rating:s width:10 height:10 user:bob", tag_limit: 5) end end @@ -1345,6 +1349,40 @@ class PostQueryBuilderTest < ActiveSupport::TestCase assert_tag_match([post2, post1], "id:#{post1.id} or rating:q") end + + should "not allow conflicting order metatags" do + assert_search_error("order:score ordfav:a") + assert_search_error("order:score ordfavgroup:a") + assert_search_error("order:score ordpool:a") + assert_search_error("ordfav:a ordpool:b") + end + + should "not allow metatags that can't be used more than once" do + assert_search_error("order:score order:favcount") + assert_search_error("ordfav:a ordfav:b") + assert_search_error("ordfavgroup:a ordfavgroup:b") + assert_search_error("ordpool:a ordpool:b") + assert_search_error("limit:5 limit:10") + assert_search_error("random:5 random:10") + end + + should "not allow non-negatable metatags to be negated" do + assert_search_error("-order:score") + assert_search_error("-ordfav:a") + assert_search_error("-ordfavgroup:a") + assert_search_error("-ordpool:a") + assert_search_error("-limit:20") + assert_search_error("-random:20") + end + + should "not allow non-OR'able metatags to be OR'd" do + assert_search_error("a or order:score") + assert_search_error("a or ordfav:a") + assert_search_error("a or ordfavgroup:a") + assert_search_error("a or ordpool:a") + assert_search_error("a or limit:20") + assert_search_error("a or random:20") + end end context "Parsing:" do diff --git a/test/unit/post_sets/post_test.rb b/test/unit/post_sets/post_test.rb index b1bb73a60..056a8ee8b 100644 --- a/test/unit/post_sets/post_test.rb +++ b/test/unit/post_sets/post_test.rb @@ -82,7 +82,7 @@ module PostSets should "fail" do @set = PostSets::Post.new("a b c", user: create(:user)) - assert_raises(PostQueryBuilder::TagLimitError) do + assert_raises(PostQuery::TagLimitError) do @set.posts end end