From a5ed8c72c90731fa0c40b44831e6378b8a9264b7 Mon Sep 17 00:00:00 2001 From: evazion Date: Tue, 2 Nov 2021 00:41:05 -0500 Subject: [PATCH] search: fix parsing of invalid metatag values. * Change `age:` metatag to require time units. This means e.g. `age:<600` no longer works; instead you have to say `age:<600sec`. * Allow time units in the `age:` metatag to be abbreviated as long as they're unambiguous. This means `age:<60sec`, `age:<5min`, and `age:<5mon` now work, in addition to `age:<60s` and `age:<60seconds`. * Allow the `ratio:` metatag to be written like `ratio:16/9` in addition to `ratio:16:9`. * Fix invalid date searches like `date:foo` or `date:05-15-2021` to return nothing instead of raising an "undefined method 'beginning_of_day' for nil" exception. (`date:05-15-2021` is invalid because it's parsed as DD-MM-YYYY). * Fix invalid searches like `score:foo`, `ratio:foo`, and `mpixels:foo` to return nothing instead of being treated like `score:0`, `ratio:0`, `mpixels:0`. * Fix `age:<60m` to return nothing instead of silently being treated like `age:<60seconds`. * Fix `age:foo` to return nothing instead of silently being treated like `age:0d` (return all uploads from today). Fixes #4389. --- app/logical/duration_parser.rb | 31 ++++---- app/logical/post_query_builder.rb | 42 ++++++----- test/unit/post_query_builder_test.rb | 105 +++++++++++++++++++++++---- 3 files changed, 130 insertions(+), 48 deletions(-) diff --git a/app/logical/duration_parser.rb b/app/logical/duration_parser.rb index 48d9fc28e..9674dd70c 100644 --- a/app/logical/duration_parser.rb +++ b/app/logical/duration_parser.rb @@ -1,27 +1,32 @@ +require 'abbrev' + module DurationParser def self.parse(string) - string =~ /(\d+)(s(econds?)?|mi(nutes?)?|h(ours?)?|d(ays?)?|w(eeks?)?|mo(nths?)?|y(ears?)?)?/i + abbrevs = Abbrev.abbrev(%w[seconds minutes hours days weeks months years]) - size = $1.to_i - unit = $2 + raise unless string =~ /(.*?)([a-z]+)\z/i + size = Float($1) + unit = abbrevs.fetch($2.downcase) case unit - when /^s/i + when "seconds" size.seconds - when /^mi/i + when "minutes" size.minutes - when /^h/i + when "hours" size.hours - when /^d/i + when "days" size.days - when /^w/i + when "weeks" size.weeks - when /^mo/i - size.months - when /^y/i - size.years + when "months" + size * (365.25.days / 12) + when "years" + size * (365.25.days) else - size.seconds + raise NotImplementedError end + rescue + raise ArgumentError, "'#{string}' is not a valid duration" end end diff --git a/app/logical/post_query_builder.rb b/app/logical/post_query_builder.rb index 0146bf4bc..633eaf1a1 100644 --- a/app/logical/post_query_builder.rb +++ b/app/logical/post_query_builder.rb @@ -12,6 +12,7 @@ class PostQueryBuilder # 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. MAX_WILDCARD_TAGS = 100 @@ -259,6 +260,8 @@ class PostQueryBuilder def attribute_matches(value, field, type = :integer) operator, *args = parse_metatag_value(value, type) Post.where_operator(field, operator, *args) + rescue ParseError + Post.none end def user_matches(field, username) @@ -815,45 +818,43 @@ class PostQueryBuilder end # Parse a simple string value into a Ruby type. - # @param object [String] the value to parse + # @param string [String] the value to parse # @param type [Symbol] the value's type # @return [Object] the parsed value - def parse_cast(object, type) + def parse_cast(string, type) case type when :enum - object.to_s.downcase + string.downcase when :integer - object.to_i + Integer(string) # raises ArgumentError if string is invalid when :float - object.to_f + Float(string) # raises ArgumentError if string is invalid when :md5 - object.to_s.downcase + raise ParseError, "#{string} is not a valid MD5" unless string.match?(/\A[0-9a-fA-F]{32}\z/) + string.downcase when :date, :datetime - Time.zone.parse(object) rescue nil + date = Time.zone.parse(string) + raise ParseError, "#{string} is not a valid date" if date.nil? + date when :age - DurationParser.parse(object).ago + DurationParser.parse(string).ago when :interval - DurationParser.parse(object) + DurationParser.parse(string) when :ratio - object =~ /\A(\d+(?:\.\d+)?):(\d+(?:\.\d+)?)\Z/i - - if $1 && $2.to_f != 0.0 - ($1.to_f / $2.to_f).round(2) - else - object.to_f.round(2) - end + string = string.tr(":", "/") # "2:3" => "2/3" + Rational(string).to_f.round(2) # raises ArgumentError or ZeroDivisionError if string is invalid when :filesize - object =~ /\A(\d+(?:\.\d*)?|\d*\.\d+)([kKmM]?)[bB]?\Z/ + raise ParseError, "#{string} is not a valid filesize" unless string =~ /\A(\d+(?:\.\d*)?|\d*\.\d+)([kKmM]?)[bB]?\Z/ - size = $1.to_f + size = Float($1) unit = $2 conversion_factor = case unit @@ -868,8 +869,11 @@ class PostQueryBuilder (size * conversion_factor).to_i else - raise NotImplementedError, "unrecognized type #{type} for #{object}" + raise NotImplementedError, "unrecognized type #{type} for #{string}" end + + rescue ArgumentError, ZeroDivisionError => e + raise ParseError, e.message end def parse_metatag_value(string, type) diff --git a/test/unit/post_query_builder_test.rb b/test/unit/post_query_builder_test.rb index 775b38537..b2afd8cee 100644 --- a/test/unit/post_query_builder_test.rb +++ b/test/unit/post_query_builder_test.rb @@ -165,6 +165,47 @@ class PostQueryBuilderTest < ActiveSupport::TestCase end end + context "for an invalid metatag value" do + should "return nothing" do + post = create(:post_with_file, created_at: Time.parse("2021-06-15 12:00:00"), score: 42, filename: "test.jpg") + + assert_tag_match([], "score:foo") + assert_tag_match([], "score:42x") + assert_tag_match([], "score:x42") + assert_tag_match([], "score:42.0") + + assert_tag_match([], "mpixels:foo") + assert_tag_match([], "mpixels:0.1675foo") + assert_tag_match([], "mpixels:foo0.1675") + + assert_tag_match([], "ratio:foo") + assert_tag_match([], "ratio:1.49foo") + assert_tag_match([], "ratio:foo1.49") + assert_tag_match([], "ratio:1:0") + assert_tag_match([], "ratio:1/0") + assert_tag_match([], "ratio:-149/-100") + + assert_tag_match([], "filesize:foo") + assert_tag_match([], "filesize:28086foo") + assert_tag_match([], "filesize:foo28086") + + assert_tag_match([], "filesize:foo") + assert_tag_match([], "filesize:28086foo") + assert_tag_match([], "filesize:foo28086") + + assert_tag_match([], "date:foo") + assert_tag_match([], "date:2021-13-01") + assert_tag_match([], "date:2021-01-32") + assert_tag_match([], "date:01-32-2021") + assert_tag_match([], "date:13-01-2021") + + assert_tag_match([], "age:foo") + assert_tag_match([], "age:30") + + assert_tag_match([], "md5:foo") + end + end + should "return posts for the id: metatag" do posts = create_list(:post, 3) @@ -583,16 +624,22 @@ class PostQueryBuilderTest < ActiveSupport::TestCase end should "return posts for the date: metatag" do - post = create(:post, created_at: Time.parse("2017-01-01 12:00")) + post = create(:post, created_at: Time.zone.parse("2017-05-15 12:00")) - assert_tag_match([post], "date:2017-01-01") - assert_tag_match([], "-date:2017-01-01") + assert_tag_match([post], "date:2017-05-15") + assert_tag_match([post], "date:2017/05/15") + assert_tag_match([post], "date:2017.05.15") + + assert_tag_match([post], "date:15-5-2017") + assert_tag_match([post], "date:15/5/2017") + assert_tag_match([post], "date:15.5.2017") + + assert_tag_match([], "-date:2017-05-15") end should "return posts for the age: metatag" do post = create(:post) - assert_tag_match([post], "age:<60") assert_tag_match([post], "age:<60s") assert_tag_match([post], "age:<1mi") assert_tag_match([post], "age:<1h") @@ -604,8 +651,8 @@ class PostQueryBuilderTest < ActiveSupport::TestCase assert_tag_match([post], "age:<=1y") assert_tag_match([post], "age:>0s") assert_tag_match([post], "age:>=0s") - assert_tag_match([post], "age:0s..1m") - assert_tag_match([post], "age:1m..0s") + assert_tag_match([post], "age:0s..1min") + assert_tag_match([post], "age:1min..0s") assert_tag_match([], "age:>1y") assert_tag_match([], "age:>=1y") @@ -614,14 +661,35 @@ class PostQueryBuilderTest < ActiveSupport::TestCase assert_tag_match([post], "-age:>1y") assert_tag_match([], "-age:<1y") + + assert_tag_match([], "age:<60") end should "return posts for the ratio: metatag" do - post = create(:post, image_width: 1000, image_height: 500) + post = create(:post_with_file, filename: "test.jpg") - assert_tag_match([post], "ratio:2:1") - assert_tag_match([post], "ratio:2.0") - assert_tag_match([], "-ratio:2.0") + assert_tag_match([post], "ratio:1.49") + assert_tag_match([post], "ratio:.149e1") + assert_tag_match([post], "ratio:0.149e1") + assert_tag_match([post], "ratio:149e-2") + assert_tag_match([post], "ratio:1490:1000") + assert_tag_match([post], "ratio:149:100") + assert_tag_match([post], "ratio:149/100") + + assert_tag_match([], "-ratio:1.49") + end + + should "return posts for the mpixels:N metatag" do + post = create(:post_with_file, filename: "test.jpg") + + assert_tag_match([post], "mpixels:0.1675") + assert_tag_match([post], "mpixels:+0.1675") + assert_tag_match([post], "mpixels:.1675") + assert_tag_match([post], "mpixels:+.1675") + assert_tag_match([post], "mpixels:1675e-4") + + assert_tag_match([], "mpixels:0.2") + assert_tag_match([], "-mpixels:0.1675") end should "return posts for the duration: metatag" do @@ -755,15 +823,20 @@ class PostQueryBuilderTest < ActiveSupport::TestCase end should "return posts for the md5: metatag" do - post1 = create(:post, md5: "abcd") + post1 = create(:post_with_file, filename: "test.jpg") post2 = create(:post) - assert_tag_match([post1], "md5:abcd") - assert_tag_match([post1], "md5:ABCD") - assert_tag_match([post1], "md5:123,abcd") - assert_tag_match([], "md5:abcd md5:xyz") + assert_tag_match([post1], "md5:ecef68c44edb8a0d6a3070b5f8e8ee76") + assert_tag_match([post1], "md5:ECEF68C44EDB8A0D6A3070B5F8E8EE76") + assert_tag_match([post1], "md5:081a5c3b92d8980d1aadbd215bfac5b9,ecef68c44edb8a0d6a3070b5f8e8ee76") - assert_tag_match([post2], "-md5:abcd") + assert_tag_match([post2], "-md5:ecef68c44edb8a0d6a3070b5f8e8ee76") + assert_tag_match([post2], "-md5:ECEF68C44EDB8A0D6A3070B5F8E8EE76") + + assert_tag_match([], "md5:xyz") + assert_tag_match([], "md5:ecef68c44edb8a0d6a3070b5f8e8ee76 md5:xyz") + + assert_tag_match([post2, post1], "-md5:xyz") end should "return posts for a source: search" do