From a0ea2fa3b01c61d4d390b5e4b6cddc8006b204ec Mon Sep 17 00:00:00 2001
From: Mike Dalessio <mike.dalessio@gmail.com>
Date: Mon, 9 Sep 2024 06:59:42 -0400
Subject: [PATCH] Change fetch link card service to parse as HTML5 (#31814)

---
 app/lib/link_details_extractor.rb             | 36 +++++++++----------
 app/services/fetch_link_card_service.rb       |  4 +--
 spec/services/fetch_link_card_service_spec.rb |  4 +--
 3 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/app/lib/link_details_extractor.rb b/app/lib/link_details_extractor.rb
index bd78aef7a..dff57f74f 100644
--- a/app/lib/link_details_extractor.rb
+++ b/app/lib/link_details_extractor.rb
@@ -157,11 +157,11 @@ class LinkDetailsExtractor
   end
 
   def title
-    html_entities_decode(structured_data&.headline || opengraph_tag('og:title') || document.xpath('//title').map(&:content).first)&.strip
+    html_entities.decode(structured_data&.headline || opengraph_tag('og:title') || document.xpath('//title').map(&:content).first)&.strip
   end
 
   def description
-    html_entities_decode(structured_data&.description || opengraph_tag('og:description') || meta_tag('description'))
+    html_entities.decode(structured_data&.description || opengraph_tag('og:description') || meta_tag('description'))
   end
 
   def published_at
@@ -181,7 +181,7 @@ class LinkDetailsExtractor
   end
 
   def provider_name
-    html_entities_decode(structured_data&.publisher_name || opengraph_tag('og:site_name'))
+    html_entities.decode(structured_data&.publisher_name || opengraph_tag('og:site_name'))
   end
 
   def provider_url
@@ -189,7 +189,7 @@ class LinkDetailsExtractor
   end
 
   def author_name
-    html_entities_decode(structured_data&.author_name || opengraph_tag('og:author') || opengraph_tag('og:author:username'))
+    html_entities.decode(structured_data&.author_name || opengraph_tag('og:author') || opengraph_tag('og:author:username'))
   end
 
   def author_url
@@ -258,7 +258,7 @@ class LinkDetailsExtractor
 
       next if json_ld.blank?
 
-      structured_data = StructuredData.new(html_entities_decode(json_ld))
+      structured_data = StructuredData.new(html_entities.decode(json_ld))
 
       next unless structured_data.valid?
 
@@ -274,11 +274,20 @@ class LinkDetailsExtractor
   end
 
   def detect_encoding_and_parse_document
-    [detect_encoding, nil, header_encoding].uniq.each do |encoding|
-      document = Nokogiri::HTML(@html, nil, encoding)
-      return document if document.to_s.valid_encoding?
+    html = nil
+    encoding = nil
+
+    [detect_encoding, header_encoding].compact.each do |enc|
+      html = @html.dup.force_encoding(enc)
+      if html.valid_encoding?
+        encoding = enc
+        break
+      end
     end
-    Nokogiri::HTML(@html, nil, 'UTF-8')
+
+    html = @html unless encoding
+
+    Nokogiri::HTML5(html, nil, encoding)
   end
 
   def detect_encoding
@@ -299,15 +308,6 @@ class LinkDetailsExtractor
     end
   end
 
-  def html_entities_decode(string)
-    return if string.nil?
-
-    unicode_string = string.to_s.encode('UTF-8')
-    raise EncodingError, 'cannot convert string to valid UTF-8' unless unicode_string.valid_encoding?
-
-    html_entities.decode(unicode_string)
-  end
-
   def html_entities
     @html_entities ||= HTMLEntities.new(:expanded)
   end
diff --git a/app/services/fetch_link_card_service.rb b/app/services/fetch_link_card_service.rb
index 49b820557..36d5c490a 100644
--- a/app/services/fetch_link_card_service.rb
+++ b/app/services/fetch_link_card_service.rb
@@ -29,7 +29,7 @@ class FetchLinkCardService < BaseService
     end
 
     attach_card if @card&.persisted?
-  rescue HTTP::Error, OpenSSL::SSL::SSLError, Addressable::URI::InvalidURIError, Mastodon::HostValidationError, Mastodon::LengthValidationError, EncodingError, ActiveRecord::RecordInvalid => e
+  rescue HTTP::Error, OpenSSL::SSL::SSLError, Addressable::URI::InvalidURIError, Mastodon::HostValidationError, Mastodon::LengthValidationError, Encoding::UndefinedConversionError, ActiveRecord::RecordInvalid => e
     Rails.logger.debug { "Error fetching link #{@original_url}: #{e}" }
     nil
   end
@@ -80,7 +80,7 @@ class FetchLinkCardService < BaseService
     urls = if @status.local?
              @status.text.scan(URL_PATTERN).map { |array| Addressable::URI.parse(array[1]).normalize }
            else
-             document = Nokogiri::HTML(@status.text)
+             document = Nokogiri::HTML5(@status.text)
              links = document.css('a')
 
              links.filter_map { |a| Addressable::URI.parse(a['href']) unless skip_link?(a) }.filter_map(&:normalize)
diff --git a/spec/services/fetch_link_card_service_spec.rb b/spec/services/fetch_link_card_service_spec.rb
index 2f64f4055..1d61e33c0 100644
--- a/spec/services/fetch_link_card_service_spec.rb
+++ b/spec/services/fetch_link_card_service_spec.rb
@@ -192,8 +192,8 @@ RSpec.describe FetchLinkCardService do
         context 'when encoding problems appear in title tag' do
           let(:status) { Fabricate(:status, text: 'Check out http://example.com/latin1_posing_as_utf8_broken') }
 
-          it 'does not create a preview card' do
-            expect(status.preview_card).to be_nil
+          it 'creates a preview card anyway that replaces invalid bytes with U+FFFD (replacement char)' do
+            expect(status.preview_card.title).to eq("Tofu � l'orange")
           end
         end
       end