Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add Rack::NestedParams; based on Rails UrlEncodedPairParser
* Cleaned up whitespace errors [rtomayko] * Added note to README [rtomayko]
- Loading branch information
Showing
4 changed files
with
192 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Original file line | Diff line number | Diff line change |
---|---|---|---|
@@ -0,0 +1,143 @@ | |||
require 'cgi' | |||
require 'strscan' | |||
|
|||
module Rack | |||
# Rack middleware for parsing POST/PUT body data into nested parameters | |||
class NestedParams | |||
|
|||
CONTENT_TYPE = 'CONTENT_TYPE'.freeze | |||
POST_BODY = 'rack.input'.freeze | |||
FORM_INPUT = 'rack.request.form_input'.freeze | |||
FORM_HASH = 'rack.request.form_hash'.freeze | |||
FORM_VARS = 'rack.request.form_vars'.freeze | |||
|
|||
# supported content type | |||
URL_ENCODED = 'application/x-www-form-urlencoded'.freeze | |||
|
|||
def initialize(app) | |||
@app = app | |||
end | |||
|
|||
def call(env) | |||
if form_vars = env[FORM_VARS] | |||
env[FORM_HASH] = parse_query_parameters(form_vars) | |||
elsif env[CONTENT_TYPE] == URL_ENCODED | |||
post_body = env[POST_BODY] | |||
env[FORM_INPUT] = post_body | |||
env[FORM_HASH] = parse_query_parameters(post_body.read) | |||
post_body.rewind if post_body.respond_to?(:rewind) | |||
end | |||
@app.call(env) | |||
end | |||
|
|||
## the rest is nabbed from Rails ## | |||
|
|||
def parse_query_parameters(query_string) | |||
return {} if query_string.nil? or query_string.empty? | |||
|
|||
pairs = query_string.split('&').collect do |chunk| | |||
next if chunk.empty? | |||
key, value = chunk.split('=', 2) | |||
next if key.empty? | |||
value = value.nil? ? nil : CGI.unescape(value) | |||
[ CGI.unescape(key), value ] | |||
end.compact | |||
|
|||
UrlEncodedPairParser.new(pairs).result | |||
end | |||
|
|||
class UrlEncodedPairParser < StringScanner | |||
attr_reader :top, :parent, :result | |||
|
|||
def initialize(pairs = []) | |||
super('') | |||
@result = {} | |||
pairs.each { |key, value| parse(key, value) } | |||
end | |||
|
|||
KEY_REGEXP = %r{([^\[\]=&]+)} | |||
BRACKETED_KEY_REGEXP = %r{\[([^\[\]=&]+)\]} | |||
|
|||
# Parse the query string | |||
def parse(key, value) | |||
self.string = key | |||
@top, @parent = result, nil | |||
|
|||
# First scan the bare key | |||
key = scan(KEY_REGEXP) or return | |||
key = post_key_check(key) | |||
|
|||
# Then scan as many nestings as present | |||
until eos? | |||
r = scan(BRACKETED_KEY_REGEXP) or return | |||
key = self[1] | |||
key = post_key_check(key) | |||
end | |||
|
|||
bind(key, value) | |||
end | |||
|
|||
private | |||
# After we see a key, we must look ahead to determine our next action. Cases: | |||
# | |||
# [] follows the key. Then the value must be an array. | |||
# = follows the key. (A value comes next) | |||
# & or the end of string follows the key. Then the key is a flag. | |||
# otherwise, a hash follows the key. | |||
def post_key_check(key) | |||
if scan(/\[\]/) # a[b][] indicates that b is an array | |||
container(key, Array) | |||
nil | |||
elsif check(/\[[^\]]/) # a[b] indicates that a is a hash | |||
container(key, Hash) | |||
nil | |||
else # End of key? We do nothing. | |||
key | |||
end | |||
end | |||
|
|||
# Add a container to the stack. | |||
def container(key, klass) | |||
type_conflict! klass, top[key] if top.is_a?(Hash) && top.key?(key) && ! top[key].is_a?(klass) | |||
value = bind(key, klass.new) | |||
type_conflict! klass, value unless value.is_a?(klass) | |||
push(value) | |||
end | |||
|
|||
# Push a value onto the 'stack', which is actually only the top 2 items. | |||
def push(value) | |||
@parent, @top = @top, value | |||
end | |||
|
|||
# Bind a key (which may be nil for items in an array) to the provided value. | |||
def bind(key, value) | |||
if top.is_a? Array | |||
if key | |||
if top[-1].is_a?(Hash) && ! top[-1].key?(key) | |||
top[-1][key] = value | |||
else | |||
top << {key => value} | |||
end | |||
push top.last | |||
return top[key] | |||
else | |||
top << value | |||
return value | |||
end | |||
elsif top.is_a? Hash | |||
key = CGI.unescape(key) | |||
parent << (@top = {}) if top.key?(key) && parent.is_a?(Array) | |||
top[key] ||= value | |||
return top[key] | |||
else | |||
raise ArgumentError, "Don't know what to do: top is #{top.inspect}" | |||
end | |||
end | |||
|
|||
def type_conflict!(klass, value) | |||
raise TypeError, "Conflicting types for parameter containers. Expected an instance of #{klass} but found an instance of #{value.class}. This can be caused by colliding Array and Hash parameters like qs[]=value&qs[key]=value. (The parameters received were #{value.inspect}.)" | |||
end | |||
end | |||
|
|||
end | |||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Original file line | Diff line number | Diff line change |
---|---|---|---|
@@ -0,0 +1,45 @@ | |||
require 'rack/mock' | |||
require 'rack/contrib/nested_params' | |||
require 'rack/methodoverride' | |||
|
|||
context Rack::NestedParams do | |||
|
|||
App = lambda { |env| [200, {'Content-Type' => 'text/plain'}, Rack::Request.new(env)] } | |||
|
|||
def env_for_post_with_headers(path, headers, body) | |||
Rack::MockRequest.env_for(path, {:method => "POST", :input => body}.merge(headers)) | |||
end | |||
|
|||
def form_post(params, content_type = 'application/x-www-form-urlencoded') | |||
params = Rack::Utils.build_query(params) if Hash === params | |||
env_for_post_with_headers('/', {'CONTENT_TYPE' => content_type}, params) | |||
end | |||
|
|||
def middleware | |||
Rack::NestedParams.new(App) | |||
end | |||
|
|||
specify "should handle requests with POST body Content-Type of application/x-www-form-urlencoded" do | |||
req = middleware.call(form_post({'foo[bar][baz]' => 'nested'})).last | |||
req.POST.should.equal({"foo" => { "bar" => { "baz" => "nested" }}}) | |||
end | |||
|
|||
specify "should not parse requests with other Content-Type" do | |||
req = middleware.call(form_post({'foo[bar][baz]' => 'nested'}, 'text/plain')).last | |||
req.POST.should.equal({}) | |||
end | |||
|
|||
specify "should work even after another middleware already parsed the request" do | |||
app = Rack::MethodOverride.new(middleware) | |||
req = app.call(form_post({'_method' => 'put', 'foo[bar]' => 'nested'})).last | |||
req.POST.should.equal({'_method' => 'put', "foo" => { "bar" => "nested" }}) | |||
req.put?.should.equal true | |||
end | |||
|
|||
specify "should make first boolean have precedence even after request already parsed" do | |||
app = Rack::MethodOverride.new(middleware) | |||
req = app.call(form_post("foo=1&foo=0")).last | |||
req.POST.should.equal({"foo" => '1'}) | |||
end | |||
|
|||
end |
86a2ced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 Rack Core :)
(doesn’t have to be the Rails implementation though)
86a2ced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Second what he said
86a2ced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yikes that implementation is pretty large, has anyone benchmarked this? I doubt its a big deal but if it were to big in core as fast as possible would be nice, unless its really not an issue
86a2ced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we bake it into Rack::Request, it can be much simpler and faster.
Its much more difficult to post process the hash than to just parse the original query string.
http://rack.lighthouseapp.com/projects/22435/tickets/3-add-support-for-nested-params-parsing-in-rackrequest
86a2ced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nested params on Rack::Request or Rack::Utils should definitely be considered for rack core. Huge +1 to that. In the meantime, rack-contrib can provide middleware / utilities.
86a2ced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for Rack core, -1 for using this exact code. I added it for my own purposes, but I think it’s too large and opinionated for Rack
86a2ced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sinatra’s with a few modifications would be the cleanest that I have come across. _why’s is not bad either but you cant really read it and its slower
86a2ced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-1 for rack-core. Seriously, one can make the given specs pass with like 10 lines of code.
86a2ced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-1?? why? it belongs in core, then people can stop creating weird solutions like the rails one lol, rack core can just supply good/small/fast solution
86a2ced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@visionmedia I’ve adapted Merbs parser for Rack. Its pretty small and pretty fast (you know those Merb guys ;) Please +1 the ticket I posted.
86a2ced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please voice your opinion!
http://groups.google.com/group/rack-devel/browse_thread/thread/732e2c1e014bde30
86a2ced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m with @manveru on this one. Minus 1 for rack core. This parsing strategy is based on a convention that just happens to be shared by more than one framework. This makes it ideal for rack-contrib. Placing it in rack core makes a statement about rack – that it is more than just an interface between web apps/frameworks and the web server, and that it is the appropriate place for shared code. I view rack as more of an interface than an abstract class, but yield to Christian and the rest of the core team to weigh in on the core principles defining what goes into core.
86a2ced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think converting “foo[bar]” into ruby’s foo[‘bar’] is a lot more than convention. It’s common sense.
86a2ced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the sound of it, looks like manveru’s objection is to the code and not the feature itself. Which is valid. If 10 lines can comply with the same spec, and is faster, why not.
86a2ced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly, I’m not against the feature, but the discussion is up on the mailing list already anyway.