about summary refs log tree commit diff
path: root/app
diff options
context:
space:
mode:
authorAkihiko Odaki <akihiko.odaki.4i@stu.hosei.ac.jp>2018-03-26 21:02:10 +0900
committerEugen Rochko <eugen@zeonfederated.com>2018-03-26 14:02:10 +0200
commit40e5d2303ba1edc51beae66cc15263675980106a (patch)
tree42364f04c30bab43a27cc6ea17173ae825cad153 /app
parent18965cb0e611b226c6252f1669f228f5b95f1ac6 (diff)
Validate HTTP response length while receiving (#6891)
to_s method of HTTP::Response keeps blocking while it receives the whole
content, no matter how it is big. This means it may waste time to receive
unacceptably large files. It may also consume memory and disk in the
process. This solves the inefficency by checking response length while
receiving.
Diffstat (limited to 'app')
-rw-r--r--app/helpers/jsonld_helper.rb2
-rw-r--r--app/lib/exceptions.rb1
-rw-r--r--app/lib/provider_discovery.rb2
-rw-r--r--app/lib/request.rb31
-rw-r--r--app/models/account.rb1
-rw-r--r--app/models/application_record.rb1
-rw-r--r--app/models/concerns/account_avatar.rb4
-rw-r--r--app/models/concerns/account_header.rb4
-rw-r--r--app/models/concerns/remotable.rb6
-rw-r--r--app/models/custom_emoji.rb6
-rw-r--r--app/models/media_attachment.rb7
-rw-r--r--app/models/preview_card.rb5
-rw-r--r--app/services/fetch_atom_service.rb11
-rw-r--r--app/services/fetch_link_card_service.rb2
-rw-r--r--app/services/resolve_account_service.rb2
-rw-r--r--app/workers/pubsubhubbub/confirmation_worker.rb2
16 files changed, 62 insertions, 25 deletions
diff --git a/app/helpers/jsonld_helper.rb b/app/helpers/jsonld_helper.rb
index 957a2cbc9..dfb8fcb8b 100644
--- a/app/helpers/jsonld_helper.rb
+++ b/app/helpers/jsonld_helper.rb
@@ -61,7 +61,7 @@ module JsonLdHelper
 
   def fetch_resource_without_id_validation(uri)
     build_request(uri).perform do |response|
-      response.code == 200 ? body_to_json(response.to_s) : nil
+      response.code == 200 ? body_to_json(response.body_with_limit) : nil
     end
   end
 
diff --git a/app/lib/exceptions.rb b/app/lib/exceptions.rb
index 95e3365c2..e88e98eae 100644
--- a/app/lib/exceptions.rb
+++ b/app/lib/exceptions.rb
@@ -5,6 +5,7 @@ module Mastodon
   class NotPermittedError < Error; end
   class ValidationError < Error; end
   class HostValidationError < ValidationError; end
+  class LengthValidationError < ValidationError; end
   class RaceConditionError < Error; end
 
   class UnexpectedResponseError < Error
diff --git a/app/lib/provider_discovery.rb b/app/lib/provider_discovery.rb
index bbd3a2d43..3bec7211b 100644
--- a/app/lib/provider_discovery.rb
+++ b/app/lib/provider_discovery.rb
@@ -18,7 +18,7 @@ class ProviderDiscovery < OEmbed::ProviderDiscovery
              else
                Request.new(:get, url).perform do |res|
                  raise OEmbed::NotFound, url if res.code != 200 || res.mime_type != 'text/html'
-                 Nokogiri::HTML(res.to_s)
+                 Nokogiri::HTML(res.body_with_limit)
                end
              end
 
diff --git a/app/lib/request.rb b/app/lib/request.rb
index 8a127c65f..dca93a6e9 100644
--- a/app/lib/request.rb
+++ b/app/lib/request.rb
@@ -40,7 +40,7 @@ class Request
     end
 
     begin
-      yield response
+      yield response.extend(ClientLimit)
     ensure
       http_client.close
     end
@@ -99,6 +99,33 @@ class Request
     @http_client ||= HTTP.timeout(:per_operation, timeout).follow(max_hops: 2)
   end
 
+  module ClientLimit
+    def body_with_limit(limit = 1.megabyte)
+      raise Mastodon::LengthValidationError if content_length.present? && content_length > limit
+
+      if charset.nil?
+        encoding = Encoding::BINARY
+      else
+        begin
+          encoding = Encoding.find(charset)
+        rescue ArgumentError
+          encoding = Encoding::BINARY
+        end
+      end
+
+      contents = String.new(encoding: encoding)
+
+      while (chunk = readpartial)
+        contents << chunk
+        chunk.clear
+
+        raise Mastodon::LengthValidationError if contents.bytesize > limit
+      end
+
+      contents
+    end
+  end
+
   class Socket < TCPSocket
     class << self
       def open(host, *args)
@@ -118,5 +145,5 @@ class Request
     end
   end
 
-  private_constant :Socket
+  private_constant :ClientLimit, :Socket
 end
diff --git a/app/models/account.rb b/app/models/account.rb
index 9a83d979f..25e7d7436 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -55,7 +55,6 @@ class Account < ApplicationRecord
   include AccountHeader
   include AccountInteractions
   include Attachmentable
-  include Remotable
   include Paginable
 
   enum protocol: [:ostatus, :activitypub]
diff --git a/app/models/application_record.rb b/app/models/application_record.rb
index 71fbba5b3..83134d41a 100644
--- a/app/models/application_record.rb
+++ b/app/models/application_record.rb
@@ -2,4 +2,5 @@
 
 class ApplicationRecord < ActiveRecord::Base
   self.abstract_class = true
+  include Remotable
 end
diff --git a/app/models/concerns/account_avatar.rb b/app/models/concerns/account_avatar.rb
index 9e34a9461..2d5ebfca3 100644
--- a/app/models/concerns/account_avatar.rb
+++ b/app/models/concerns/account_avatar.rb
@@ -4,6 +4,7 @@ module AccountAvatar
   extend ActiveSupport::Concern
 
   IMAGE_MIME_TYPES = ['image/jpeg', 'image/png', 'image/gif'].freeze
+  LIMIT = 2.megabytes
 
   class_methods do
     def avatar_styles(file)
@@ -19,7 +20,8 @@ module AccountAvatar
     # Avatar upload
     has_attached_file :avatar, styles: ->(f) { avatar_styles(f) }, convert_options: { all: '-strip' }, processors: [:lazy_thumbnail]
     validates_attachment_content_type :avatar, content_type: IMAGE_MIME_TYPES
-    validates_attachment_size :avatar, less_than: 2.megabytes
+    validates_attachment_size :avatar, less_than: LIMIT
+    remotable_attachment :avatar, LIMIT
   end
 
   def avatar_original_url
diff --git a/app/models/concerns/account_header.rb b/app/models/concerns/account_header.rb
index 04c576b28..ef40b8126 100644
--- a/app/models/concerns/account_header.rb
+++ b/app/models/concerns/account_header.rb
@@ -4,6 +4,7 @@ module AccountHeader
   extend ActiveSupport::Concern
 
   IMAGE_MIME_TYPES = ['image/jpeg', 'image/png', 'image/gif'].freeze
+  LIMIT = 2.megabytes
 
   class_methods do
     def header_styles(file)
@@ -19,7 +20,8 @@ module AccountHeader
     # Header upload
     has_attached_file :header, styles: ->(f) { header_styles(f) }, convert_options: { all: '-strip' }, processors: [:lazy_thumbnail]
     validates_attachment_content_type :header, content_type: IMAGE_MIME_TYPES
-    validates_attachment_size :header, less_than: 2.megabytes
+    validates_attachment_size :header, less_than: LIMIT
+    remotable_attachment :header, LIMIT
   end
 
   def header_original_url
diff --git a/app/models/concerns/remotable.rb b/app/models/concerns/remotable.rb
index 0f18c5d96..3b8c507c3 100644
--- a/app/models/concerns/remotable.rb
+++ b/app/models/concerns/remotable.rb
@@ -3,8 +3,8 @@
 module Remotable
   extend ActiveSupport::Concern
 
-  included do
-    attachment_definitions.each_key do |attachment_name|
+  class_methods do
+    def remotable_attachment(attachment_name, limit)
       attribute_name  = "#{attachment_name}_remote_url".to_sym
       method_name     = "#{attribute_name}=".to_sym
       alt_method_name = "reset_#{attachment_name}!".to_sym
@@ -33,7 +33,7 @@ module Remotable
                         File.extname(filename)
                       end
 
-            send("#{attachment_name}=", StringIO.new(response.to_s))
+            send("#{attachment_name}=", StringIO.new(response.body_with_limit(limit)))
             send("#{attachment_name}_file_name=", basename + extname)
 
             self[attribute_name] = url if has_attribute?(attribute_name)
diff --git a/app/models/custom_emoji.rb b/app/models/custom_emoji.rb
index a77b53c98..476178e86 100644
--- a/app/models/custom_emoji.rb
+++ b/app/models/custom_emoji.rb
@@ -19,6 +19,8 @@
 #
 
 class CustomEmoji < ApplicationRecord
+  LIMIT = 50.kilobytes
+
   SHORTCODE_RE_FRAGMENT = '[a-zA-Z0-9_]{2,}'
 
   SCAN_RE = /(?<=[^[:alnum:]:]|\n|^)
@@ -29,14 +31,14 @@ class CustomEmoji < ApplicationRecord
 
   has_attached_file :image, styles: { static: { format: 'png', convert_options: '-coalesce -strip' } }
 
-  validates_attachment :image, content_type: { content_type: 'image/png' }, presence: true, size: { in: 0..50.kilobytes }
+  validates_attachment :image, content_type: { content_type: 'image/png' }, presence: true, size: { less_than: LIMIT }
   validates :shortcode, uniqueness: { scope: :domain }, format: { with: /\A#{SHORTCODE_RE_FRAGMENT}\z/ }, length: { minimum: 2 }
 
   scope :local,      -> { where(domain: nil) }
   scope :remote,     -> { where.not(domain: nil) }
   scope :alphabetic, -> { order(domain: :asc, shortcode: :asc) }
 
-  include Remotable
+  remotable_attachment :image, LIMIT
 
   def local?
     domain.nil?
diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb
index a4d9cd9d1..ac2aa7ed2 100644
--- a/app/models/media_attachment.rb
+++ b/app/models/media_attachment.rb
@@ -56,6 +56,8 @@ class MediaAttachment < ApplicationRecord
     },
   }.freeze
 
