From 10f77556cd65de4c7c4ec1081604f1e81f93ead1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Mon, 6 May 2024 17:01:05 +0200 Subject: [PATCH] FIX: ensure no infinite category loop If there's ever a circular reference in categories, don't go into an infinite loop when generating the category slug. Instead, keep track of parent ids, and bail out as soon as we're encountering one more than once. --- app/models/category.rb | 10 ++++++---- spec/models/category_spec.rb | 35 +++++++++++++++++++++++++++++++---- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/app/models/category.rb b/app/models/category.rb index 8022f580463..75686ccc39c 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -1091,11 +1091,13 @@ class Category < ActiveRecord::Base .find_each { |category| category.create_category_definition } end - def slug_path + def slug_path(parent_ids = Set.new) if self.parent_category_id.present? - slug_path = self.parent_category.slug_path - slug_path.push(self.slug_for_url) - slug_path + if parent_ids.add?(self.parent_category_id) + self.parent_category.slug_path(parent_ids) << self.slug_for_url + else + [] + end else [self.slug_for_url] end diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb index cdd1b4372de..006bf280595 100644 --- a/spec/models/category_spec.rb +++ b/spec/models/category_spec.rb @@ -47,8 +47,7 @@ RSpec.describe Category do it "should delete associated sidebar_section_links when category is destroyed" do category_sidebar_section_link = Fabricate(:category_sidebar_section_link) - category_sidebar_section_link_2 = - Fabricate(:category_sidebar_section_link, linkable: category_sidebar_section_link.linkable) + Fabricate(:category_sidebar_section_link, linkable: category_sidebar_section_link.linkable) tag_sidebar_section_link = Fabricate(:tag_sidebar_section_link) expect { category_sidebar_section_link.linkable.destroy! }.to change { @@ -996,7 +995,7 @@ RSpec.describe Category do ) category.clear_auto_bump_cache! - post1 = create_post(category: category, created_at: 15.seconds.ago) + create_post(category: category, created_at: 15.seconds.ago) # no limits on post creation or category creation please RateLimiter.enable @@ -1419,7 +1418,7 @@ RSpec.describe Category do it 'returns nil when input is ["category:invalid-slug:sub-subcategory"] and maximum category nesting is 3' do SiteSetting.max_category_nesting = 3 - sub_subcategory = Fabricate(:category, parent_category: subcategory, slug: "sub-subcategory") + Fabricate(:category, parent_category: subcategory, slug: "sub-subcategory") expect(Category.ids_from_slugs(%w[category:invalid-slug:sub-subcategory])).to eq([]) end @@ -1464,6 +1463,34 @@ RSpec.describe Category do end end + describe "#slug_path" do + before { SiteSetting.max_category_nesting = 3 } + + fab!(:grandparent) { Fabricate(:category, slug: "foo") } + fab!(:parent) { Fabricate(:category, parent_category: grandparent, slug: "bar") } + let(:child) { Fabricate(:category, parent_category: parent, slug: "boo") } + + it "returns the slug for categories without parents" do + expect(grandparent.slug_path).to eq [grandparent.slug] + end + + it "returns the slug for categories with parent" do + expect(parent.slug_path).to eq [grandparent.slug, parent.slug] + end + + it "returns the slug for categories with grand-parent" do + expect(child.slug_path).to eq [grandparent.slug, parent.slug, child.slug] + end + + it "avoids infinite loops with circular references" do + grandparent.parent_category = parent + grandparent.save!(validate: false) + + expect(grandparent.slug_path).to eq [parent.slug, grandparent.slug] + expect(parent.slug_path).to eq [grandparent.slug, parent.slug] + end + end + describe "#slug_ref" do fab!(:category) { Fabricate(:category, slug: "foo") }