#398

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)

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

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

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

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(
+          '<meta property="og:image" content="https://example.com/thumb.svg">',
+          'text/html',
+          'https://example.com/page')
+      svg = Preview::HttpFetcher::Response.new(
+          '<svg></svg>',
+          '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(
+          '<meta property="og:image" content="https://example.com/thumb.jpg">',
+          '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

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