+  LIMIT = 8.megabytes
+
   belongs_to :account, inverse_of: :media_attachments, optional: true
   belongs_to :status,  inverse_of: :media_attachments, optional: true
 
@@ -64,10 +66,9 @@ class MediaAttachment < ApplicationRecord
                     processors: ->(f) { file_processors f },
                     convert_options: { all: '-quality 90 -strip' }
 
-  include Remotable
-
   validates_attachment_content_type :file, content_type: IMAGE_MIME_TYPES + VIDEO_MIME_TYPES
-  validates_attachment_size :file, less_than: 8.megabytes
+  validates_attachment_size :file, less_than: LIMIT
+  remotable_attachment :file, LIMIT
 
   validates :account, presence: true
   validates :description, length: { maximum: 420 }, if: :local?
diff --git a/app/models/preview_card.rb b/app/models/preview_card.rb
index 86eecdfe5..0c82f06ce 100644
--- a/app/models/preview_card.rb
+++ b/app/models/preview_card.rb
@@ -26,6 +26,7 @@
 
 class PreviewCard < ApplicationRecord
   IMAGE_MIME_TYPES = ['image/jpeg', 'image/png', 'image/gif'].freeze
+  LIMIT = 1.megabytes
 
   self.inheritance_column = false
 
@@ -36,11 +37,11 @@ class PreviewCard < ApplicationRecord
   has_attached_file :image, styles: { original: { geometry: '400x400>', file_geometry_parser: FastGeometryParser } }, convert_options: { all: '-quality 80 -strip' }
 
   include Attachmentable
