From a5e4bc21f35f4b96142cfba042f3903597fbdc0d Mon Sep 17 00:00:00 2001 From: Matthew Gerrior Date: Sat, 11 Jun 2016 16:52:07 -0400 Subject: [PATCH 1/2] Allow multiple values for a single parameter key. --- spec/param_parser_spec.cr | 27 +++++++++++++++++++++++++++ spec/request_spec.cr | 14 ++++++++++++++ src/kemal/param_parser.cr | 22 ++++++++++++++++++---- src/kemal/request.cr | 15 +++++++++++++-- 4 files changed, 72 insertions(+), 6 deletions(-) create mode 100644 spec/request_spec.cr diff --git a/spec/param_parser_spec.cr b/spec/param_parser_spec.cr index f2d6c63..18b1d24 100644 --- a/spec/param_parser_spec.cr +++ b/spec/param_parser_spec.cr @@ -11,6 +11,16 @@ describe "ParamParser" do query_params["hasan"].should eq "cemal" end + it "parses multiple values for query params" do + route = Route.new "POST", "/" do |env| + hasan = env.params.query["hasan"] + "Hello #{hasan}" + end + request = HTTP::Request.new("POST", "/?hasan=cemal&hasan=lamec") + query_params = Kemal::ParamParser.new(request).query + query_params["hasan"].should eq ["cemal", "lamec"] + end + it "parses url params" do kemal = Kemal::RouteHandler::INSTANCE kemal.add_route "POST", "/hello/:hasan" do |env| @@ -45,6 +55,23 @@ describe "ParamParser" do body_params.should eq({"name" => "serdar", "age" => "99"}) end + it "parses multiple values in request body" do + route = Route.new "POST", "/" do |env| + hasan = env.params.body["hasan"] + "Hello #{hasan}" + end + + request = HTTP::Request.new( + "POST", + "/", + body: "hasan=cemal&hasan=lamec", + headers: HTTP::Headers{"Content-Type": "application/x-www-form-urlencoded"}, + ) + + body_params = Kemal::ParamParser.new(request).body + body_params.should eq({"hasan" => ["cemal", "lamec"]}) + end + context "when content type is application/json" do it "parses request body" do route = Route.new "POST", "/" { } diff --git a/spec/request_spec.cr b/spec/request_spec.cr new file mode 100644 index 0000000..83cad17 --- /dev/null +++ b/spec/request_spec.cr @@ -0,0 +1,14 @@ +require "./spec_helper" + +describe "Request" do + it "uses the last method override provided" do + request = HTTP::Request.new( + "POST", + "/", + body: "_method=PUT&_method=PATCH", + headers: HTTP::Headers{"Content-Type": "application/x-www-form-urlencoded"}, + ) + + request.override_method.should eq("PATCH") + end +end diff --git a/src/kemal/param_parser.cr b/src/kemal/param_parser.cr index 96ceced..e92cd0b 100644 --- a/src/kemal/param_parser.cr +++ b/src/kemal/param_parser.cr @@ -11,8 +11,8 @@ class Kemal::ParamParser def initialize(@request : HTTP::Request) @url = {} of String => String - @query = {} of String => String - @body = {} of String => String + @query = {} of String => String | Array(String) + @body = {} of String => String | Array(String) @json = {} of String => AllParamTypes @url_parsed = false @query_parsed = false @@ -68,12 +68,26 @@ class Kemal::ParamParser end def parse_part(part) - part_params = {} of String => String + part_params = {} of String => String | Array(String) + if part HTTP::Params.parse(part) do |key, value| - part_params[key as String] ||= value as String + key_string = key as String + value_string = value as String + current_value = part_params[key_string]? + + part_params[key_string] = if current_value + if current_value.is_a?(Array) + current_value << value_string + else + [current_value, value_string] + end + else + value_string + end end end + part_params end end diff --git a/src/kemal/request.cr b/src/kemal/request.cr index 946dfed..a7886c1 100644 --- a/src/kemal/request.cr +++ b/src/kemal/request.cr @@ -10,18 +10,29 @@ class HTTP::Request # Checks if request params contain _method param to override request incoming method private def check_for_method_override! @override_method = @method + if @method == "POST" params = Kemal::ParamParser.new(self).body + if params.has_key?("_method") && HTTP::Request.override_method_valid?(params["_method"]) - @override_method = params["_method"] + _method = params["_method"] + + @override_method = if _method.is_a?(Array) + _method.last + else + _method + end end end + @override_method end # Checks if method contained in _method param is valid one def self.override_method_valid?(override_method) - return false unless override_method.is_a?(String) + return false unless override_method.is_a?(String) || override_method.is_a?(Array(String)) + + override_method = override_method.last if override_method.is_a?(Array(String)) override_method = override_method.upcase override_method == "PUT" || override_method == "PATCH" || override_method == "DELETE" end From d1f9c4394b18e0e0b27332a6beff333f596e33de Mon Sep 17 00:00:00 2001 From: Matthew Gerrior Date: Tue, 14 Jun 2016 07:32:13 -0400 Subject: [PATCH 2/2] Opt for built-in HTTP::Params class instead. --- spec/param_parser_spec.cr | 24 +++++++++++++++--------- spec/request_spec.cr | 14 -------------- src/kemal/param_parser.cr | 26 +++----------------------- src/kemal/request.cr | 15 ++------------- 4 files changed, 20 insertions(+), 59 deletions(-) delete mode 100644 spec/request_spec.cr diff --git a/spec/param_parser_spec.cr b/spec/param_parser_spec.cr index 18b1d24..6f4507e 100644 --- a/spec/param_parser_spec.cr +++ b/spec/param_parser_spec.cr @@ -18,7 +18,7 @@ describe "ParamParser" do end request = HTTP::Request.new("POST", "/?hasan=cemal&hasan=lamec") query_params = Kemal::ParamParser.new(request).query - query_params["hasan"].should eq ["cemal", "lamec"] + query_params.fetch_all("hasan").should eq ["cemal", "lamec"] end it "parses url params" do @@ -49,10 +49,14 @@ describe "ParamParser" do ) query_params = Kemal::ParamParser.new(request).query - query_params.should eq({"hasan" => "cemal"}) + {"hasan": "cemal"}.each do |k, v| + query_params[k].should eq(v) + end body_params = Kemal::ParamParser.new(request).body - body_params.should eq({"name" => "serdar", "age" => "99"}) + {"name": "serdar", "age": "99"}.each do |k, v| + body_params[k].should eq(v) + end end it "parses multiple values in request body" do @@ -69,7 +73,7 @@ describe "ParamParser" do ) body_params = Kemal::ParamParser.new(request).body - body_params.should eq({"hasan" => ["cemal", "lamec"]}) + body_params.fetch_all("hasan").should eq(["cemal", "lamec"]) end context "when content type is application/json" do @@ -112,7 +116,9 @@ describe "ParamParser" do ) query_params = Kemal::ParamParser.new(request).query - query_params.should eq({"foo": "bar"}) + {"foo": "bar"}.each do |k, v| + query_params[k].should eq(v) + end json_params = Kemal::ParamParser.new(request).json json_params.should eq({"_json": [1]}) @@ -131,10 +137,10 @@ describe "ParamParser" do url_params.should eq({} of String => String) query_params = Kemal::ParamParser.new(request).query - query_params.should eq({} of String => String) + query_params.to_s.should eq("") body_params = Kemal::ParamParser.new(request).body - body_params.should eq({} of String => String) + body_params.to_s.should eq("") json_params = Kemal::ParamParser.new(request).json json_params.should eq({} of String => AllParamTypes) @@ -158,10 +164,10 @@ describe "ParamParser" do ) query_params = Kemal::ParamParser.new(request).query - query_params.should eq({"hasan" => "cemal"}) + query_params["hasan"].should eq("cemal") body_params = Kemal::ParamParser.new(request).body - body_params.should eq({} of String => String) + body_params.to_s.should eq("") end end end diff --git a/spec/request_spec.cr b/spec/request_spec.cr deleted file mode 100644 index 83cad17..0000000 --- a/spec/request_spec.cr +++ /dev/null @@ -1,14 +0,0 @@ -require "./spec_helper" - -describe "Request" do - it "uses the last method override provided" do - request = HTTP::Request.new( - "POST", - "/", - body: "_method=PUT&_method=PATCH", - headers: HTTP::Headers{"Content-Type": "application/x-www-form-urlencoded"}, - ) - - request.override_method.should eq("PATCH") - end -end diff --git a/src/kemal/param_parser.cr b/src/kemal/param_parser.cr index e92cd0b..d3a428c 100644 --- a/src/kemal/param_parser.cr +++ b/src/kemal/param_parser.cr @@ -11,8 +11,8 @@ class Kemal::ParamParser def initialize(@request : HTTP::Request) @url = {} of String => String - @query = {} of String => String | Array(String) - @body = {} of String => String | Array(String) + @query = HTTP::Params.new({} of String => Array(String)) + @body = HTTP::Params.new({} of String => Array(String)) @json = {} of String => AllParamTypes @url_parsed = false @query_parsed = false @@ -68,26 +68,6 @@ class Kemal::ParamParser end def parse_part(part) - part_params = {} of String => String | Array(String) - - if part - HTTP::Params.parse(part) do |key, value| - key_string = key as String - value_string = value as String - current_value = part_params[key_string]? - - part_params[key_string] = if current_value - if current_value.is_a?(Array) - current_value << value_string - else - [current_value, value_string] - end - else - value_string - end - end - end - - part_params + HTTP::Params.parse(part || "") end end diff --git a/src/kemal/request.cr b/src/kemal/request.cr index a7886c1..946dfed 100644 --- a/src/kemal/request.cr +++ b/src/kemal/request.cr @@ -10,29 +10,18 @@ class HTTP::Request # Checks if request params contain _method param to override request incoming method private def check_for_method_override! @override_method = @method - if @method == "POST" params = Kemal::ParamParser.new(self).body - if params.has_key?("_method") && HTTP::Request.override_method_valid?(params["_method"]) - _method = params["_method"] - - @override_method = if _method.is_a?(Array) - _method.last - else - _method - end + @override_method = params["_method"] end end - @override_method end # Checks if method contained in _method param is valid one def self.override_method_valid?(override_method) - return false unless override_method.is_a?(String) || override_method.is_a?(Array(String)) - - override_method = override_method.last if override_method.is_a?(Array(String)) + return false unless override_method.is_a?(String) override_method = override_method.upcase override_method == "PUT" || override_method == "PATCH" || override_method == "DELETE" end