From f05c3afe4f308a6af30a76acf1de14fd5eabbb35 Mon Sep 17 00:00:00 2001 From: Pantelis Vratsalis Date: Wed, 14 Nov 2018 09:09:03 +0200 Subject: [PATCH 1/2] Move HTTP::Multipart to MIME namespace --- .../{http => mime}/multipart/builder_spec.cr | 48 +++++++++---------- .../{http => mime}/multipart/parser_spec.cr | 26 +++++----- spec/std/{http => mime}/multipart_spec.cr | 12 ++--- src/http/formdata.cr | 6 +-- src/http/formdata/builder.cr | 2 +- src/http/formdata/parser.cr | 2 +- src/mime.cr | 1 + src/{http => mime}/multipart.cr | 14 +++--- src/{http => mime}/multipart/builder.cr | 8 ++-- src/{http => mime}/multipart/parser.cr | 6 +-- 10 files changed, 62 insertions(+), 63 deletions(-) rename spec/std/{http => mime}/multipart/builder_spec.cr (79%) rename spec/std/{http => mime}/multipart/parser_spec.cr (80%) rename spec/std/{http => mime}/multipart_spec.cr (77%) rename src/{http => mime}/multipart.cr (88%) rename src/{http => mime}/multipart/builder.cr (97%) rename src/{http => mime}/multipart/parser.cr (96%) diff --git a/spec/std/http/multipart/builder_spec.cr b/spec/std/mime/multipart/builder_spec.cr similarity index 79% rename from spec/std/http/multipart/builder_spec.cr rename to spec/std/mime/multipart/builder_spec.cr index d1c4c50ba038..b330ab3f2e74 100644 --- a/spec/std/http/multipart/builder_spec.cr +++ b/spec/std/mime/multipart/builder_spec.cr @@ -1,10 +1,10 @@ require "spec" -require "http" +require "mime" -describe HTTP::Multipart::Builder do +describe MIME::Multipart::Builder do it "generates valid multipart messages" do io = IO::Memory.new - builder = HTTP::Multipart::Builder.new(io, "fixed-boundary") + builder = MIME::Multipart::Builder.new(io, "fixed-boundary") headers = HTTP::Headers{"X-Foo" => "bar"} builder.body_part headers, "body part 1" @@ -30,7 +30,7 @@ describe HTTP::Multipart::Builder do it "generates valid multipart messages with preamble and epilogue" do io = IO::Memory.new - builder = HTTP::Multipart::Builder.new(io, "fixed-boundary") + builder = MIME::Multipart::Builder.new(io, "fixed-boundary") builder.preamble "Here is a preamble to explain why multipart/mixed " builder.preamble "exists and why your mail client should support it" @@ -64,7 +64,7 @@ describe HTTP::Multipart::Builder do describe "#content_type" do it "calculates the content type" do - builder = HTTP::Multipart::Builder.new(IO::Memory.new, "a delimiter string with a quote in \"") + builder = MIME::Multipart::Builder.new(IO::Memory.new, "a delimiter string with a quote in \"") builder.content_type("alternative").should eq(%q(multipart/alternative; boundary="a delimiter string with a quote in \"")) end end @@ -72,7 +72,7 @@ describe HTTP::Multipart::Builder do describe ".preamble" do it "accepts different data types" do io = IO::Memory.new - builder = HTTP::Multipart::Builder.new(io, "boundary") + builder = MIME::Multipart::Builder.new(io, "boundary") builder.preamble "string\r\n" builder.preamble "slice\r\n".to_slice @@ -103,10 +103,10 @@ describe HTTP::Multipart::Builder do end it "raises when called after starting the body" do - builder = HTTP::Multipart::Builder.new(IO::Memory.new) + builder = MIME::Multipart::Builder.new(IO::Memory.new) builder.body_part HTTP::Headers.new, "test" - expect_raises(HTTP::Multipart::Error, "Cannot generate preamble: body already started") do + expect_raises(MIME::Multipart::Error, "Cannot generate preamble: body already started") do builder.preamble "test" end end @@ -115,7 +115,7 @@ describe HTTP::Multipart::Builder do describe ".body_part" do it "accepts different data types" do io = IO::Memory.new - builder = HTTP::Multipart::Builder.new(io, "boundary") + builder = MIME::Multipart::Builder.new(io, "boundary") headers = HTTP::Headers{"X-Foo" => "Bar"} @@ -160,21 +160,21 @@ describe HTTP::Multipart::Builder do end it "raises when called after finishing" do - builder = HTTP::Multipart::Builder.new(IO::Memory.new) + builder = MIME::Multipart::Builder.new(IO::Memory.new) builder.body_part HTTP::Headers.new, "test" builder.finish - expect_raises(HTTP::Multipart::Error, "Cannot generate body part: already finished") do + expect_raises(MIME::Multipart::Error, "Cannot generate body part: already finished") do builder.body_part HTTP::Headers.new, "test" end end it "raises when called after epilogue" do - builder = HTTP::Multipart::Builder.new(IO::Memory.new) + builder = MIME::Multipart::Builder.new(IO::Memory.new) builder.body_part HTTP::Headers.new, "test" builder.epilogue "test" - expect_raises(HTTP::Multipart::Error, "Cannot generate body part: after epilogue") do + expect_raises(MIME::Multipart::Error, "Cannot generate body part: after epilogue") do builder.body_part HTTP::Headers.new, "test" end end @@ -183,7 +183,7 @@ describe HTTP::Multipart::Builder do describe ".epilogue" do it "accepts different data types" do io = IO::Memory.new - builder = HTTP::Multipart::Builder.new(io, "boundary") + builder = MIME::Multipart::Builder.new(io, "boundary") builder.body_part(HTTP::Headers.new) @@ -215,26 +215,26 @@ describe HTTP::Multipart::Builder do end it "raises when called after finishing" do - builder = HTTP::Multipart::Builder.new(IO::Memory.new) + builder = MIME::Multipart::Builder.new(IO::Memory.new) builder.body_part HTTP::Headers.new, "test" builder.finish - expect_raises(HTTP::Multipart::Error, "Cannot generate epilogue: already finished") do + expect_raises(MIME::Multipart::Error, "Cannot generate epilogue: already finished") do builder.epilogue "test" end end it "raises when called with no body parts" do - builder = HTTP::Multipart::Builder.new(IO::Memory.new) + builder = MIME::Multipart::Builder.new(IO::Memory.new) - expect_raises(HTTP::Multipart::Error, "Cannot generate epilogue: no body parts") do + expect_raises(MIME::Multipart::Error, "Cannot generate epilogue: no body parts") do builder.epilogue "test" end builder.preamble "test" - expect_raises(HTTP::Multipart::Error, "Cannot generate epilogue: no body parts") do + expect_raises(MIME::Multipart::Error, "Cannot generate epilogue: no body parts") do builder.epilogue "test" end end @@ -242,26 +242,26 @@ describe HTTP::Multipart::Builder do describe ".finish" do it "raises if no body exists" do - builder = HTTP::Multipart::Builder.new(IO::Memory.new) + builder = MIME::Multipart::Builder.new(IO::Memory.new) - expect_raises(HTTP::Multipart::Error, "Cannot finish multipart: no body parts") do + expect_raises(MIME::Multipart::Error, "Cannot finish multipart: no body parts") do builder.finish end builder.preamble "test" - expect_raises(HTTP::Multipart::Error, "Cannot finish multipart: no body parts") do + expect_raises(MIME::Multipart::Error, "Cannot finish multipart: no body parts") do builder.finish end end it "raises if already finished" do - builder = HTTP::Multipart::Builder.new(IO::Memory.new) + builder = MIME::Multipart::Builder.new(IO::Memory.new) builder.body_part HTTP::Headers.new, "test" builder.finish - expect_raises(HTTP::Multipart::Error, "Cannot finish multipart: already finished") do + expect_raises(MIME::Multipart::Error, "Cannot finish multipart: already finished") do builder.finish end end diff --git a/spec/std/http/multipart/parser_spec.cr b/spec/std/mime/multipart/parser_spec.cr similarity index 80% rename from spec/std/http/multipart/parser_spec.cr rename to spec/std/mime/multipart/parser_spec.cr index 1d21c4b2bf16..0cca2bbabf01 100644 --- a/spec/std/http/multipart/parser_spec.cr +++ b/spec/std/mime/multipart/parser_spec.cr @@ -1,9 +1,9 @@ require "spec" -require "http" +require "mime" private def parse(delim, data, *, gsub = true) data_io = IO::Memory.new(gsub ? data.gsub('\n', "\r\n") : data) - parser = HTTP::Multipart::Parser.new(data_io, delim) + parser = MIME::Multipart::Parser.new(data_io, delim) parsed = [] of {headers: HTTP::Headers, body: String} while parser.has_next? @@ -12,7 +12,7 @@ private def parse(delim, data, *, gsub = true) parsed end -describe HTTP::Multipart::Parser do +describe MIME::Multipart::Parser do it "parses basic multipart messages" do data = parse "AaB03x", <<-MULTIPART --AaB03x @@ -60,19 +60,19 @@ describe HTTP::Multipart::Parser do end it "handles invalid multipart data" do - expect_raises(HTTP::Multipart::Error, "EOF reading delimiter") do + expect_raises(MIME::Multipart::Error, "EOF reading delimiter") do parse "AaB03x", "--AaB03x", gsub: false end - expect_raises(HTTP::Multipart::Error, "EOF reading delimiter") do + expect_raises(MIME::Multipart::Error, "EOF reading delimiter") do parse "AaB03x", "--AaB03x\r\n\r\n--AaB03x", gsub: false end - expect_raises(HTTP::Multipart::Error, "EOF reading delimiter padding") do + expect_raises(MIME::Multipart::Error, "EOF reading delimiter padding") do parse "AaB03x", "--AaB03x ", gsub: false end - expect_raises(HTTP::Multipart::Error, "padding contained non-whitespace character") do + expect_raises(MIME::Multipart::Error, "padding contained non-whitespace character") do parse "AaB03x", "--AaB03x foo \r\n\r\n--AaB03x--", gsub: false end end @@ -89,26 +89,26 @@ describe HTTP::Multipart::Parser do Foo --AaB03x-- MULTIPART - parser = HTTP::Multipart::Parser.new(IO::Memory.new(input.gsub('\n', "\r\n")), "AaB03x") + parser = MIME::Multipart::Parser.new(IO::Memory.new(input.gsub('\n', "\r\n")), "AaB03x") parser.next { } parser.has_next?.should eq(false) - expect_raises(HTTP::Multipart::Error, "Multipart parser already finished parsing") do + expect_raises(MIME::Multipart::Error, "Multipart parser already finished parsing") do parser.next { } end end it "raises calling #next after errored" do - parser = HTTP::Multipart::Parser.new(IO::Memory.new("--AaB03x--"), "AaB03x") + parser = MIME::Multipart::Parser.new(IO::Memory.new("--AaB03x--"), "AaB03x") - expect_raises(HTTP::Multipart::Error, "no parts") do + expect_raises(MIME::Multipart::Error, "no parts") do parser.next { } end parser.has_next?.should eq(false) - expect_raises(HTTP::Multipart::Error, "Multipart parser is in an errored state") do + expect_raises(MIME::Multipart::Error, "Multipart parser is in an errored state") do parser.next { } end end @@ -126,7 +126,7 @@ describe HTTP::Multipart::Parser do --b-- MULTIPART - parser = HTTP::Multipart::Parser.new(IO::Memory.new(input.gsub('\n', "\r\n")), "b") + parser = MIME::Multipart::Parser.new(IO::Memory.new(input.gsub('\n', "\r\n")), "b") ios = [] of IO diff --git a/spec/std/http/multipart_spec.cr b/spec/std/mime/multipart_spec.cr similarity index 77% rename from spec/std/http/multipart_spec.cr rename to spec/std/mime/multipart_spec.cr index dcbe019109bd..bd36396dafc8 100644 --- a/spec/std/http/multipart_spec.cr +++ b/spec/std/mime/multipart_spec.cr @@ -1,11 +1,11 @@ require "spec" -require "http" +require "mime" -describe HTTP::Multipart do +describe MIME::Multipart do describe ".parse" do it "parses multipart messages" do multipart = "--aA40\r\nContent-Type: text/plain\r\n\r\nabcd\r\n--aA40--" - HTTP::Multipart.parse(IO::Memory.new(multipart), "aA40") do |headers, io| + MIME::Multipart.parse(IO::Memory.new(multipart), "aA40") do |headers, io| headers["Content-Type"].should eq("text/plain") io.gets_to_end.should eq("abcd") end @@ -15,12 +15,12 @@ describe HTTP::Multipart do describe ".parse_boundary" do it "parses unquoted boundaries" do content_type = "multipart/mixed; boundary=a_-47HDS" - HTTP::Multipart.parse_boundary(content_type).should eq("a_-47HDS") + MIME::Multipart.parse_boundary(content_type).should eq("a_-47HDS") end it "parses quoted boundaries" do content_type = %{multipart/mixed; boundary="aA_-<>()"} - HTTP::Multipart.parse_boundary(content_type).should eq(%{aA_-<>()}) + MIME::Multipart.parse_boundary(content_type).should eq(%{aA_-<>()}) end end @@ -30,7 +30,7 @@ describe HTTP::Multipart do body = "--aA40\r\nContent-Type: text/plain\r\n\r\nbody\r\n--aA40--" request = HTTP::Request.new("POST", "/", headers, body) - HTTP::Multipart.parse(request) do |headers, io| + MIME::Multipart.parse(request) do |headers, io| headers["Content-Type"].should eq("text/plain") io.gets_to_end.should eq("body") end diff --git a/src/http/formdata.cr b/src/http/formdata.cr index f7d63526f836..19cd2f606042 100644 --- a/src/http/formdata.cr +++ b/src/http/formdata.cr @@ -113,7 +113,7 @@ module HTTP::FormData body = request.body raise Error.new "Cannot extract form-data from HTTP request: body is empty" unless body - boundary = request.headers["Content-Type"]?.try { |header| Multipart.parse_boundary(header) } + boundary = request.headers["Content-Type"]?.try { |header| MIME::Multipart.parse_boundary(header) } raise Error.new "Cannot extract form-data from HTTP request: could not find boundary in Content-Type" unless boundary parse(body, boundary) { |part| yield part } @@ -178,7 +178,7 @@ module HTTP::FormData # ``` # # See: `FormData::Builder` - def self.build(io, boundary = Multipart.generate_boundary) + def self.build(io, boundary = MIME::Multipart.generate_boundary) builder = Builder.new(io, boundary) yield builder builder.finish @@ -202,7 +202,7 @@ module HTTP::FormData # ``` # # See: `FormData::Builder` - def self.build(response : HTTP::Server::Response, boundary = Multipart.generate_boundary) + def self.build(response : HTTP::Server::Response, boundary = MIME::Multipart.generate_boundary) builder = Builder.new(response, boundary) yield builder builder.finish diff --git a/src/http/formdata/builder.cr b/src/http/formdata/builder.cr index 57247b0067f9..5353842fdd9f 100644 --- a/src/http/formdata/builder.cr +++ b/src/http/formdata/builder.cr @@ -15,7 +15,7 @@ module HTTP::FormData class Builder # Creates a new `FormData::Builder` which writes to *io*, using the # multipart boundary *boundary*. - def initialize(@io : IO, @boundary = Multipart.generate_boundary) + def initialize(@io : IO, @boundary = MIME::Multipart.generate_boundary) @state = :START end diff --git a/src/http/formdata/parser.cr b/src/http/formdata/parser.cr index 24ab8bcc6a48..b4395e60a6bf 100644 --- a/src/http/formdata/parser.cr +++ b/src/http/formdata/parser.cr @@ -2,7 +2,7 @@ module HTTP::FormData class Parser # Creates a new parser which parses *io* with multipart boundary *boundary*. def initialize(io, boundary) - @multipart = Multipart::Parser.new(io, boundary) + @multipart = MIME::Multipart::Parser.new(io, boundary) end # Parses the next form-data part and yields field name, io, `FileMetadata`, diff --git a/src/mime.cr b/src/mime.cr index 40e73a25effe..7f362c5d94e0 100644 --- a/src/mime.cr +++ b/src/mime.cr @@ -1,4 +1,5 @@ require "crystal/system/mime" +require "./mime/**" # This module implements a global MIME registry. # diff --git a/src/http/multipart.cr b/src/mime/multipart.cr similarity index 88% rename from src/http/multipart.cr rename to src/mime/multipart.cr index 2a099ee02413..cffb52a53776 100644 --- a/src/http/multipart.cr +++ b/src/mime/multipart.cr @@ -2,11 +2,11 @@ require "random/secure" require "./multipart/*" require "mime/media_type" -# The `HTTP::Multipart` module contains utilities for parsing MIME multipart +# The `MIME::Multipart` module contains utilities for parsing MIME multipart # messages, which contain multiple body parts, each containing a header section # and binary body. The `multipart/form-data` content-type has a separate set of -# utilities in the `HTTP::FormData` module. -module HTTP::Multipart +# utilities in the `MIME::FormData` module. +module MIME::Multipart # Parses a MIME multipart message, yielding `HTTP::Headers` and an `IO` for # each body part. # @@ -15,7 +15,7 @@ module HTTP::Multipart # # ``` # multipart = "--aA40\r\nContent-Type: text/plain\r\n\r\nbody\r\n--aA40--" - # HTTP::Multipart.parse(IO::Memory.new(multipart), "aA40") do |headers, io| + # MIME::Multipart.parse(IO::Memory.new(multipart), "aA40") do |headers, io| # headers["Content-Type"] # => "text/plain" # io.gets_to_end # => "body" # end @@ -33,7 +33,7 @@ module HTTP::Multipart # `nil` is the boundary was not found. # # ``` - # HTTP::Multipart.parse_boundary("multipart/mixed; boundary=\"abcde\"") # => "abcde" + # MIME::Multipart.parse_boundary("multipart/mixed; boundary=\"abcde\"") # => "abcde" # ``` def self.parse_boundary(content_type) type = MIME::MediaType.parse?(content_type) @@ -58,7 +58,7 @@ module HTTP::Multipart # body = "--aA40\r\nContent-Type: text/plain\r\n\r\nbody\r\n--aA40--" # request = HTTP::Request.new("POST", "/", headers, body) # - # HTTP::Multipart.parse(request) do |headers, io| + # MIME::Multipart.parse(request) do |headers, io| # headers["Content-Type"] # => "text/plain" # io.gets_to_end # => "body" # end @@ -93,7 +93,7 @@ module HTTP::Multipart # Returns a unique string suitable for use as a multipart boundary. # # ``` - # HTTP::Multipart.generate_boundary # => "---------------------------dQu6bXHYb4m5zrRC3xPTGwV" + # MIME::Multipart.generate_boundary # => "---------------------------dQu6bXHYb4m5zrRC3xPTGwV" # ``` def self.generate_boundary "--------------------------#{Random::Secure.urlsafe_base64(18)}" diff --git a/src/http/multipart/builder.cr b/src/mime/multipart/builder.cr similarity index 97% rename from src/http/multipart/builder.cr rename to src/mime/multipart/builder.cr index 991372cefb57..006f6e66b138 100644 --- a/src/http/multipart/builder.cr +++ b/src/mime/multipart/builder.cr @@ -1,13 +1,11 @@ -require "mime/media_type" - -module HTTP::Multipart +module MIME::Multipart # Builds a multipart MIME message. # # ### Example # # ``` # io = IO::Memory.new # This is a stub. Actually, any IO can be used. - # multipart = HTTP::Multipart::Builder.new(io) + # multipart = MIME::Multipart::Builder.new(io) # multipart.body_part HTTP::Headers{"Content-Type" => "text/plain"}, "hello!" # multipart.finish # io.to_s # => "----------------------------DTf61dRTHYzprx7rwVQhTWr7\r\nContent-Type: text/plain\r\n\r\nhello!\r\n----------------------------DTf61dRTHYzprx7rwVQhTWr7--" @@ -28,7 +26,7 @@ module HTTP::Multipart # # ``` # io = IO::Memory.new # This is a stub. Actually, any IO can be used. - # builder = HTTP::Multipart::Builder.new(io, "a4VF") + # builder = MIME::Multipart::Builder.new(io, "a4VF") # builder.content_type("mixed") # => "multipart/mixed; boundary=\"a4VF\"" # ``` def content_type(subtype = "mixed") diff --git a/src/http/multipart/parser.cr b/src/mime/multipart/parser.cr similarity index 96% rename from src/http/multipart/parser.cr rename to src/mime/multipart/parser.cr index e3dc443836e5..e000c01c27c5 100644 --- a/src/http/multipart/parser.cr +++ b/src/mime/multipart/parser.cr @@ -1,11 +1,11 @@ -module HTTP::Multipart +module MIME::Multipart # Parses multipart MIME messages. # # ### Example # # ``` # multipart = "--aA40\r\nContent-Type: text/plain\r\n\r\nbody\r\n--aA40--" - # parser = HTTP::Multipart::Parser.new(IO::Memory.new(multipart), "aA40") + # parser = MIME::Multipart::Parser.new(IO::Memory.new(multipart), "aA40") # # while parser.has_next? # parser.next do |headers, io| @@ -37,7 +37,7 @@ module HTTP::Multipart # # ``` # multipart = "--aA40\r\nContent-Type: text/plain\r\n\r\nbody\r\n--aA40--" - # parser = HTTP::Multipart::Parser.new(IO::Memory.new(multipart), "aA40") + # parser = MIME::Multipart::Parser.new(IO::Memory.new(multipart), "aA40") # parser.next do |headers, io| # headers["Content-Type"] # => "text/plain" # io.gets_to_end # => "body" From 6b25e16bdd545bfab285fd6cc8bb9ce3a576d748 Mon Sep 17 00:00:00 2001 From: Pantelis Vratsalis Date: Mon, 10 Dec 2018 18:42:21 +0200 Subject: [PATCH 2/2] Changes based on PR feedback --- spec/std/mime/multipart/builder_spec.cr | 2 +- spec/std/mime/multipart/parser_spec.cr | 2 +- spec/std/mime/multipart_spec.cr | 2 +- src/http/formdata.cr | 1 + src/mime.cr | 1 - src/mime/multipart.cr | 2 +- 6 files changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/std/mime/multipart/builder_spec.cr b/spec/std/mime/multipart/builder_spec.cr index b330ab3f2e74..e0411a0f485f 100644 --- a/spec/std/mime/multipart/builder_spec.cr +++ b/spec/std/mime/multipart/builder_spec.cr @@ -1,5 +1,5 @@ require "spec" -require "mime" +require "mime/multipart" describe MIME::Multipart::Builder do it "generates valid multipart messages" do diff --git a/spec/std/mime/multipart/parser_spec.cr b/spec/std/mime/multipart/parser_spec.cr index 0cca2bbabf01..c09c3aed844c 100644 --- a/spec/std/mime/multipart/parser_spec.cr +++ b/spec/std/mime/multipart/parser_spec.cr @@ -1,5 +1,5 @@ require "spec" -require "mime" +require "mime/multipart" private def parse(delim, data, *, gsub = true) data_io = IO::Memory.new(gsub ? data.gsub('\n', "\r\n") : data) diff --git a/spec/std/mime/multipart_spec.cr b/spec/std/mime/multipart_spec.cr index bd36396dafc8..b3fd52f2fac8 100644 --- a/spec/std/mime/multipart_spec.cr +++ b/spec/std/mime/multipart_spec.cr @@ -1,5 +1,5 @@ require "spec" -require "mime" +require "mime/multipart" describe MIME::Multipart do describe ".parse" do diff --git a/src/http/formdata.cr b/src/http/formdata.cr index 19cd2f606042..54c2e6d46eaf 100644 --- a/src/http/formdata.cr +++ b/src/http/formdata.cr @@ -1,4 +1,5 @@ require "./formdata/**" +require "mime/multipart" # Contains utilities for parsing `multipart/form-data` messages, which are # commonly used for encoding HTML form data. diff --git a/src/mime.cr b/src/mime.cr index 7f362c5d94e0..40e73a25effe 100644 --- a/src/mime.cr +++ b/src/mime.cr @@ -1,5 +1,4 @@ require "crystal/system/mime" -require "./mime/**" # This module implements a global MIME registry. # diff --git a/src/mime/multipart.cr b/src/mime/multipart.cr index cffb52a53776..a5a3dd1dcc29 100644 --- a/src/mime/multipart.cr +++ b/src/mime/multipart.cr @@ -5,7 +5,7 @@ require "mime/media_type" # The `MIME::Multipart` module contains utilities for parsing MIME multipart # messages, which contain multiple body parts, each containing a header section # and binary body. The `multipart/form-data` content-type has a separate set of -# utilities in the `MIME::FormData` module. +# utilities in the `HTTP::FormData` module. module MIME::Multipart # Parses a MIME multipart message, yielding `HTTP::Headers` and an `IO` for # each body part.