Check Content-Length in ResponseWithLimitAdapter (#31285)

app/lib/request.rb

@@ -234,13 +234,17 @@ class Request
     end
 
     def body_with_limit(limit = 1.megabyte)
-      raise Mastodon::LengthValidationError if content_length.present? && content_length > limit
+      require_limit_not_exceeded!(limit)
 
       contents = truncated_body(limit)
-      raise Mastodon::LengthValidationError if contents.bytesize > limit
+      raise Mastodon::LengthValidationError, "Body size exceeds limit of #{limit}" if contents.bytesize > limit
 
       contents
     end
+
+    def require_limit_not_exceeded!(limit)
+      raise Mastodon::LengthValidationError, "Content-Length #{content_length} exceeds limit of #{limit}" if content_length.present? && content_length > limit
+    end
   end
 
   if ::HTTP::Response.methods.include?(:body_with_limit) && !Rails.env.production?

lib/paperclip/response_with_limit_adapter.rb

@@ -16,6 +16,8 @@ module Paperclip
     private
 
     def cache_current_values
+      @target.response.require_limit_not_exceeded!(@target.limit)
+
       @original_filename = truncated_filename
       @tempfile = copy_to_tempfile(@target)
       @content_type = ContentTypeDetector.new(@tempfile.path).detect
@@ -27,16 +29,15 @@ module Paperclip
 
       source.response.body.each do |chunk|
         bytes_read += chunk.bytesize
+        raise Mastodon::LengthValidationError, "Body size exceeds limit of #{source.limit}" if bytes_read > source.limit
 
         destination.write(chunk)
         chunk.clear
-
-        raise Mastodon::LengthValidationError if bytes_read > source.limit
       end
 
       destination.rewind
       destination
-    rescue Mastodon::LengthValidationError
+    rescue
       destination.close(true)
       raise
     ensure

spec/lib/paperclip/response_with_limit_adapter_spec.rb

@@ -0,0 +1,93 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Paperclip::ResponseWithLimitAdapter do
+  subject { described_class.new(response_with_limit) }
+
+  before { stub_request(:get, url).to_return(headers: headers, body: body) }
+
+  let(:response_with_limit) { ResponseWithLimit.new(response, 50.kilobytes) }
+  let(:response) { Request.new(:get, url).perform(&:itself) }
+  let(:url) { 'https://example.com/dir/foo.png' }
+  let(:headers) { nil }
+  let(:body) { attachment_fixture('600x400.jpeg').binmode.read }
+
+  it 'writes temporary file' do
+    expect(subject.tempfile.read).to eq body
+    expect(subject.size).to eq body.bytesize
+  end
+
+  context 'with Content-Disposition header' do
+    let(:headers) { { 'Content-Disposition' => 'attachment; filename="bar.png"' } }
+
+    it 'uses filename from header' do
+      expect(subject.original_filename).to eq 'bar.png'
+    end
+
+    it 'detects MIME type from content' do
+      expect(subject.content_type).to eq 'image/jpeg'
+    end
+  end
+
+  context 'without Content-Disposition header' do
+    it 'uses filename from path' do
+      expect(subject.original_filename).to eq 'foo.png'
+    end
+
+    it 'detects MIME type from content' do
+      expect(subject.content_type).to eq 'image/jpeg'
+    end
+  end
+
+  context 'without filename in path' do
+    let(:url) { 'https://example.com/' }
+
+    it 'falls back to "data"' do
+      expect(subject.original_filename).to eq 'data'
+    end
+
+    it 'detects MIME type from content' do
+      expect(subject.content_type).to eq 'image/jpeg'
+    end
+  end
+
+  context 'with very long filename' do
+    let(:url) { 'https://example.com/abcdefghijklmnopqrstuvwxyz.0123456789' }
+
+    it 'truncates the filename' do
+      expect(subject.original_filename).to eq 'abcdefghijklmnopqrst.0123'
+    end
+  end
+
+  context 'when response size exceeds limit' do
+    context 'with Content-Length header' do
+      let(:headers) { { 'Content-Length' => 5.megabytes } }
+
+      it 'raises without reading the body' do
+        allow(response).to receive(:body).and_call_original
+
+        expect { subject }.to raise_error(Mastodon::LengthValidationError, 'Content-Length 5242880 exceeds limit of 51200')
+
+        expect(response).to_not have_received(:body)
+      end
+    end
+
+    context 'without Content-Length header' do
+      let(:body) { SecureRandom.random_bytes(1.megabyte) }
+
+      it 'raises while reading the body' do
+        expect { subject }.to raise_error(Mastodon::LengthValidationError, 'Body size exceeds limit of 51200')
+        expect(response.content_length).to be_nil
+      end
+    end
+  end
+
+  context 'when response times out' do
+    it 'raises' do
+      allow(response.body.connection).to receive(:readpartial).and_raise(HTTP::TimeoutError)
+
+      expect { subject }.to raise_error(HTTP::TimeoutError)
+    end
+  end
+end

spec/lib/request_spec.rb

@@ -100,7 +100,7 @@ describe Request do
   describe "response's body_with_limit method" do
     it 'rejects body more than 1 megabyte by default' do
       stub_request(:any, 'http://example.com').to_return(body: SecureRandom.random_bytes(2.megabytes))
-      expect { subject.perform(&:body_with_limit) }.to raise_error Mastodon::LengthValidationError
+      expect { subject.perform(&:body_with_limit) }.to raise_error(Mastodon::LengthValidationError, 'Body size exceeds limit of 1048576')
     end
 
     it 'accepts body less than 1 megabyte by default' do
@@ -110,17 +110,17 @@ describe Request do
 
     it 'rejects body by given size' do
       stub_request(:any, 'http://example.com').to_return(body: SecureRandom.random_bytes(2.kilobytes))
-      expect { subject.perform { |response| response.body_with_limit(1.kilobyte) } }.to raise_error Mastodon::LengthValidationError
+      expect { subject.perform { |response| response.body_with_limit(1.kilobyte) } }.to raise_error(Mastodon::LengthValidationError, 'Body size exceeds limit of 1024')
     end
 
     it 'rejects too large chunked body' do
       stub_request(:any, 'http://example.com').to_return(body: SecureRandom.random_bytes(2.megabytes), headers: { 'Transfer-Encoding' => 'chunked' })
-      expect { subject.perform(&:body_with_limit) }.to raise_error Mastodon::LengthValidationError
+      expect { subject.perform(&:body_with_limit) }.to raise_error(Mastodon::LengthValidationError, 'Body size exceeds limit of 1048576')
     end
 
     it 'rejects too large monolithic body' do
       stub_request(:any, 'http://example.com').to_return(body: SecureRandom.random_bytes(2.megabytes), headers: { 'Content-Length' => 2.megabytes })
-      expect { subject.perform(&:body_with_limit) }.to raise_error Mastodon::LengthValidationError
+      expect { subject.perform(&:body_with_limit) }.to raise_error(Mastodon::LengthValidationError, 'Content-Length 2097152 exceeds limit of 1048576')
     end
 
     it 'truncates large monolithic body' do