-  include Remotable
 
   validates :url, presence: true, uniqueness: true
   validates_attachment_content_type :image, content_type: IMAGE_MIME_TYPES
-  validates_attachment_size :image, less_than: 1.megabytes
+  validates_attachment_size :image, less_than: LIMIT
+  remotable_attachment :image, LIMIT
 
   before_save :extract_dimensions, if: :link?
 
diff --git a/app/services/fetch_atom_service.rb b/app/services/fetch_atom_service.rb
index 48ad5dcd3..62dea8298 100644
--- a/app/services/fetch_atom_service.rb
+++ b/app/services/fetch_atom_service.rb
@@ -38,13 +38,14 @@ class FetchAtomService < BaseService
     return nil if response.code != 200
 
     if response.mime_type == 'application/atom+xml'
-      [@url, { prefetched_body: response.to_s }, :ostatus]
+      [@url, { prefetched_body: response.body_with_limit }, :ostatus]
     elsif ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].include?(response.mime_type)
-      json = body_to_json(response.to_s)
+      body = response.body_with_limit
+      json = body_to_json(body)
       if supported_context?(json) && json['type'] == 'Person' && json['inbox'].present?
-        [json['id'], { prefetched_body: response.to_s, id: true }, :activitypub]
+        [json['id'], { prefetched_body: body, id: true }, :activitypub]
       elsif supported_context?(json) && json['type'] == 'Note'
