diff --git a/lib/onebox.rb b/lib/onebox.rb index feee7c8e6da..d5d1b9ae672 100644 --- a/lib/onebox.rb +++ b/lib/onebox.rb @@ -16,7 +16,7 @@ module Onebox DEFAULTS = { connect_timeout: 5, timeout: 10, - max_download_kb: (10 * 1024), # 10MB + max_download_kb: 2048, # 2MB load_paths: [File.join(Rails.root, "lib/onebox/templates")], allowed_ports: [80, 443], allowed_schemes: %w[http https], diff --git a/lib/onebox/engine/standard_embed.rb b/lib/onebox/engine/standard_embed.rb index 2473dfe5829..90bd59094a7 100644 --- a/lib/onebox/engine/standard_embed.rb +++ b/lib/onebox/engine/standard_embed.rb @@ -98,7 +98,13 @@ module Onebox ).first favicon = favicon.nil? ? nil : (favicon["href"].nil? ? nil : favicon["href"].strip) - Onebox::Helpers.get_absolute_image_url(favicon, url) + return nil if favicon.blank? + + absolute_url = Onebox::Helpers.get_absolute_image_url(favicon, url) + + return nil if absolute_url.length > UrlHelper::MAX_URL_LENGTH + + absolute_url end def get_description diff --git a/lib/url_helper.rb b/lib/url_helper.rb index eb973777d91..e299a8e3232 100644 --- a/lib/url_helper.rb +++ b/lib/url_helper.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class UrlHelper - MAX_URL_LENGTH = 100_000 + MAX_URL_LENGTH = 2_000 # At the moment this handles invalid URLs that browser address bar accepts # where second # is not encoded diff --git a/spec/lib/onebox/engine/standard_embed_spec.rb b/spec/lib/onebox/engine/standard_embed_spec.rb index 6c62ae5276e..55cffc4a9be 100644 --- a/spec/lib/onebox/engine/standard_embed_spec.rb +++ b/spec/lib/onebox/engine/standard_embed_spec.rb @@ -66,6 +66,34 @@ RSpec.describe Onebox::Engine::StandardEmbed do expect(instance.raw).to eq({ title: "do not override me" }) end + it "sets favicon URL" do + html_doc = + mocked_html_doc( + twitter_data: { + "name" => "twitter:url", + "content" => "cool.url", + }, + favicon_url: "https://favicon.co/default.ico", + ) + Onebox::Helpers.stubs(fetch_html_doc: html_doc) + + expect(instance.raw).to eq({ url: "cool.url", favicon: "https://favicon.co/default.ico" }) + end + + it "ignores suspiciously long favicon URLs" do + html_doc = + mocked_html_doc( + twitter_data: { + "name" => "twitter:url", + "content" => "cool.url", + }, + favicon_url: "https://favicon.co/#{"a" * 2_000}.ico", + ) + Onebox::Helpers.stubs(fetch_html_doc: html_doc) + + expect(instance.raw).to eq({ url: "cool.url" }) + end + it "sets oembed data" do Onebox::Helpers.stubs(fetch_html_doc: nil) Onebox::Oembed.any_instance.stubs(:data).returns({ description: "description" }) @@ -84,11 +112,11 @@ RSpec.describe Onebox::Engine::StandardEmbed do private - def mocked_html_doc(twitter_data: nil) + def mocked_html_doc(twitter_data: nil, favicon_url: nil) html_doc = mock html_doc.stubs(at_css: nil, at: nil) stub_twitter(html_doc, twitter_data) - stub_favicon(html_doc) + stub_favicon(html_doc, favicon_url) stub_json_ld html_doc end @@ -97,13 +125,13 @@ RSpec.describe Onebox::Engine::StandardEmbed do html_doc.expects(:css).with("meta").at_least_once.returns([twitter_data]) end - def stub_favicon(html_doc) + def stub_favicon(html_doc, favicon_url = nil) html_doc .stubs(:css) .with( 'link[rel="shortcut icon"], link[rel="icon shortcut"], link[rel="shortcut"], link[rel="icon"]', ) - .returns([]) + .returns([{ "href" => favicon_url }.compact]) end def stub_json_ld diff --git a/spec/lib/url_helper_spec.rb b/spec/lib/url_helper_spec.rb index e98d5ac829d..7bfad982594 100644 --- a/spec/lib/url_helper_spec.rb +++ b/spec/lib/url_helper_spec.rb @@ -166,7 +166,7 @@ RSpec.describe UrlHelper do end it "raises error if too long" do - expect do UrlHelper.normalized_encode("https://#{"a" * 100_000}.com") end.to raise_error( + expect do UrlHelper.normalized_encode("https://#{"a" * 2_000}.com") end.to raise_error( ArgumentError, ) end