From 9d0cafd06b45bd8169727d8994a849a222c3515e Mon Sep 17 00:00:00 2001 From: Christian Schmidt Date: Tue, 6 Aug 2024 10:23:48 +0200 Subject: [PATCH] Check Content-Length in ResponseWithLimitAdapter (#31285) --- app/lib/request.rb | 8 +- lib/paperclip/response_with_limit_adapter.rb | 7 +- .../response_with_limit_adapter_spec.rb | 93 +++++++++++++++++++ spec/lib/request_spec.rb | 8 +- 4 files changed, 107 insertions(+), 9 deletions(-) create mode 100644 spec/lib/paperclip/response_with_limit_adapter_spec.rb diff --git a/app/lib/request.rb b/app/lib/request.rb index 8d4120868d..ab42e82300 100644 --- a/app/lib/request.rb +++ b/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? diff --git a/lib/paperclip/response_with_limit_adapter.rb b/lib/paperclip/response_with_limit_adapter.rb index ff7a938abb..6c01d9825d 100644 --- a/lib/paperclip/response_with_limit_adapter.rb +++ b/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 diff --git a/spec/lib/paperclip/response_with_limit_adapter_spec.rb b/spec/lib/paperclip/response_with_limit_adapter_spec.rb new file mode 100644 index 0000000000..baf8bf5bb7 --- /dev/null +++ b/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 diff --git a/spec/lib/request_spec.rb b/spec/lib/request_spec.rb index c7620cf9b6..c99f18838b 100644 --- a/spec/lib/request_spec.rb +++ b/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