-        [json['id'], { prefetched_body: response.to_s, id: true }, :activitypub]
+        [json['id'], { prefetched_body: body, id: true }, :activitypub]
       else
         @unsupported_activity = true
         nil
@@ -61,7 +62,7 @@ class FetchAtomService < BaseService
   end
 
   def process_html(response)
-    page = Nokogiri::HTML(response.to_s)
+    page = Nokogiri::HTML(response.body_with_limit)
 
     json_link = page.xpath('//link[@rel="alternate"]').find { |link| ['application/activity+json', 'application/ld+json; profile="https://www.w3.org/ns/activitystreams"'].include?(link['type']) }
     atom_link = page.xpath('//link[@rel="alternate"]').find { |link| link['type'] == 'application/atom+xml' }
diff --git a/app/services/fetch_link_card_service.rb b/app/services/fetch_link_card_service.rb
index 26deb5ecc..d5920a417 100644
--- a/app/services/fetch_link_card_service.rb
+++ b/app/services/fetch_link_card_service.rb
@@ -45,7 +45,7 @@ class FetchLinkCardService < BaseService
 
     Request.new(:get, @url).perform do |res|
       if res.code == 200 && res.mime_type == 'text/html'
-        @html = res.to_s
+        @html = res.body_with_limit
         @html_charset = res.charset
       else
         @html = nil
diff --git a/app/services/resolve_account_service.rb b/app/services/resolve_account_service.rb
index 034821dc0..744ea24f4 100644
--- a/app/services/resolve_account_service.rb
+++ b/app/services/resolve_account_service.rb
@@ -181,7 +181,7 @@ class ResolveAccountService < BaseService
 
     @atom_body = Request.new(:get, atom_url).perform do |response|
       raise Mastodon::UnexpectedResponseError, response unless response.code == 200
-      response.to_s
+      response.body_with_limit
     end
   end
 
diff --git a/app/workers/pubsubhubbub/confirmation_worker.rb b/app/workers/pubsubhubbub/confirmation_worker.rb
index cc2d1225b..c0e7b677e 100644
--- a/app/workers/pubsubhubbub/confirmation_worker.rb
+++ b/app/workers/pubsubhubbub/confirmation_worker.rb
@@ -57,7 +57,7 @@ class Pubsubhubbub::ConfirmationWorker
 
   def callback_get_with_params
     Request.new(:get, subscription.callback_url, params: callback_params).perform do |response|
-      @callback_response_body = response.body.to_s
+      @callback_response_body = response.body_with_limit
     end
   end