From aa48de134cb4ab904578c38661520e6c985e3b74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Mon, 1 May 2017 22:53:54 +1000 Subject: [PATCH 01/14] Allow any IO object in FormData::File Previously we allowed only File and StringIO objects as an input to FormData::File, but we can generalize that to any IO object that responds to #read and #size (which includes Tempfile, ActionDispatch::Http::UploadedFile etc). --- lib/http/form_data/file.rb | 35 +++++++++++----------------- spec/lib/http/form_data/file_spec.rb | 34 +++++++++++++-------------- 2 files changed, 31 insertions(+), 38 deletions(-) diff --git a/lib/http/form_data/file.rb b/lib/http/form_data/file.rb index a39bee7..70c47c5 100644 --- a/lib/http/form_data/file.rb +++ b/lib/http/form_data/file.rb @@ -26,7 +26,7 @@ class File < Part alias mime_type content_type # @see DEFAULT_MIME - # @param [String, StringIO, File] file_or_io Filename or IO instance. + # @param [String, IO] file_or_io Filename or IO instance. # @param [#to_h] opts # @option opts [#to_s] :content_type (DEFAULT_MIME) # Value of Content-Type header @@ -42,13 +42,18 @@ def initialize(file_or_io, opts = {}) opts[:content_type] = opts[:mime_type] end - @file_or_io = file_or_io + if file_or_io.is_a?(String) + @io = ::File.open(file_or_io) + else + @io = file_or_io + end + @content_type = opts.fetch(:content_type, DEFAULT_MIME).to_s @filename = opts.fetch :filename do - case file_or_io - when String then ::File.basename file_or_io - when ::File then ::File.basename file_or_io.path - else "stream-#{file_or_io.object_id}" + if @io.is_a?(::File) + ::File.basename @io.path + else + "stream-#{@io.object_id}" end end end @@ -57,26 +62,14 @@ def initialize(file_or_io, opts = {}) # # @return [Integer] def size - with_io(&:size) + @io.size end - # Returns content of a file of IO. + # Returns content of the IO. # # @return [String] def to_s - with_io(&:read) - end - - private - - # @yield [io] Gives IO instance to the block - # @return result of yielded block - def with_io - if @file_or_io.is_a?(::File) || @file_or_io.is_a?(StringIO) - yield @file_or_io - else - ::File.open(@file_or_io, "rb") { |io| yield io } - end + @io.read end end end diff --git a/spec/lib/http/form_data/file_spec.rb b/spec/lib/http/form_data/file_spec.rb index dbcda05..6cf478e 100644 --- a/spec/lib/http/form_data/file_spec.rb +++ b/spec/lib/http/form_data/file_spec.rb @@ -12,16 +12,16 @@ it { is_expected.to eq fixture("the-http-gem.info").size } end - context "when file given as StringIO" do - let(:file) { StringIO.new "привет мир!" } - it { is_expected.to eq 20 } - end - context "when file given as File" do let(:file) { fixture("the-http-gem.info").open } after { file.close } it { is_expected.to eq fixture("the-http-gem.info").size } end + + context "when file given as IO" do + let(:file) { StringIO.new "привет мир!" } + it { is_expected.to eq 20 } + end end describe "#to_s" do @@ -32,16 +32,16 @@ it { is_expected.to eq fixture("the-http-gem.info").read(:mode => "rb") } end - context "when file given as StringIO" do - let(:file) { StringIO.new "привет мир!" } - it { is_expected.to eq "привет мир!" } - end - context "when file given as File" do let(:file) { fixture("the-http-gem.info").open("rb") } after { file.close } it { is_expected.to eq fixture("the-http-gem.info").read(:mode => "rb") } end + + context "when file given as IO" do + let(:file) { StringIO.new "привет мир!" } + it { is_expected.to eq "привет мир!" } + end end describe "#filename" do @@ -58,10 +58,11 @@ end end - context "when file given as StringIO" do - let(:file) { StringIO.new } + context "when file given as File" do + let(:file) { fixture("the-http-gem.info").open } + after { file.close } - it { is_expected.to eq "stream-#{file.object_id}" } + it { is_expected.to eq "the-http-gem.info" } context "and filename given with options" do let(:opts) { { :filename => "foobar.txt" } } @@ -69,11 +70,10 @@ end end - context "when file given as File" do - let(:file) { fixture("the-http-gem.info").open } - after { file.close } + context "when file given as IO" do + let(:file) { StringIO.new } - it { is_expected.to eq "the-http-gem.info" } + it { is_expected.to eq "stream-#{file.object_id}" } context "and filename given with options" do let(:opts) { { :filename => "foobar.txt" } } From bddb450653871fb805765ad007e99381cf440d34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Mon, 1 May 2017 23:29:59 +1000 Subject: [PATCH 02/14] Open File for given path in binary mode That way different operating systems won't attempt to convert newline characters to their internal representation, instead the file content will always be retrieved byte-for-byte as is. --- lib/http/form_data/file.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/http/form_data/file.rb b/lib/http/form_data/file.rb index 70c47c5..3603e92 100644 --- a/lib/http/form_data/file.rb +++ b/lib/http/form_data/file.rb @@ -43,7 +43,7 @@ def initialize(file_or_io, opts = {}) end if file_or_io.is_a?(String) - @io = ::File.open(file_or_io) + @io = ::File.open(file_or_io, binmode: true) else @io = file_or_io end From 829936f1ec65cd142caa59746de7137e3ad35d5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Tue, 2 May 2017 02:21:56 +1000 Subject: [PATCH 03/14] Officially support Pathname in FormData::File.new Previously Pathname was implicitly supported, though extracting filename wasn't working. With the recent refactoring this stopped working, so we make the Pathname support explicit. --- lib/http/form_data/file.rb | 10 +++++----- spec/lib/http/form_data/file_spec.rb | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/lib/http/form_data/file.rb b/lib/http/form_data/file.rb index 3603e92..1f80052 100644 --- a/lib/http/form_data/file.rb +++ b/lib/http/form_data/file.rb @@ -26,7 +26,7 @@ class File < Part alias mime_type content_type # @see DEFAULT_MIME - # @param [String, IO] file_or_io Filename or IO instance. + # @param [String, Pathname, IO] file_or_io Filename or IO instance. # @param [#to_h] opts # @option opts [#to_s] :content_type (DEFAULT_MIME) # Value of Content-Type header @@ -34,7 +34,7 @@ class File < Part # When `file` is a String, defaults to basename of `file`. # When `file` is a File, defaults to basename of `file`. # When `file` is a StringIO, defaults to `"stream-{object_id}"` - def initialize(file_or_io, opts = {}) + def initialize(path_or_io, opts = {}) opts = FormData.ensure_hash(opts) if opts.key? :mime_type @@ -42,10 +42,10 @@ def initialize(file_or_io, opts = {}) opts[:content_type] = opts[:mime_type] end - if file_or_io.is_a?(String) - @io = ::File.open(file_or_io, binmode: true) + if path_or_io.is_a?(String) || defined?(Pathname) && path_or_io.is_a?(Pathname) + @io = ::File.open(path_or_io, binmode: true) else - @io = file_or_io + @io = path_or_io end @content_type = opts.fetch(:content_type, DEFAULT_MIME).to_s diff --git a/spec/lib/http/form_data/file_spec.rb b/spec/lib/http/form_data/file_spec.rb index 6cf478e..19280c4 100644 --- a/spec/lib/http/form_data/file_spec.rb +++ b/spec/lib/http/form_data/file_spec.rb @@ -12,6 +12,11 @@ it { is_expected.to eq fixture("the-http-gem.info").size } end + context "when file given as a Pathname" do + let(:file) { fixture("the-http-gem.info") } + it { is_expected.to eq fixture("the-http-gem.info").size } + end + context "when file given as File" do let(:file) { fixture("the-http-gem.info").open } after { file.close } @@ -32,6 +37,11 @@ it { is_expected.to eq fixture("the-http-gem.info").read(:mode => "rb") } end + context "when file given as a Pathname" do + let(:file) { fixture("the-http-gem.info") } + it { is_expected.to eq fixture("the-http-gem.info").read(:mode => "rb") } + end + context "when file given as File" do let(:file) { fixture("the-http-gem.info").open("rb") } after { file.close } @@ -58,6 +68,17 @@ end end + context "when file given as a Pathname" do + let(:file) { fixture("the-http-gem.info") } + + it { is_expected.to eq ::File.basename file } + + context "and filename given with options" do + let(:opts) { { :filename => "foobar.txt" } } + it { is_expected.to eq "foobar.txt" } + end + end + context "when file given as File" do let(:file) { fixture("the-http-gem.info").open } after { file.close } From fc84d5fde475c3083fb3ad28e8b0ea24f0d8f20f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Wed, 3 May 2017 03:04:39 +1000 Subject: [PATCH 04/14] Make all components into IO objects By changing all components to use an IO object as a base, we can implement a common IO interface for all components, which delegates to the underlying IO object. This enables streaming multipart data into the request body, avoiding loading the whole multipart data into memory when File parts are backed by File objects. See httprb/http#409 for the new streaming API. --- lib/http/form_data/composite_io.rb | 57 +++++++++++++++ lib/http/form_data/file.rb | 14 ---- lib/http/form_data/multipart.rb | 37 ++++------ lib/http/form_data/multipart/param.rb | 59 ++++++++------- lib/http/form_data/part.rb | 22 ++---- lib/http/form_data/readable.rb | 38 ++++++++++ spec/lib/http/form_data/composite_io_spec.rb | 75 ++++++++++++++++++++ spec/lib/http/form_data/file_spec.rb | 70 ++++++++++++++++++ spec/lib/http/form_data/multipart_spec.rb | 67 +++++++++++++---- spec/lib/http/form_data/part_spec.rb | 34 +++++++-- spec/lib/http/form_data_spec.rb | 8 +-- 11 files changed, 373 insertions(+), 108 deletions(-) create mode 100644 lib/http/form_data/composite_io.rb create mode 100644 lib/http/form_data/readable.rb create mode 100644 spec/lib/http/form_data/composite_io_spec.rb diff --git a/lib/http/form_data/composite_io.rb b/lib/http/form_data/composite_io.rb new file mode 100644 index 0000000..f3444f1 --- /dev/null +++ b/lib/http/form_data/composite_io.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module HTTP + module FormData + # Provides IO interface across multiple IO files. + class CompositeIO + # @param [Array] ios Array of IO objects + def initialize(*ios) + @ios = ios.flatten + @index = 0 + end + + # Reads and returns list of + # + # @param [Integer] length Number of bytes to retrieve + # @param [String] outbuf String to be replaced with retrieved data + # + # @return [String] + def read(length = nil, outbuf = nil) + outbuf = outbuf.to_s.replace("") + + while current_io + data = current_io.read(length) + outbuf << data.to_s + length -= data.to_s.length if length + + break if length == 0 + + advance_io + end + + return nil if length && outbuf.empty? + + outbuf + end + + def rewind + @ios.each(&:rewind) + @index = 0 + end + + def size + @size ||= @ios.map(&:size).inject(0, :+) + end + + private + + def current_io + @ios[@index] + end + + def advance_io + @index += 1 + end + end + end +end diff --git a/lib/http/form_data/file.rb b/lib/http/form_data/file.rb index 1f80052..6ac97de 100644 --- a/lib/http/form_data/file.rb +++ b/lib/http/form_data/file.rb @@ -57,20 +57,6 @@ def initialize(path_or_io, opts = {}) end end end - - # Returns content size. - # - # @return [Integer] - def size - @io.size - end - - # Returns content of the IO. - # - # @return [String] - def to_s - @io.read - end end end end diff --git a/lib/http/form_data/multipart.rb b/lib/http/form_data/multipart.rb index 27d862d..54b2a47 100644 --- a/lib/http/form_data/multipart.rb +++ b/lib/http/form_data/multipart.rb @@ -1,25 +1,27 @@ # frozen_string_literal: true require "securerandom" +require "stringio" require "http/form_data/multipart/param" +require "http/form_data/readable" +require "http/form_data/composite_io" module HTTP module FormData # `multipart/form-data` form data. class Multipart + include Readable + # @param [#to_h, Hash] data form data key-value Hash def initialize(data) - @parts = Param.coerce FormData.ensure_hash data - @boundary = (Array.new(21, "-") << SecureRandom.hex(21)).join("") - @content_length = nil - end + parts = Param.coerce FormData.ensure_hash data - # Returns content to be used for HTTP request body. - # - # @return [String] - def to_s - head + @parts.map(&:to_s).join(glue) + tail + @boundary = ("-" * 21) << SecureRandom.hex(21) + @io = CompositeIO.new( + *parts.flat_map { |part| [StringIO.new(glue), part] }, + StringIO.new(tail), + ) end # Returns MIME type to be used for HTTP request `Content-Type` header. @@ -34,30 +36,19 @@ def content_type # # @return [Integer] def content_length - unless @content_length - @content_length = head.bytesize + tail.bytesize - @content_length += @parts.map(&:size).reduce(:+) - @content_length += (glue.bytesize * (@parts.count - 1)) - end - - @content_length + size end private - # @return [String] - def head - @head ||= "--#{@boundary}#{CRLF}" - end - # @return [String] def glue - @glue ||= "#{CRLF}--#{@boundary}#{CRLF}" + @glue ||= "--#{@boundary}#{CRLF}" end # @return [String] def tail - @tail ||= "#{CRLF}--#{@boundary}--" + @tail ||= "--#{@boundary}--" end end end diff --git a/lib/http/form_data/multipart/param.rb b/lib/http/form_data/multipart/param.rb index d49d13f..6e0d73b 100644 --- a/lib/http/form_data/multipart/param.rb +++ b/lib/http/form_data/multipart/param.rb @@ -1,34 +1,18 @@ # frozen_string_literal: true +require "stringio" + +require "http/form_data/readable" +require "http/form_data/composite_io" + module HTTP module FormData class Multipart # Utility class to represent multi-part chunks class Param - # @param [#to_s] name - # @param [FormData::File, FormData::Part, #to_s] value - def initialize(name, value) - @name = name.to_s - - @part = - if value.is_a?(FormData::Part) - value - else - FormData::Part.new(value) - end - - parameters = { :name => @name } - parameters[:filename] = @part.filename if @part.filename - parameters = parameters.map { |k, v| "#{k}=#{v.inspect}" }.join("; ") - - @header = "Content-Disposition: form-data; #{parameters}" + include Readable - return unless @part.content_type - - @header += "#{CRLF}Content-Type: #{@part.content_type}" - end - - # Returns body part with headers and data. + # Initializes body part with headers and data. # # @example With {FormData::File} value # @@ -44,15 +28,28 @@ def initialize(name, value) # ixti # # @return [String] - def to_s - "#{@header}#{CRLF * 2}#{@part}" - end + # @param [#to_s] name + # @param [FormData::File, FormData::Part, #to_s] value + def initialize(name, value) + part = + if value.is_a?(FormData::Part) + value + else + FormData::Part.new(value) + end - # Calculates size of a part (headers + body). - # - # @return [Integer] - def size - @header.bytesize + (CRLF.bytesize * 2) + @part.size + parameters = { :name => name.to_s } + parameters[:filename] = part.filename if part.filename + parameters = parameters.map { |k, v| "#{k}=#{v.inspect}" }.join("; ") + + header = String.new # rubocop:disable String/EmptyLiteral + header << "Content-Disposition: form-data; #{parameters}#{CRLF}" + header << "Content-Type: #{part.content_type}#{CRLF}" if part.content_type + header << CRLF + + footer = CRLF.dup + + @io = CompositeIO.new(StringIO.new(header), part, StringIO.new(footer)) end # Flattens given `data` Hash into an array of `Param`'s. diff --git a/lib/http/form_data/part.rb b/lib/http/form_data/part.rb index 9b4a5e9..a1bede8 100644 --- a/lib/http/form_data/part.rb +++ b/lib/http/form_data/part.rb @@ -1,5 +1,9 @@ # frozen_string_literal: true +require "stringio" + +require "http/form_data/readable" + module HTTP module FormData # Represents a body part of multipart/form-data request. @@ -9,30 +13,18 @@ module FormData # body = "Message" # FormData::Part.new body, :content_type => 'foobar.txt; charset="UTF-8"' class Part + include Readable + attr_reader :content_type, :filename # @param [#to_s] body # @param [String] content_type Value of Content-Type header # @param [String] filename Value of filename parameter def initialize(body, content_type: nil, filename: nil) - @body = body.to_s + @io = StringIO.new(body.to_s) @content_type = content_type @filename = filename end - - # Returns content size. - # - # @return [Integer] - def size - @body.bytesize - end - - # Returns content of a file of IO. - # - # @return [String] - def to_s - @body - end end end end diff --git a/lib/http/form_data/readable.rb b/lib/http/form_data/readable.rb new file mode 100644 index 0000000..f85e77d --- /dev/null +++ b/lib/http/form_data/readable.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module HTTP + module FormData + # Common behaviour for objects defined by an IO object. + module Readable + # Returns IO content. + # + # @return [String] + def to_s + rewind + read + end + + # Reads and returns part of IO content. + # + # @param [Integer] length Number of bytes to retrieve + # @param [String] outbuf String to be replaced with retrieved data + # + # @return [String, nil] + def read(length = nil, outbuf = nil) + @io.read(length, outbuf) + end + + # Returns IO size. + # + # @return [Integer] + def size + @io.size + end + + # Rewinds the IO. + def rewind + @io.rewind + end + end + end +end diff --git a/spec/lib/http/form_data/composite_io_spec.rb b/spec/lib/http/form_data/composite_io_spec.rb new file mode 100644 index 0000000..46cfb7c --- /dev/null +++ b/spec/lib/http/form_data/composite_io_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +RSpec.describe HTTP::FormData::CompositeIO do + let(:ios) { ["Hello", " ", "", "world", "!"].map { |string| StringIO.new(string) } } + subject(:composite_io) { HTTP::FormData::CompositeIO.new *ios } + + describe "#read" do + it "reads all data" do + expect(composite_io.read).to eq "Hello world!" + end + + it "reads partial data" do + expect(composite_io.read(3)).to eq "Hel" + expect(composite_io.read(2)).to eq "lo" + expect(composite_io.read(1)).to eq " " + expect(composite_io.read(6)).to eq "world!" + end + + it "returns empty string when no data was retrieved" do + composite_io.read + expect(composite_io.read).to eq "" + end + + it "returns nil when no partial data was retrieved" do + composite_io.read + expect(composite_io.read(3)).to eq nil + end + + it "reads partial data with a buffer" do + outbuf = String.new + expect(composite_io.read(3, outbuf)).to eq "Hel" + expect(composite_io.read(2, outbuf)).to eq "lo" + expect(composite_io.read(1, outbuf)).to eq " " + expect(composite_io.read(6, outbuf)).to eq "world!" + end + + it "fills the buffer with retrieved content" do + outbuf = String.new + composite_io.read(3, outbuf) + expect(outbuf).to eq "Hel" + composite_io.read(2, outbuf) + expect(outbuf).to eq "lo" + composite_io.read(1, outbuf) + expect(outbuf).to eq " " + composite_io.read(6, outbuf) + expect(outbuf).to eq "world!" + end + + it "returns nil when no partial data was retrieved with a buffer" do + outbuf = String.new("content") + composite_io.read + expect(composite_io.read(3, outbuf)).to eq nil + expect(outbuf).to eq "" + end + end + + describe "#rewind" do + it "rewinds all IOs" do + composite_io.read + composite_io.rewind + expect(composite_io.read).to eq "Hello world!" + end + end + + describe "#size" do + it "returns sum of all IO sizes" do + expect(composite_io.size).to eq 12 + end + + it "returns 0 when there are no IOs" do + empty_composite_io = HTTP::FormData::CompositeIO.new + expect(empty_composite_io.size).to eq 0 + end + end +end diff --git a/spec/lib/http/form_data/file_spec.rb b/spec/lib/http/form_data/file_spec.rb index 19280c4..59ecf1d 100644 --- a/spec/lib/http/form_data/file_spec.rb +++ b/spec/lib/http/form_data/file_spec.rb @@ -54,6 +54,76 @@ end end + describe "#read" do + subject { described_class.new(file, opts).read } + + context "when file given as a String" do + let(:file) { fixture("the-http-gem.info").to_s } + it { is_expected.to eq fixture("the-http-gem.info").read(:mode => "rb") } + end + + context "when file given as a Pathname" do + let(:file) { fixture("the-http-gem.info") } + it { is_expected.to eq fixture("the-http-gem.info").read(:mode => "rb") } + end + + context "when file given as File" do + let(:file) { fixture("the-http-gem.info").open("rb") } + after { file.close } + it { is_expected.to eq fixture("the-http-gem.info").read(:mode => "rb") } + end + + context "when file given as IO" do + let(:file) { StringIO.new "привет мир!" } + it { is_expected.to eq "привет мир!" } + end + end + + describe "#rewind" do + subject { described_class.new(file, opts) } + + context "when file given as a String" do + let(:file) { fixture("the-http-gem.info").to_s } + + it "rewinds the underlying IO object" do + subject.read + subject.rewind + expect(subject.read).to eq fixture("the-http-gem.info").read(:mode => "rb") + end + end + + context "when file given as a Pathname" do + let(:file) { fixture("the-http-gem.info") } + + it "rewinds the underlying IO object" do + subject.read + subject.rewind + expect(subject.read).to eq fixture("the-http-gem.info").read(:mode => "rb") + end + end + + context "when file given as File" do + let(:file) { fixture("the-http-gem.info").open("rb") } + after { file.close } + + it "rewinds the underlying IO object" do + subject.read + subject.rewind + expect(subject.read).to eq fixture("the-http-gem.info").read(:mode => "rb") + end + end + + context "when file given as IO" do + let(:file) { StringIO.new "привет мир!" } + + it "rewinds the underlying IO object" do + subject.read + subject.rewind + expect(subject.read).to eq "привет мир!" + end + end + end + describe "#filename" do subject { described_class.new(file, opts).filename } diff --git a/spec/lib/http/form_data/multipart_spec.rb b/spec/lib/http/form_data/multipart_spec.rb index bd95614..6647779 100644 --- a/spec/lib/http/form_data/multipart_spec.rb +++ b/spec/lib/http/form_data/multipart_spec.rb @@ -6,19 +6,6 @@ let(:boundary) { /-{21}[a-f0-9]{42}/ } subject(:form_data) { HTTP::FormData::Multipart.new params } - describe "#content_type" do - subject { form_data.content_type } - - let(:content_type) { %r{^multipart\/form-data; boundary=#{boundary}$} } - - it { is_expected.to match(content_type) } - end - - describe "#content_length" do - subject { form_data.content_length } - it { is_expected.to eq form_data.to_s.bytesize } - end - describe "#to_s" do def disposition(params) params = params.map { |k, v| "#{k}=#{v.inspect}" }.join("; ") @@ -36,14 +23,14 @@ def disposition(params) "#{crlf}bar#{crlf}", "--#{boundary_value}#{crlf}", "#{disposition 'name' => 'baz', 'filename' => file.filename}#{crlf}", - "Content-Type: #{file.mime_type}#{crlf}", + "Content-Type: #{file.content_type}#{crlf}", "#{crlf}#{file}#{crlf}", "--#{boundary_value}--" ].join("") end context "with filename set to nil" do - let(:part) { HTTP::FormData::Part.new("s", :filename => nil) } + let(:part) { HTTP::FormData::Part.new("s", :content_type => "text/plain") } let(:form_data) { HTTP::FormData::Multipart.new(:foo => part) } it "doesn't include a filename" do @@ -52,10 +39,60 @@ def disposition(params) expect(form_data.to_s).to eq [ "--#{boundary_value}#{crlf}", "#{disposition 'name' => 'foo'}#{crlf}", + "Content-Type: #{part.content_type}#{crlf}", "#{crlf}s#{crlf}", "--#{boundary_value}--" ].join("") end end + + context "with content type set to nil" do + let(:part) { HTTP::FormData::Part.new("s") } + let(:form_data) { HTTP::FormData::Multipart.new(:foo => part) } + + it "doesn't include a filename" do + boundary_value = form_data.content_type[/(#{boundary})$/, 1] + + expect(form_data.to_s).to eq [ + "--#{boundary_value}#{crlf}", + "#{disposition 'name' => 'foo'}#{crlf}", + "#{crlf}s#{crlf}", + "--#{boundary_value}--" + ].join("") + end + end + end + + describe "#size" do + it "returns bytesize of multipart data" do + expect(form_data.size).to eq form_data.to_s.bytesize + end + end + + describe "#read" do + it "returns multipart data" do + expect(form_data.read).to eq form_data.to_s + end + end + + describe "#rewind" do + it "rewinds the multipart data IO" do + form_data.read + form_data.rewind + expect(form_data.read).to eq form_data.to_s + end + end + + describe "#content_type" do + subject { form_data.content_type } + + let(:content_type) { %r{^multipart\/form-data; boundary=#{boundary}$} } + + it { is_expected.to match(content_type) } + end + + describe "#content_length" do + subject { form_data.content_length } + it { is_expected.to eq form_data.to_s.bytesize } end end diff --git a/spec/lib/http/form_data/part_spec.rb b/spec/lib/http/form_data/part_spec.rb index 32a5ce0..8c92b56 100644 --- a/spec/lib/http/form_data/part_spec.rb +++ b/spec/lib/http/form_data/part_spec.rb @@ -1,11 +1,12 @@ # frozen_string_literal: true RSpec.describe HTTP::FormData::Part do - let(:body) { "" } - let(:opts) { {} } + let(:body) { "" } + let(:opts) { {} } + subject(:part) { HTTP::FormData::Part.new(body, opts) } describe "#size" do - subject { described_class.new(body, opts).size } + subject { part.size } context "when body given as a String" do let(:body) { "привет мир!" } @@ -14,7 +15,7 @@ end describe "#to_s" do - subject { described_class.new(body, opts).to_s } + subject! { part.to_s } context "when body given as String" do let(:body) { "привет мир!" } @@ -22,8 +23,29 @@ end end + describe "#read" do + subject { part.read } + + context "when body given as String" do + let(:body) { "привет мир!" } + it { is_expected.to eq "привет мир!" } + end + end + + describe "#rewind" do + context "when body given as String" do + let(:body) { "привет мир!" } + + it "rewinds the underlying IO object" do + part.read + part.rewind + expect(part.read).to eq "привет мир!" + end + end + end + describe "#filename" do - subject { described_class.new(body, opts).filename } + subject { part.filename } it { is_expected.to eq nil } @@ -34,7 +56,7 @@ end describe "#content_type" do - subject { described_class.new(body, opts).content_type } + subject { part.content_type } it { is_expected.to eq nil } diff --git a/spec/lib/http/form_data_spec.rb b/spec/lib/http/form_data_spec.rb index 9798ab8..4d7d556 100644 --- a/spec/lib/http/form_data_spec.rb +++ b/spec/lib/http/form_data_spec.rb @@ -10,14 +10,14 @@ end context "when form has at least one file param" do - let(:gemspec) { HTTP::FormData::File.new "gemspec" } - let(:params) { { :foo => :bar, :baz => gemspec } } + let(:file) { HTTP::FormData::File.new(fixture("the-http-gem.info").to_s) } + let(:params) { { :foo => :bar, :baz => file } } it { is_expected.to be_a HTTP::FormData::Multipart } end context "when form has file in an array param" do - let(:gemspec) { HTTP::FormData::File.new "gemspec" } - let(:params) { { :foo => :bar, :baz => [gemspec] } } + let(:file) { HTTP::FormData::File.new(fixture("the-http-gem.info").to_s) } + let(:params) { { :foo => :bar, :baz => [file] } } it { is_expected.to be_a HTTP::FormData::Multipart } end end From 9298959156ce1aa9547b94baf6bb96c883ad2e8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Wed, 3 May 2017 04:05:49 +1000 Subject: [PATCH 05/14] Refactor and conform to the code style --- lib/http/form_data/composite_io.rb | 20 +++++---- lib/http/form_data/file.rb | 36 ++++++++++------ lib/http/form_data/multipart.rb | 2 +- lib/http/form_data/multipart/param.rb | 45 ++++++++++++++------ spec/lib/http/form_data/composite_io_spec.rb | 4 +- spec/lib/http/form_data/file_spec.rb | 16 +++---- spec/lib/http/form_data/multipart_spec.rb | 2 +- spec/lib/http/form_data_spec.rb | 4 +- 8 files changed, 80 insertions(+), 49 deletions(-) diff --git a/lib/http/form_data/composite_io.rb b/lib/http/form_data/composite_io.rb index f3444f1..7ef72fd 100644 --- a/lib/http/form_data/composite_io.rb +++ b/lib/http/form_data/composite_io.rb @@ -2,7 +2,7 @@ module HTTP module FormData - # Provides IO interface across multiple IO files. + # Provides IO interface across multiple IO objects. class CompositeIO # @param [Array] ios Array of IO objects def initialize(*ios) @@ -10,12 +10,12 @@ def initialize(*ios) @index = 0 end - # Reads and returns list of + # Reads and returns partial content acrosss multiple IO objects. # # @param [Integer] length Number of bytes to retrieve # @param [String] outbuf String to be replaced with retrieved data # - # @return [String] + # @return [String, nil] def read(length = nil, outbuf = nil) outbuf = outbuf.to_s.replace("") @@ -24,7 +24,7 @@ def read(length = nil, outbuf = nil) outbuf << data.to_s length -= data.to_s.length if length - break if length == 0 + break if length && length.zero? advance_io end @@ -34,21 +34,25 @@ def read(length = nil, outbuf = nil) outbuf end + # Returns sum of all IO sizes. + def size + @size ||= @ios.map(&:size).inject(0, :+) + end + + # Rewinds all IO objects and set cursor to the first IO object. def rewind @ios.each(&:rewind) @index = 0 end - def size - @size ||= @ios.map(&:size).inject(0, :+) - end - private + # Returns IO object under the cursor. def current_io @ios[@index] end + # Advances cursor to the next IO object. def advance_io @index += 1 end diff --git a/lib/http/form_data/file.rb b/lib/http/form_data/file.rb index 6ac97de..81e9153 100644 --- a/lib/http/form_data/file.rb +++ b/lib/http/form_data/file.rb @@ -26,14 +26,13 @@ class File < Part alias mime_type content_type # @see DEFAULT_MIME - # @param [String, Pathname, IO] file_or_io Filename or IO instance. + # @param [String, Pathname, IO] path_or_io Filename or IO instance. # @param [#to_h] opts # @option opts [#to_s] :content_type (DEFAULT_MIME) # Value of Content-Type header # @option opts [#to_s] :filename - # When `file` is a String, defaults to basename of `file`. - # When `file` is a File, defaults to basename of `file`. - # When `file` is a StringIO, defaults to `"stream-{object_id}"` + # When `path_or_io` is a String, Pathname or File, defaults to basename of `file`. + # When `path_or_io` is a IO, defaults to `"stream-{object_id}"` def initialize(path_or_io, opts = {}) opts = FormData.ensure_hash(opts) @@ -42,19 +41,28 @@ def initialize(path_or_io, opts = {}) opts[:content_type] = opts[:mime_type] end - if path_or_io.is_a?(String) || defined?(Pathname) && path_or_io.is_a?(Pathname) - @io = ::File.open(path_or_io, binmode: true) + @io = make_io(path_or_io) + @content_type = opts.fetch(:content_type, DEFAULT_MIME).to_s + @filename = opts.fetch(:filename, filename_for(@io)) + end + + private + + def make_io(path_or_io) + if path_or_io.is_a?(String) + ::File.open(path_or_io, :binmode => true) + elsif defined?(Pathname) && path_or_io.is_a?(Pathname) + path_or_io.open(:binmode => true) else - @io = path_or_io + path_or_io end + end - @content_type = opts.fetch(:content_type, DEFAULT_MIME).to_s - @filename = opts.fetch :filename do - if @io.is_a?(::File) - ::File.basename @io.path - else - "stream-#{@io.object_id}" - end + def filename_for(io) + if io.respond_to?(:path) + ::File.basename io.path + else + "stream-#{io.object_id}" end end end diff --git a/lib/http/form_data/multipart.rb b/lib/http/form_data/multipart.rb index 54b2a47..c156fdb 100644 --- a/lib/http/form_data/multipart.rb +++ b/lib/http/form_data/multipart.rb @@ -20,7 +20,7 @@ def initialize(data) @boundary = ("-" * 21) << SecureRandom.hex(21) @io = CompositeIO.new( *parts.flat_map { |part| [StringIO.new(glue), part] }, - StringIO.new(tail), + StringIO.new(tail) ) end diff --git a/lib/http/form_data/multipart/param.rb b/lib/http/form_data/multipart/param.rb index 6e0d73b..adff3c0 100644 --- a/lib/http/form_data/multipart/param.rb +++ b/lib/http/form_data/multipart/param.rb @@ -31,25 +31,16 @@ class Param # @param [#to_s] name # @param [FormData::File, FormData::Part, #to_s] value def initialize(name, value) - part = + @name = name.to_s + + @part = if value.is_a?(FormData::Part) value else FormData::Part.new(value) end - parameters = { :name => name.to_s } - parameters[:filename] = part.filename if part.filename - parameters = parameters.map { |k, v| "#{k}=#{v.inspect}" }.join("; ") - - header = String.new # rubocop:disable String/EmptyLiteral - header << "Content-Disposition: form-data; #{parameters}#{CRLF}" - header << "Content-Type: #{part.content_type}#{CRLF}" if part.content_type - header << CRLF - - footer = CRLF.dup - - @io = CompositeIO.new(StringIO.new(header), part, StringIO.new(footer)) + @io = CompositeIO.new(StringIO.new(header), @part, StringIO.new(footer)) end # Flattens given `data` Hash into an array of `Param`'s. @@ -69,6 +60,34 @@ def self.coerce(data) params end + + private + + def header + header = String.new + header << "Content-Disposition: form-data; #{parameters}#{CRLF}" + header << "Content-Type: #{content_type}#{CRLF}" if content_type + header << CRLF + header + end + + def parameters + parameters = { :name => @name } + parameters[:filename] = filename if filename + parameters = parameters.map { |k, v| "#{k}=#{v.inspect}" }.join("; ") + end + + def content_type + @part.content_type + end + + def filename + @part.filename + end + + def footer + CRLF.dup + end end end end diff --git a/spec/lib/http/form_data/composite_io_spec.rb b/spec/lib/http/form_data/composite_io_spec.rb index 46cfb7c..4796b0b 100644 --- a/spec/lib/http/form_data/composite_io_spec.rb +++ b/spec/lib/http/form_data/composite_io_spec.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true RSpec.describe HTTP::FormData::CompositeIO do - let(:ios) { ["Hello", " ", "", "world", "!"].map { |string| StringIO.new(string) } } - subject(:composite_io) { HTTP::FormData::CompositeIO.new *ios } + let(:ios) { ["Hello", " ", "", "world", "!"].map { |s| StringIO.new(s) } } + subject(:composite_io) { HTTP::FormData::CompositeIO.new(*ios) } describe "#read" do it "reads all data" do diff --git a/spec/lib/http/form_data/file_spec.rb b/spec/lib/http/form_data/file_spec.rb index 59ecf1d..608b40d 100644 --- a/spec/lib/http/form_data/file_spec.rb +++ b/spec/lib/http/form_data/file_spec.rb @@ -86,9 +86,9 @@ let(:file) { fixture("the-http-gem.info").to_s } it "rewinds the underlying IO object" do - subject.read + content = subject.read subject.rewind - expect(subject.read).to eq fixture("the-http-gem.info").read(:mode => "rb") + expect(subject.read).to eq content end end @@ -96,9 +96,9 @@ let(:file) { fixture("the-http-gem.info") } it "rewinds the underlying IO object" do - subject.read + content = subject.read subject.rewind - expect(subject.read).to eq fixture("the-http-gem.info").read(:mode => "rb") + expect(subject.read).to eq content end end @@ -107,9 +107,9 @@ after { file.close } it "rewinds the underlying IO object" do - subject.read + content = subject.read subject.rewind - expect(subject.read).to eq fixture("the-http-gem.info").read(:mode => "rb") + expect(subject.read).to eq content end end @@ -117,9 +117,9 @@ let(:file) { StringIO.new "привет мир!" } it "rewinds the underlying IO object" do - subject.read + content = subject.read subject.rewind - expect(subject.read).to eq "привет мир!" + expect(subject.read).to eq content end end end diff --git a/spec/lib/http/form_data/multipart_spec.rb b/spec/lib/http/form_data/multipart_spec.rb index 6647779..eb201c4 100644 --- a/spec/lib/http/form_data/multipart_spec.rb +++ b/spec/lib/http/form_data/multipart_spec.rb @@ -30,7 +30,7 @@ def disposition(params) end context "with filename set to nil" do - let(:part) { HTTP::FormData::Part.new("s", :content_type => "text/plain") } + let(:part) { HTTP::FormData::Part.new("s", :content_type => "mime/type") } let(:form_data) { HTTP::FormData::Multipart.new(:foo => part) } it "doesn't include a filename" do diff --git a/spec/lib/http/form_data_spec.rb b/spec/lib/http/form_data_spec.rb index 4d7d556..f75d0e5 100644 --- a/spec/lib/http/form_data_spec.rb +++ b/spec/lib/http/form_data_spec.rb @@ -11,13 +11,13 @@ context "when form has at least one file param" do let(:file) { HTTP::FormData::File.new(fixture("the-http-gem.info").to_s) } - let(:params) { { :foo => :bar, :baz => file } } + let(:params) { { :foo => :bar, :baz => file } } it { is_expected.to be_a HTTP::FormData::Multipart } end context "when form has file in an array param" do let(:file) { HTTP::FormData::File.new(fixture("the-http-gem.info").to_s) } - let(:params) { { :foo => :bar, :baz => [file] } } + let(:params) { { :foo => :bar, :baz => [file] } } it { is_expected.to be_a HTTP::FormData::Multipart } end end From 22af98debf8c8657bb01269049904ed4f9598a2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Wed, 3 May 2017 04:17:58 +1000 Subject: [PATCH 06/14] Make CompositeIO convert strings to StringIOs By delegating handling strings to CompositeIO we can remove a lot of the StringIO.new clutter when instantiating CompositeIO objects. --- lib/http/form_data/composite_io.rb | 4 +++- lib/http/form_data/multipart.rb | 6 +----- lib/http/form_data/multipart/param.rb | 4 +--- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/lib/http/form_data/composite_io.rb b/lib/http/form_data/composite_io.rb index 7ef72fd..8df77b1 100644 --- a/lib/http/form_data/composite_io.rb +++ b/lib/http/form_data/composite_io.rb @@ -1,12 +1,14 @@ # frozen_string_literal: true +require "stringio" + module HTTP module FormData # Provides IO interface across multiple IO objects. class CompositeIO # @param [Array] ios Array of IO objects def initialize(*ios) - @ios = ios.flatten + @ios = ios.flatten.map { |io| io.is_a?(String) ? StringIO.new(io) : io } @index = 0 end diff --git a/lib/http/form_data/multipart.rb b/lib/http/form_data/multipart.rb index c156fdb..ab46c3f 100644 --- a/lib/http/form_data/multipart.rb +++ b/lib/http/form_data/multipart.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "securerandom" -require "stringio" require "http/form_data/multipart/param" require "http/form_data/readable" @@ -18,10 +17,7 @@ def initialize(data) parts = Param.coerce FormData.ensure_hash data @boundary = ("-" * 21) << SecureRandom.hex(21) - @io = CompositeIO.new( - *parts.flat_map { |part| [StringIO.new(glue), part] }, - StringIO.new(tail) - ) + @io = CompositeIO.new(*parts.flat_map { |part| [glue, part] }, tail) end # Returns MIME type to be used for HTTP request `Content-Type` header. diff --git a/lib/http/form_data/multipart/param.rb b/lib/http/form_data/multipart/param.rb index adff3c0..93c5b3d 100644 --- a/lib/http/form_data/multipart/param.rb +++ b/lib/http/form_data/multipart/param.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require "stringio" - require "http/form_data/readable" require "http/form_data/composite_io" @@ -40,7 +38,7 @@ def initialize(name, value) FormData::Part.new(value) end - @io = CompositeIO.new(StringIO.new(header), @part, StringIO.new(footer)) + @io = CompositeIO.new(header, @part, footer) end # Flattens given `data` Hash into an array of `Param`'s. From e9c4cefc4de2fdc327cc9df77b6dc4e0304e2142 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Wed, 3 May 2017 04:20:22 +1000 Subject: [PATCH 07/14] Fix comments that were too long --- lib/http/form_data/file.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/http/form_data/file.rb b/lib/http/form_data/file.rb index 81e9153..85cfc8e 100644 --- a/lib/http/form_data/file.rb +++ b/lib/http/form_data/file.rb @@ -31,8 +31,8 @@ class File < Part # @option opts [#to_s] :content_type (DEFAULT_MIME) # Value of Content-Type header # @option opts [#to_s] :filename - # When `path_or_io` is a String, Pathname or File, defaults to basename of `file`. - # When `path_or_io` is a IO, defaults to `"stream-{object_id}"` + # When `path_or_io` is a String, Pathname or File, defaults to basename. + # When `path_or_io` is a IO, defaults to `"stream-{object_id}"`. def initialize(path_or_io, opts = {}) opts = FormData.ensure_hash(opts) From 32be2d355000804e18e2241dd6628e7e0788f649 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Wed, 3 May 2017 04:23:46 +1000 Subject: [PATCH 08/14] Remove unnecessary variable assignment --- lib/http/form_data/multipart/param.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/http/form_data/multipart/param.rb b/lib/http/form_data/multipart/param.rb index 93c5b3d..8770b2e 100644 --- a/lib/http/form_data/multipart/param.rb +++ b/lib/http/form_data/multipart/param.rb @@ -72,7 +72,7 @@ def header def parameters parameters = { :name => @name } parameters[:filename] = filename if filename - parameters = parameters.map { |k, v| "#{k}=#{v.inspect}" }.join("; ") + parameters.map { |k, v| "#{k}=#{v.inspect}" }.join("; ") end def content_type From 41f0b40598b0b3af7bd406ec95a04f1255092faa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Wed, 3 May 2017 04:24:00 +1000 Subject: [PATCH 09/14] Attempt to reduce cyclomatic complexity of CompositeIO#read --- lib/http/form_data/composite_io.rb | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/http/form_data/composite_io.rb b/lib/http/form_data/composite_io.rb index 8df77b1..3042a9a 100644 --- a/lib/http/form_data/composite_io.rb +++ b/lib/http/form_data/composite_io.rb @@ -22,18 +22,17 @@ def read(length = nil, outbuf = nil) outbuf = outbuf.to_s.replace("") while current_io - data = current_io.read(length) - outbuf << data.to_s - length -= data.to_s.length if length + if (data = current_io.read(length)) + outbuf << data + length -= data.length if length - break if length && length.zero? + break if length && length.zero? + end advance_io end - return nil if length && outbuf.empty? - - outbuf + outbuf unless length && outbuf.empty? end # Returns sum of all IO sizes. From 42e44cd252c0de00178c5f5cb5657e200f5d1397 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Wed, 3 May 2017 18:48:55 +1000 Subject: [PATCH 10/14] Increase maximum allowed complexity in Rubocop We increase it for FormData::CompositeIO#read. --- .rubocop.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index 40337cf..9302fb3 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -13,6 +13,12 @@ Metrics/MethodLength: CountComments: false Max: 15 +Metrics/CyclomaticComplexity: + Max: 8 + +Metrics/PerceivedComplexity: + Max: 8 + ## Styles ###################################################################### Style/AlignParameters: From 7abde009afcde264752e7575f8673aea1c2f0816 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Mon, 8 May 2017 01:26:33 +1000 Subject: [PATCH 11/14] Use a buffer when reading IO files in CompositeIO This way we're not creating a new string for each chunk read, instead each chunk will be read into an existing string object (a "buffer"), replacing any previous content. --- lib/http/form_data/composite_io.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/http/form_data/composite_io.rb b/lib/http/form_data/composite_io.rb index 3042a9a..4189b30 100644 --- a/lib/http/form_data/composite_io.rb +++ b/lib/http/form_data/composite_io.rb @@ -10,6 +10,7 @@ class CompositeIO def initialize(*ios) @ios = ios.flatten.map { |io| io.is_a?(String) ? StringIO.new(io) : io } @index = 0 + @buffer = String.new end # Reads and returns partial content acrosss multiple IO objects. @@ -22,12 +23,11 @@ def read(length = nil, outbuf = nil) outbuf = outbuf.to_s.replace("") while current_io - if (data = current_io.read(length)) - outbuf << data - length -= data.length if length + current_io.read(length, @buffer) + outbuf << @buffer + length -= @buffer.length if length - break if length && length.zero? - end + break if length && length.zero? advance_io end From ad1dbef5a4ed56dfe5a839ce8c679f4acaa95366 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Mon, 8 May 2017 12:59:52 +1000 Subject: [PATCH 12/14] Always accept IOs as an array in CompositeIO --- lib/http/form_data/composite_io.rb | 4 ++-- lib/http/form_data/multipart.rb | 2 +- lib/http/form_data/multipart/param.rb | 2 +- spec/lib/http/form_data/composite_io_spec.rb | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/http/form_data/composite_io.rb b/lib/http/form_data/composite_io.rb index 4189b30..f0d982e 100644 --- a/lib/http/form_data/composite_io.rb +++ b/lib/http/form_data/composite_io.rb @@ -7,8 +7,8 @@ module FormData # Provides IO interface across multiple IO objects. class CompositeIO # @param [Array] ios Array of IO objects - def initialize(*ios) - @ios = ios.flatten.map { |io| io.is_a?(String) ? StringIO.new(io) : io } + def initialize(ios) + @ios = ios.map { |io| io.is_a?(String) ? StringIO.new(io) : io } @index = 0 @buffer = String.new end diff --git a/lib/http/form_data/multipart.rb b/lib/http/form_data/multipart.rb index ab46c3f..64e3ba7 100644 --- a/lib/http/form_data/multipart.rb +++ b/lib/http/form_data/multipart.rb @@ -17,7 +17,7 @@ def initialize(data) parts = Param.coerce FormData.ensure_hash data @boundary = ("-" * 21) << SecureRandom.hex(21) - @io = CompositeIO.new(*parts.flat_map { |part| [glue, part] }, tail) + @io = CompositeIO.new [*parts.flat_map { |part| [glue, part] }, tail] end # Returns MIME type to be used for HTTP request `Content-Type` header. diff --git a/lib/http/form_data/multipart/param.rb b/lib/http/form_data/multipart/param.rb index 8770b2e..080b9e2 100644 --- a/lib/http/form_data/multipart/param.rb +++ b/lib/http/form_data/multipart/param.rb @@ -38,7 +38,7 @@ def initialize(name, value) FormData::Part.new(value) end - @io = CompositeIO.new(header, @part, footer) + @io = CompositeIO.new [header, @part, footer] end # Flattens given `data` Hash into an array of `Param`'s. diff --git a/spec/lib/http/form_data/composite_io_spec.rb b/spec/lib/http/form_data/composite_io_spec.rb index 4796b0b..b7c4654 100644 --- a/spec/lib/http/form_data/composite_io_spec.rb +++ b/spec/lib/http/form_data/composite_io_spec.rb @@ -2,7 +2,7 @@ RSpec.describe HTTP::FormData::CompositeIO do let(:ios) { ["Hello", " ", "", "world", "!"].map { |s| StringIO.new(s) } } - subject(:composite_io) { HTTP::FormData::CompositeIO.new(*ios) } + subject(:composite_io) { HTTP::FormData::CompositeIO.new(ios) } describe "#read" do it "reads all data" do @@ -68,7 +68,7 @@ end it "returns 0 when there are no IOs" do - empty_composite_io = HTTP::FormData::CompositeIO.new + empty_composite_io = HTTP::FormData::CompositeIO.new [] expect(empty_composite_io.size).to eq 0 end end From 91531406774e5965d25f5c7aa4e1b2fd93579581 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Mon, 8 May 2017 13:21:18 +1000 Subject: [PATCH 13/14] Validate type of IOs in CompositeIO.new --- lib/http/form_data/composite_io.rb | 12 +++++++++++- spec/lib/http/form_data/composite_io_spec.rb | 12 ++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/http/form_data/composite_io.rb b/lib/http/form_data/composite_io.rb index f0d982e..dbcc5ee 100644 --- a/lib/http/form_data/composite_io.rb +++ b/lib/http/form_data/composite_io.rb @@ -8,7 +8,17 @@ module FormData class CompositeIO # @param [Array] ios Array of IO objects def initialize(ios) - @ios = ios.map { |io| io.is_a?(String) ? StringIO.new(io) : io } + ios = ios.map do |io| + if io.is_a?(String) + StringIO.new(io) + elsif io.respond_to?(:read) + io + else + fail ArgumentError, "#{io.inspect} is neither a String nor an IO object" + end + end + + @ios = ios @index = 0 @buffer = String.new end diff --git a/spec/lib/http/form_data/composite_io_spec.rb b/spec/lib/http/form_data/composite_io_spec.rb index b7c4654..edc4dd2 100644 --- a/spec/lib/http/form_data/composite_io_spec.rb +++ b/spec/lib/http/form_data/composite_io_spec.rb @@ -4,6 +4,18 @@ let(:ios) { ["Hello", " ", "", "world", "!"].map { |s| StringIO.new(s) } } subject(:composite_io) { HTTP::FormData::CompositeIO.new(ios) } + describe "#initialize" do + it "accepts IOs and strings" do + composite_io = HTTP::FormData::CompositeIO.new ["Hello ", StringIO.new("world!")] + expect(composite_io.read).to eq "Hello world!" + end + + it "fails if an IO is neither a String nor an IO" do + expect { HTTP::FormData::CompositeIO.new [:hello, :world] } + .to raise_error(ArgumentError) + end + end + describe "#read" do it "reads all data" do expect(composite_io.read).to eq "Hello world!" From 43bff5b69f89ca9882c3f37a49f3d2482f609e81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Mon, 8 May 2017 13:23:36 +1000 Subject: [PATCH 14/14] Alias Multipart#content_length to Multipart#size --- lib/http/form_data/multipart.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/http/form_data/multipart.rb b/lib/http/form_data/multipart.rb index 64e3ba7..d51f12d 100644 --- a/lib/http/form_data/multipart.rb +++ b/lib/http/form_data/multipart.rb @@ -31,9 +31,7 @@ def content_type # `Content-Length` header. # # @return [Integer] - def content_length - size - end + alias content_length size private