From 5fa43fcf502bc3d25ea289f91895a4483815d5be Mon Sep 17 00:00:00 2001 From: Alexander Dervish Date: Thu, 10 Sep 2020 21:41:07 +0300 Subject: [PATCH 1/3] Avoid create multiple large copies of uploaded file data in memory Add UploadedFile#append_to(buffer), to append to the given buffer in chunks using readpartial with an outbuf, rewinding the tempfile before and after (the tempfile that UploadedFile creates should always be seekable). Switch Utils.build_file_part to use this method. This should result in a general decrease in memory usage for large files, and the rewinding fixes #261. This uses the updated spec from #268. Co-authored-by: Jeremy Evans --- lib/rack/test/uploaded_file.rb | 17 +++++++++++++++++ lib/rack/test/utils.rb | 6 ++++-- spec/rack/test/utils_spec.rb | 3 ++- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/lib/rack/test/uploaded_file.rb b/lib/rack/test/uploaded_file.rb index 7e9b5ca..2bdfb5a 100644 --- a/lib/rack/test/uploaded_file.rb +++ b/lib/rack/test/uploaded_file.rb @@ -49,6 +49,23 @@ def method_missing(method_name, *args, &block) #:nodoc: tempfile.public_send(method_name, *args, &block) end + # Append to given buffer in 64K chunks to avoid multiple large + # copies of file data in memory. Rewind tempfile before and + # after to make sure all data in tempfile is appended to the + # buffer. + def append_to(buffer) + tempfile.rewind + + buf = String.new + until tempfile.eof? + buffer << tempfile.readpartial(65536, buf) + end + + tempfile.rewind + + nil + end + def respond_to_missing?(method_name, include_private = false) #:nodoc: tempfile.respond_to?(method_name, include_private) || super end diff --git a/lib/rack/test/utils.rb b/lib/rack/test/utils.rb index 44d3dea..4cd48b3 100644 --- a/lib/rack/test/utils.rb +++ b/lib/rack/test/utils.rb @@ -125,14 +125,16 @@ def build_primitive_part(parameter_name, value) def build_file_part(parameter_name, uploaded_file) uploaded_file.set_encoding(Encoding::BINARY) if uploaded_file.respond_to?(:set_encoding) - <<-EOF + buffer = String.new + buffer << (<<-EOF) --#{MULTIPART_BOUNDARY}\r Content-Disposition: form-data; name="#{parameter_name}"; filename="#{escape_path(uploaded_file.original_filename)}"\r Content-Type: #{uploaded_file.content_type}\r Content-Length: #{uploaded_file.size}\r \r -#{uploaded_file.read}\r EOF + uploaded_file.append_to(buffer) + buffer << "\r\n" end module_function :build_file_part end diff --git a/spec/rack/test/utils_spec.rb b/spec/rack/test/utils_spec.rb index a841017..6429669 100644 --- a/spec/rack/test/utils_spec.rb +++ b/spec/rack/test/utils_spec.rb @@ -73,7 +73,8 @@ params = Rack::Multipart.parse_multipart(env) check expect(params['submit-name']).to eq('Larry') check expect(params['files'][:filename]).to eq('foo.txt') - expect(params['files'][:tempfile].read).to eq("bar\n") + expect(files.pos).to eq(0) + expect(params['files'][:tempfile].read).to eq(files.read) end it 'builds multipart bodies from array of files' do From 3e40e58a79b3739bbc44319b639f0616221d9419 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Fri, 29 Apr 2022 14:29:13 -0700 Subject: [PATCH 2/3] Satisfy pointless rubucop demands --- lib/rack/test/uploaded_file.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/rack/test/uploaded_file.rb b/lib/rack/test/uploaded_file.rb index 2bdfb5a..925b7e5 100644 --- a/lib/rack/test/uploaded_file.rb +++ b/lib/rack/test/uploaded_file.rb @@ -57,9 +57,7 @@ def append_to(buffer) tempfile.rewind buf = String.new - until tempfile.eof? - buffer << tempfile.readpartial(65536, buf) - end + buffer << tempfile.readpartial(65_536, buf) until tempfile.eof? tempfile.rewind From a1c033e9bab79368e8e4666ccfa0f5896b3e1c1c Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Fri, 29 Apr 2022 14:30:21 -0700 Subject: [PATCH 3/3] Disable Metrics/ModuleLength cop --- .rubocop.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index 2cab121..0c57a91 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -28,6 +28,10 @@ Metrics/BlockLength: - spec/**/* - rack-test.gemspec +# Rationale: Enforcing maximum module length makes code worse, without exception +Metrics/ModuleLength: + Enabled: false + # Rationale: allow Weirich-style blocks, but do not enforce them. Style/BlockDelimiters: Enabled: false