Improve handling of encoding problems when creating link previews (#30929)
This commit is contained in:
		
					parent
					
						
							
								05f0d51005
							
						
					
				
			
			
				commit
				
					
						016c1e4e78
					
				
			
		
					 5 changed files with 76 additions and 10 deletions
				
			
		|  | @ -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 | ||||
|  |  | |||
|  | @ -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 | ||||
|  |  | |||
							
								
								
									
										17
									
								
								spec/fixtures/requests/latin1_posing_as_utf8_broken.txt
									
										
									
									
										vendored
									
									
										Normal file
									
								
							
							
						
						
									
										17
									
								
								spec/fixtures/requests/latin1_posing_as_utf8_broken.txt
									
										
									
									
										vendored
									
									
										Normal file
									
								
							|  | @ -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 | ||||
| 
 | ||||
| <!doctype html> | ||||
| <html lang="en"> | ||||
| <head> | ||||
|   <meta charset="UTF-8"> | ||||
|   <title>Tofu á l'orange</title> | ||||
| </head> | ||||
| <body> | ||||
|   <h2>Tofu á l'orange</h2> | ||||
| </body> | ||||
| </html> | ||||
							
								
								
									
										17
									
								
								spec/fixtures/requests/latin1_posing_as_utf8_recoverable.txt
									
										
									
									
										vendored
									
									
										Normal file
									
								
							
							
						
						
									
										17
									
								
								spec/fixtures/requests/latin1_posing_as_utf8_recoverable.txt
									
										
									
									
										vendored
									
									
										Normal file
									
								
							|  | @ -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 | ||||
| 
 | ||||
| <!doctype html> | ||||
| <html lang="en"> | ||||
| <head> | ||||
|   <meta charset="UTF-8"> | ||||
|   <title>Tofu with orange sauce</title> | ||||
| </head> | ||||
| <body> | ||||
|   <h2>Tofu á l'orange</h2> | ||||
| </body> | ||||
| </html> | ||||
|  | @ -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 | ||||
| 
 | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue