From cbaf7c949bdb720149da9a8d5dfe34f58a837495 Mon Sep 17 00:00:00 2001
From: Bianca Nenciu <nbianca@users.noreply.github.com>
Date: Wed, 23 Mar 2022 17:36:08 +0200
Subject: [PATCH] FIX: Make sure max_oneboxes_per_post is enforced (#16215)

PostAnalyzer and CookedPostProcessor both replace URLs with oneboxes.
PostAnalyzer did not use the max_oneboxes_per_post site and setting and
CookedPostProcessor replaced at most max_oneboxes_per_post URLs ignoring
the oneboxes that were replaced already by PostAnalyzer.
---
 app/models/post_analyzer.rb            |  4 ++++
 lib/cooked_processor_mixin.rb          |  2 +-
 spec/lib/cooked_post_processor_spec.rb | 18 ++++++++++++++++--
 spec/models/post_analyzer_spec.rb      | 19 +++++++++++++++++++
 4 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/app/models/post_analyzer.rb b/app/models/post_analyzer.rb
index bd8d58e79b0..4030df5d592 100644
--- a/app/models/post_analyzer.rb
+++ b/app/models/post_analyzer.rb
@@ -31,7 +31,11 @@ class PostAnalyzer
       cooked = PrettyText.cook(raw, opts)
     end
 
+    limit = SiteSetting.max_oneboxes_per_post
     result = Oneboxer.apply(cooked) do |url|
+      next if limit <= 0
+      limit -= 1
+
       @onebox_urls << url
       if opts[:invalidate_oneboxes]
         Oneboxer.invalidate(url)
diff --git a/lib/cooked_processor_mixin.rb b/lib/cooked_processor_mixin.rb
index 7c6c8ed1faf..9985c1684b1 100644
--- a/lib/cooked_processor_mixin.rb
+++ b/lib/cooked_processor_mixin.rb
@@ -3,7 +3,7 @@
 module CookedProcessorMixin
 
   def post_process_oneboxes
-    limit = SiteSetting.max_oneboxes_per_post
+    limit = SiteSetting.max_oneboxes_per_post - @doc.css("aside.onebox, a.inline-onebox").size
     oneboxes = {}
     inlineOneboxes = {}
 
diff --git a/spec/lib/cooked_post_processor_spec.rb b/spec/lib/cooked_post_processor_spec.rb
index 72e96274e35..7800930a8c0 100644
--- a/spec/lib/cooked_post_processor_spec.rb
+++ b/spec/lib/cooked_post_processor_spec.rb
@@ -56,6 +56,20 @@ describe CookedPostProcessor do
 
       before do
         SiteSetting.enable_inline_onebox_on_all_domains = true
+        Oneboxer.stubs(:cached_onebox).with(url).returns <<~HTML
+          <aside class="onebox allowlistedgeneric" data-onebox-src="https://meta.discourse.org/t/mini-inline-onebox-support-rfc/66400">
+            <header class="source">
+              <a href="https://meta.discourse.org/t/mini-inline-onebox-support-rfc/66400" target="_blank" rel="noopener">meta.discourse.org</a>
+            </header>
+            <article class="onebox-body">
+              <h3><a href="https://meta.discourse.org/t/mini-inline-onebox-support-rfc/66400" target="_blank" rel="noopener">some title</a></h3>
+              <p>some description</p>
+            </article>
+            <div class="onebox-metadata"></div>
+            <div style="clear: both"></div>
+          </aside>
+        HTML
+        Oneboxer.stubs(:cached_onebox).with(not_oneboxed_url).returns(nil)
 
         %i{head get}.each do |method|
           stub_request(method, url).to_return(
@@ -90,11 +104,11 @@ describe CookedPostProcessor do
           count: 2
         )
 
-        expect(cpp.html).to have_tag('aside.onebox a', text: title, count: 2)
+        expect(cpp.html).to have_tag('aside.onebox a', text: title, count: 1)
 
         expect(cpp.html).to have_tag('aside.onebox a',
           text: url_hostname,
-          count: 2
+          count: 1
         )
 
         expect(cpp.html).to have_tag('a',
diff --git a/spec/models/post_analyzer_spec.rb b/spec/models/post_analyzer_spec.rb
index c0813d5343e..b82f8d0d4c7 100644
--- a/spec/models/post_analyzer_spec.rb
+++ b/spec/models/post_analyzer_spec.rb
@@ -58,6 +58,25 @@ describe PostAnalyzer do
       cooked = post_analyzer.cook('*this is italic*')
       expect(cooked).to eq('<p><em>this is italic</em></p>')
     end
+
+    it 'should respect SiteSetting.max_oneboxes_per_post' do
+      SiteSetting.max_oneboxes_per_post = 2
+      Oneboxer.expects(:cached_onebox).with(url).returns('something').twice
+
+      cooked = post_analyzer.cook(<<~RAW)
+        #{url}
+
+        #{url}
+
+        #{url}
+      RAW
+
+      expect(cooked).to match_html(<<~HTML)
+        <p>something</p>
+        <p>something</p>
+        <p><a href="#{url}" class="onebox" target="_blank" rel="noopener nofollow ugc">#{url}</a></p>
+      HTML
+    end
   end
 
   context "links" do