From c1c7c9e8c1549ac3fcc2fa48f099f27828bfc0c8 Mon Sep 17 00:00:00 2001 From: nikobee Date: Fri, 17 Nov 2023 16:06:11 +0000 Subject: [PATCH 1/6] AO3-6626: prevent tags from being created with Chinese or Japanese commas --- app/models/tag.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/tag.rb b/app/models/tag.rb index 519a36de453..f4b6019f198 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -168,8 +168,8 @@ def commentable_owners maximum: ArchiveConfig.TAG_MAX, message: "^Tag name '%{value}' is too long -- try using less than %{count} characters or using commas to separate your tags." validates_format_of :name, - with: /\A[^,*<>^{}=`\\%]+\z/, - message: "^Tag name '%{value}' cannot include the following restricted characters: , ^ * < > { } = ` \\ %" + with: /\A[^,，、*<>^{}=`\\%]+\z/, + message: "^Tag name '%{value}' cannot include the following restricted characters: , ^ * < > { } = ` ， 、 \\ %" validates_presence_of :sortable_name From b16b7186ca4d8b05cb1c662a99ff29438f1c18a4 Mon Sep 17 00:00:00 2001 From: nikobee Date: Fri, 17 Nov 2023 16:34:19 +0000 Subject: [PATCH 2/6] AO3-6626: fix indentation per styleguide --- app/models/tag.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/tag.rb b/app/models/tag.rb index f4b6019f198..18ed8ae9883 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -168,7 +168,7 @@ def commentable_owners maximum: ArchiveConfig.TAG_MAX, message: "^Tag name '%{value}' is too long -- try using less than %{count} characters or using commas to separate your tags." validates_format_of :name, - with: /\A[^,，、*<>^{}=`\\%]+\z/, + with: /\A[^,，、*<>^{}=`\\%]+\z/, message: "^Tag name '%{value}' cannot include the following restricted characters: , ^ * < > { } = ` ， 、 \\ %" validates_presence_of :sortable_name From 250b48fb28214cd84e0a325c273ec51d32012fb1 Mon Sep 17 00:00:00 2001 From: nikobee Date: Sun, 19 Nov 2023 17:38:15 +0000 Subject: [PATCH 3/6] AO3-6626 correct rubocop violations --- app/models/tag.rb | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/app/models/tag.rb b/app/models/tag.rb index 18ed8ae9883..9740deb8546 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -110,7 +110,7 @@ def commentable_owners end has_many :mergers, foreign_key: 'merger_id', class_name: 'Tag' - belongs_to :merger, class_name: 'Tag' + belongs_to :merger, class_name: "Tag" belongs_to :fandom belongs_to :media belongs_to :last_wrangler, polymorphic: true @@ -161,17 +161,18 @@ def commentable_owners has_many :tag_set_associations, dependent: :destroy has_many :parent_tag_set_associations, class_name: 'TagSetAssociation', foreign_key: 'parent_tag_id', dependent: :destroy - validates_presence_of :name + validates :name, presence: true validates :name, uniqueness: true - validates_length_of :name, minimum: 1, message: "cannot be blank." - validates_length_of :name, - maximum: ArchiveConfig.TAG_MAX, - message: "^Tag name '%{value}' is too long -- try using less than %{count} characters or using commas to separate your tags." - validates_format_of :name, - with: /\A[^,，、*<>^{}=`\\%]+\z/, - message: "^Tag name '%{value}' cannot include the following restricted characters: , ^ * < > { } = ` ， 、 \\ %" - - validates_presence_of :sortable_name + validates :name, + length: { minimum: 1, + message: "cannot be blank." } + validates :name, + length: { maximum: ArchiveConfig.TAG_MAX, + message: "^Tag name '%{value}' is too long -- try using less than %{count} characters or using commas to separate your tags." } + validates :name, + format: { with: /\A[^,，、*<>^{}=`\\%]+\z/, + message: "^Tag name '%{value}' cannot include the following restricted characters: , ^ * < > { } = ` ， 、 \\ %" } + validates :sortable_name, presence: true validate :unwrangleable_status def unwrangleable_status From 0a3c354d8ee57158e2bbc1b415bd348d435773fd Mon Sep 17 00:00:00 2001 From: nikobee Date: Mon, 20 Nov 2023 08:32:36 +0000 Subject: [PATCH 4/6] AO3-6626 test all forbidden characters --- spec/models/tag_spec.rb | 16 ++++++++++------ spec/spec_helper.rb | 1 + 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index 16daf57b9bb..fd186cab2ec 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -49,7 +49,7 @@ expect(@fandom_tag.taggings_count).to eq 1 end - it 'will start caching a when tag when that tag is used significantly' do + it 'will start caching a tag when that tag is used significantly' do (1..ArchiveConfig.TAGGINGS_COUNT_MIN_CACHE_COUNT).each do |try| FactoryBot.create(:work, fandom_string: @fandom_tag.name) RedisJobSpawner.perform_now("TagCountUpdateJob") @@ -168,11 +168,15 @@ def expect_tag_update_flag_in_redis_to_be(flag) expect(tag.errors[:name].join).to match(/too long/) end - it "should not be valid with disallowed characters" do - tag = Tag.new - tag.name = "badi[j\k]l@example.com', 'this is"not\allowed@example.com', 'this\ still\"not/\/\allowed@example.com', "nodomain", "foo@oops", "ast*risk@example.com", "asterisk@ex*ample.com"].freeze + BAD_TAGS = ["bad, tag", "also， bad", "no、good", "wild*card", "lessertag", "^tag", "{open", "close}", "not=allowed", "suspicious`character", "no%maths"].freeze INVALID_URLS = %w[no_scheme.com ftp://ftp.address.com http://www.b@d!35.com https://www.b@d!35.com http://b@d!35.com https://www.b@d!35.com].freeze VALID_URLS = %w[http://rocksalt-recs.livejournal.com/196316.html https://rocksalt-recs.livejournal.com/196316.html].freeze INACTIVE_URLS = %w[https://www.iaminactive.com http://www.iaminactive.com https://iaminactive.com http://iaminactive.com].freeze From 192bfc8f2b9a21d97c9a43ef6931385611eeaca5 Mon Sep 17 00:00:00 2001 From: nikobee Date: Mon, 20 Nov 2023 08:44:20 +0000 Subject: [PATCH 5/6] AO3-6626 update to meet rubocop standards --- spec/models/tag_spec.rb | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index fd186cab2ec..2730b18436c 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -6,7 +6,7 @@ User.current_user = nil end - context 'checking count caching' do + context "checking count caching" do before(:each) do # Set the minimal amount of time a tag can be cached for. ArchiveConfig.TAGGINGS_COUNT_MIN_TIME = 1 @@ -17,15 +17,15 @@ @fandom_tag = FactoryBot.create(:fandom) end - context 'without updating taggings_count_cache' do - it 'should not cache tags which are not used much' do + context "without updating taggings_count_cache" do + it "should not cache tags which are not used much" do FactoryBot.create(:work, fandom_string: @fandom_tag.name) @fandom_tag.reload expect(@fandom_tag.taggings_count_cache).to eq 0 expect(@fandom_tag.taggings_count).to eq 1 end - it 'will start caching a when tag when that tag is used significantly' do + it "will start caching a when tag when that tag is used significantly" do (1..ArchiveConfig.TAGGINGS_COUNT_MIN_CACHE_COUNT).each do |try| FactoryBot.create(:work, fandom_string: @fandom_tag.name) @fandom_tag.reload @@ -40,8 +40,8 @@ end end - context 'updating taggings_count_cache' do - it 'should not cache tags which are not used much' do + context "updating taggings_count_cache" do + it "should not cache tags which are not used much" do FactoryBot.create(:work, fandom_string: @fandom_tag.name) RedisJobSpawner.perform_now("TagCountUpdateJob") @fandom_tag.reload @@ -49,7 +49,7 @@ expect(@fandom_tag.taggings_count).to eq 1 end - it 'will start caching a tag when that tag is used significantly' do + it "will start caching a tag when that tag is used significantly" do (1..ArchiveConfig.TAGGINGS_COUNT_MIN_CACHE_COUNT).each do |try| FactoryBot.create(:work, fandom_string: @fandom_tag.name) RedisJobSpawner.perform_now("TagCountUpdateJob") @@ -65,7 +65,7 @@ expect(@fandom_tag.taggings_count).to eq ArchiveConfig.TAGGINGS_COUNT_MIN_CACHE_COUNT end - it "Writes to the database do not happen immeadiately" do + it "Writes to the database do not happen immediately" do (1..40 * ArchiveConfig.TAGGINGS_COUNT_CACHE_DIVISOR - 1).each do |try| @fandom_tag.taggings_count = try @fandom_tag.reload @@ -254,20 +254,20 @@ def expect_tag_update_flag_in_redis_to_be(flag) expect(tag.save).to be_falsey end - it 'autocomplete should work' do - tag_character = FactoryBot.create(:character, canonical: true, name: 'kirk') - tag_fandom = FactoryBot.create(:fandom, name: 'Star Trek', canonical: true) + it "autocomplete should work" do + tag_character = FactoryBot.create(:character, canonical: true, name: "kirk") + tag_fandom = FactoryBot.create(:fandom, name: "Star Trek", canonical: true) tag_fandom.add_to_autocomplete - results = Tag.autocomplete_fandom_lookup(term: 'ki', fandom: 'Star Trek') + results = Tag.autocomplete_fandom_lookup(term: "ki", fandom: "Star Trek") expect(results.include?("#{tag_character.id}: #{tag_character.name}")).to be_truthy expect(results.include?("brave_sire_robin")).to be_falsey end - it 'old tag maker still works' do - tag_adult = Rating.create_canonical('adult', true) - tag_normal = ArchiveWarning.create_canonical('other') - expect(tag_adult.name).to eq('adult') - expect(tag_normal.name).to eq('other') + it "old tag maker still works" do + tag_adult = Rating.create_canonical("adult", true) + tag_normal = ArchiveWarning.create_canonical("other") + expect(tag_adult.name).to eq("adult") + expect(tag_normal.name).to eq("other") expect(tag_adult.adult).to be_truthy expect(tag_normal.adult).to be_falsey end From ed22ff2985eadd57a5e2b398bfa0177d186e858c Mon Sep 17 00:00:00 2001 From: nikobee Date: Mon, 20 Nov 2023 18:25:05 +0000 Subject: [PATCH 6/6] AO3-6626 define forbidden tags inline --- spec/models/tag_spec.rb | 16 +++++++++++++++- spec/spec_helper.rb | 1 - 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index 2730b18436c..340c2d87991 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -169,7 +169,21 @@ def expect_tag_update_flag_in_redis_to_be(flag) end context "tags using restricted characters should not be saved" do - BAD_TAGS.each do |tag| + [ + "bad, tag", + "also， bad", + "no、good", + "wild*card", + "back\\slash", + "lessertag", + "^tag", + "{open", + "close}", + "not=allowed", + "suspicious`character", + "no%maths" + ].each do |tag| forbidden_tag = Tag.new forbidden_tag.name = tag it "is not saved and receives an error message about restricted characters" do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 1bed282068e..b791b4e083d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -137,7 +137,6 @@ config.use_transactional_fixtures = true BAD_EMAILS = ["Abc.example.com", "A@b@c@example.com", 'a\"b(c)d,e:f;gi[j\k]l@example.com', 'this is"not\allowed@example.com', 'this\ still\"not/\/\allowed@example.com', "nodomain", "foo@oops", "ast*risk@example.com", "asterisk@ex*ample.com"].freeze - BAD_TAGS = ["bad, tag", "also， bad", "no、good", "wild*card", "lessertag", "^tag", "{open", "close}", "not=allowed", "suspicious`character", "no%maths"].freeze INVALID_URLS = %w[no_scheme.com ftp://ftp.address.com http://www.b@d!35.com https://www.b@d!35.com http://b@d!35.com https://www.b@d!35.com].freeze VALID_URLS = %w[http://rocksalt-recs.livejournal.com/196316.html https://rocksalt-recs.livejournal.com/196316.html].freeze INACTIVE_URLS = %w[https://www.iaminactive.com http://www.iaminactive.com https://iaminactive.com http://iaminactive.com].freeze