From 016c1e4e788890f0c81a47640f76de136a0a8f32 Mon Sep 17 00:00:00 2001 From: David Roetzel Date: Fri, 5 Jul 2024 13:54:38 +0200 Subject: [PATCH] Improve handling of encoding problems when creating link previews (#30929) --- app/lib/link_details_extractor.rb | 22 +++++++++++---- app/services/fetch_link_card_service.rb | 2 +- .../requests/latin1_posing_as_utf8_broken.txt | 17 +++++++++++ .../latin1_posing_as_utf8_recoverable.txt | 17 +++++++++++ spec/services/fetch_link_card_service_spec.rb | 28 +++++++++++++++++-- 5 files changed, 76 insertions(+), 10 deletions(-) create mode 100644 spec/fixtures/requests/latin1_posing_as_utf8_broken.txt create mode 100644 spec/fixtures/requests/latin1_posing_as_utf8_recoverable.txt diff --git a/app/lib/link_details_extractor.rb b/app/lib/link_details_extractor.rb index dbfdd33fcc..9436d20b58 100644 --- a/app/lib/link_details_extractor.rb +++ b/app/lib/link_details_extractor.rb @@ -156,11 +156,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 @@ -180,7 +180,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 @@ -188,7 +188,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 @@ -257,7 +257,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? @@ -273,10 +273,11 @@ class LinkDetailsExtractor end def detect_encoding_and_parse_document - [detect_encoding, nil, @html_charset, 'UTF-8'].uniq.each do |encoding| + [detect_encoding, nil, @html_charset].uniq.each do |encoding| document = Nokogiri::HTML(@html, nil, encoding) return document if document.to_s.valid_encoding? end + Nokogiri::HTML(@html, nil, 'UTF-8') end def detect_encoding @@ -290,6 +291,15 @@ class LinkDetailsExtractor end end + def html_entities_decode(string) + return if string.nil? + + unicode_string = string.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 8bc9f912c5..436e024c99 100644 --- a/app/services/fetch_link_card_service.rb +++ b/app/services/fetch_link_card_service.rb @@ -32,7 +32,7 @@ class FetchLinkCardService < BaseService end attach_card if @card&.persisted? - rescue HTTP::Error, OpenSSL::SSL::SSLError, Addressable::URI::InvalidURIError, Mastodon::HostValidationError, Mastodon::LengthValidationError, Encoding::UndefinedConversionError => e + rescue HTTP::Error, OpenSSL::SSL::SSLError, Addressable::URI::InvalidURIError, Mastodon::HostValidationError, Mastodon::LengthValidationError, EncodingError => e Rails.logger.debug { "Error fetching link #{@original_url}: #{e}" } nil end diff --git a/spec/fixtures/requests/latin1_posing_as_utf8_broken.txt b/spec/fixtures/requests/latin1_posing_as_utf8_broken.txt new file mode 100644 index 0000000000..ed8a4716a3 --- /dev/null +++ b/spec/fixtures/requests/latin1_posing_as_utf8_broken.txt @@ -0,0 +1,17 @@ +HTTP/1.1 200 OK +server: nginx +date: Thu, 13 Jun 2024 14:33:13 GMT +content-type: text/html; charset=utf-8 +content-length: 158 +accept-ranges: bytes + + + + + + Tofu á l'orange + + +

Tofu á l'orange

+ + diff --git a/spec/fixtures/requests/latin1_posing_as_utf8_recoverable.txt b/spec/fixtures/requests/latin1_posing_as_utf8_recoverable.txt new file mode 100644 index 0000000000..a24985832c --- /dev/null +++ b/spec/fixtures/requests/latin1_posing_as_utf8_recoverable.txt @@ -0,0 +1,17 @@ +HTTP/1.1 200 OK +server: nginx +date: Thu, 13 Jun 2024 14:33:13 GMT +content-type: text/html; charset=utf-8 +content-length: 158 +accept-ranges: bytes + + + + + + Tofu with orange sauce + + +

Tofu á l'orange

+ + diff --git a/spec/services/fetch_link_card_service_spec.rb b/spec/services/fetch_link_card_service_spec.rb index d83a527514..547737b614 100644 --- a/spec/services/fetch_link_card_service_spec.rb +++ b/spec/services/fetch_link_card_service_spec.rb @@ -27,6 +27,8 @@ RSpec.describe FetchLinkCardService do stub_request(:get, 'http://example.com/koi8-r').to_return(request_fixture('koi8-r.txt')) stub_request(:get, 'http://example.com/windows-1251').to_return(request_fixture('windows-1251.txt')) stub_request(:get, 'http://example.com/low_confidence_latin1').to_return(request_fixture('low_confidence_latin1.txt')) + stub_request(:get, 'http://example.com/latin1_posing_as_utf8_broken').to_return(request_fixture('latin1_posing_as_utf8_broken.txt')) + stub_request(:get, 'http://example.com/latin1_posing_as_utf8_recoverable').to_return(request_fixture('latin1_posing_as_utf8_recoverable.txt')) stub_request(:get, 'http://example.com/aergerliche-umlaute').to_return(request_fixture('redirect_with_utf8_url.txt')) Rails.cache.write('oembed_endpoint:example.com', oembed_cache) if oembed_cache @@ -159,10 +161,30 @@ RSpec.describe FetchLinkCardService do end context 'with a URL of a page in ISO-8859-1 encoding, that charlock_holmes cannot detect' do - let(:status) { Fabricate(:status, text: 'Check out http://example.com/low_confidence_latin1') } + context 'when encoding in http header is correct' do + let(:status) { Fabricate(:status, text: 'Check out http://example.com/low_confidence_latin1') } - it 'decodes the HTML' do - expect(status.preview_card.title).to eq("Tofu á l'orange") + it 'decodes the HTML' do + expect(status.preview_card.title).to eq("Tofu á l'orange") + end + end + + context 'when encoding in http header is incorrect' do + context 'when encoding problems appear in unrelated tags' do + let(:status) { Fabricate(:status, text: 'Check out http://example.com/latin1_posing_as_utf8_recoverable') } + + it 'decodes the HTML' do + expect(status.preview_card.title).to eq('Tofu with orange sauce') + end + end + + 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 + end + end end end