From 360d3c2a9ca43017554b1712451896f23372434d Mon Sep 17 00:00:00 2001 From: miteruzo Date: Fri, 10 Jul 2026 01:16:08 +0900 Subject: [PATCH] #398 --- backend/app/services/preview/http_fetcher.rb | 26 ++++++++-- .../app/services/preview/thumbnail_fetcher.rb | 15 ++++-- backend/app/services/preview/url_safety.rb | 2 + .../services/preview/http_fetcher_spec.rb | 23 +++++++++ .../preview/thumbnail_fetcher_spec.rb | 51 +++++++++++++++++++ .../spec/services/preview/url_safety_spec.rb | 15 ++++++ 6 files changed, 125 insertions(+), 7 deletions(-) create mode 100644 backend/spec/services/preview/http_fetcher_spec.rb create mode 100644 backend/spec/services/preview/thumbnail_fetcher_spec.rb create mode 100644 backend/spec/services/preview/url_safety_spec.rb diff --git a/backend/app/services/preview/http_fetcher.rb b/backend/app/services/preview/http_fetcher.rb index b88f91a..30c1644 100644 --- a/backend/app/services/preview/http_fetcher.rb +++ b/backend/app/services/preview/http_fetcher.rb @@ -28,11 +28,29 @@ module Preview raise FetchFailed, 'redirect が多すぎます．' end - raise FetchFailed, 'redirect 先が不正です．' if location.blank? + if location.blank? + log_failure(:blank_redirect_location, + url: uri.to_s, + redirects:, + content_type: response['content-type'], + content_length: response['content-length']) + raise FetchFailed, 'redirect 先が不正です．' + end - return fetch(URI.join(uri, location).to_s, - max_bytes:, - redirects: redirects - 1) + redirect_url = + begin + URI.join(uri, location).to_s + rescue URI::InvalidURIError => e + log_failure(:invalid_redirect_location, + url: uri.to_s, + redirects:, + location:, + error: e.class.name, + message: e.message) + raise FetchFailed, 'redirect 先が不正です．' + end + + return fetch(redirect_url, max_bytes:, redirects: redirects - 1) end unless response.is_a?(Net::HTTPSuccess) diff --git a/backend/app/services/preview/thumbnail_fetcher.rb b/backend/app/services/preview/thumbnail_fetcher.rb index 4df99ad..746aa13 100644 --- a/backend/app/services/preview/thumbnail_fetcher.rb +++ b/backend/app/services/preview/thumbnail_fetcher.rb @@ -1,6 +1,9 @@ module Preview class ThumbnailFetcher class GenerationFailed < StandardError; end + ALLOWED_IMAGE_CONTENT_TYPES = [ + 'image/jpeg', 'image/png', 'image/gif', 'image/webp' + ].freeze HTML_MAX_BYTES = 1.megabyte NICONICO_XML_MAX_BYTES = 256.kilobytes @@ -32,7 +35,7 @@ module Preview return nil if url.blank? response = HttpFetcher.fetch(url) - return nil unless response.content_type.downcase.start_with?('image/') + return nil unless allowed_image_content_type?(response.content_type) response.body rescue HttpFetcher::FetchTimeout @@ -43,7 +46,7 @@ module Preview def self.fetch_image!(url) response = HttpFetcher.fetch(url) - unless response.content_type.downcase.start_with?('image/') + unless allowed_image_content_type?(response.content_type) raise GenerationFailed, 'サムネール画像が見つかりませんでした．' end @@ -80,7 +83,13 @@ module Preview nil end + def self.allowed_image_content_type?(content_type) + mime_type = content_type.to_s.split(';', 2).first.downcase.strip + ALLOWED_IMAGE_CONTENT_TYPES.include?(mime_type) + end + private_class_method :fetch_image_or_nil, :fetch_image!, - :niconico_thumbnail_url + :niconico_thumbnail_url, + :allowed_image_content_type? end end diff --git a/backend/app/services/preview/url_safety.rb b/backend/app/services/preview/url_safety.rb index 34e61c2..e150f19 100644 --- a/backend/app/services/preview/url_safety.rb +++ b/backend/app/services/preview/url_safety.rb @@ -40,6 +40,8 @@ module Preview end [uri, parsed_addresses.map(&:to_s)] + rescue Resolv::ResolvError + raise UnsafeUrl, 'URL のホストを解決できません．' rescue URI::InvalidURIError, IPAddr::InvalidAddressError raise UnsafeUrl, 'URL が不正です．' end diff --git a/backend/spec/services/preview/http_fetcher_spec.rb b/backend/spec/services/preview/http_fetcher_spec.rb new file mode 100644 index 0000000..c166562 --- /dev/null +++ b/backend/spec/services/preview/http_fetcher_spec.rb @@ -0,0 +1,23 @@ +require 'rails_helper' + +RSpec.describe Preview::HttpFetcher do + describe '.fetch' do + it 'raises FetchFailed when redirect location is invalid' do + redirect = Net::HTTPFound.new('1.1', '302', 'Found') + redirect['location'] = 'http://[invalid' + + allow(Preview::UrlSafety).to receive(:validate) + .with('https://example.com/page') + .and_return([URI.parse('https://example.com/page'), ['203.0.113.10']]) + allow(described_class).to receive(:request) + .and_return(redirect) + allow(Rails.logger).to receive(:warn) + + expect { + described_class.fetch('https://example.com/page') + }.to raise_error(Preview::HttpFetcher::FetchFailed) + expect(Rails.logger).to have_received(:warn) + .with(/invalid_redirect_location/) + end + end +end diff --git a/backend/spec/services/preview/thumbnail_fetcher_spec.rb b/backend/spec/services/preview/thumbnail_fetcher_spec.rb new file mode 100644 index 0000000..3088e0d --- /dev/null +++ b/backend/spec/services/preview/thumbnail_fetcher_spec.rb @@ -0,0 +1,51 @@ +require 'rails_helper' + +RSpec.describe Preview::ThumbnailFetcher do + describe '.fetch' do + it 'rejects svg thumbnails' do + page = Preview::HttpFetcher::Response.new( + '', + 'text/html', + 'https://example.com/page') + svg = Preview::HttpFetcher::Response.new( + '', + 'image/svg+xml', + 'https://example.com/thumb.svg') + + allow(Preview::UrlSafety).to receive(:validate) + .and_return([URI.parse('https://example.com/page'), ['203.0.113.10']]) + allow(Preview::HttpFetcher).to receive(:fetch) + .with('https://example.com/page', max_bytes: described_class::HTML_MAX_BYTES) + .and_return(page) + allow(Preview::HttpFetcher).to receive(:fetch) + .with('https://example.com/thumb.svg') + .and_return(svg) + + expect { + described_class.fetch('https://example.com/page') + }.to raise_error(Preview::ThumbnailFetcher::GenerationFailed) + end + + it 'accepts allowed image content type with parameters' do + page = Preview::HttpFetcher::Response.new( + '', + 'text/html', + 'https://example.com/page') + image = Preview::HttpFetcher::Response.new( + 'jpeg-bytes', + 'image/jpeg; charset=binary', + 'https://example.com/thumb.jpg') + + allow(Preview::UrlSafety).to receive(:validate) + .and_return([URI.parse('https://example.com/page'), ['203.0.113.10']]) + allow(Preview::HttpFetcher).to receive(:fetch) + .with('https://example.com/page', max_bytes: described_class::HTML_MAX_BYTES) + .and_return(page) + allow(Preview::HttpFetcher).to receive(:fetch) + .with('https://example.com/thumb.jpg') + .and_return(image) + + expect(described_class.fetch('https://example.com/page')).to eq('jpeg-bytes') + end + end +end diff --git a/backend/spec/services/preview/url_safety_spec.rb b/backend/spec/services/preview/url_safety_spec.rb new file mode 100644 index 0000000..a1002f1 --- /dev/null +++ b/backend/spec/services/preview/url_safety_spec.rb @@ -0,0 +1,15 @@ +require 'rails_helper' + +RSpec.describe Preview::UrlSafety do + describe '.validate' do + it 'raises UnsafeUrl when DNS resolution fails' do + allow(Resolv).to receive(:getaddresses) + .with('missing.example') + .and_raise(Resolv::ResolvError) + + expect { + described_class.validate('https://missing.example') + }.to raise_error(Preview::UrlSafety::UnsafeUrl) + end + end +end