Hi Chris, On Mon, 2011-08-15 at 14:07 -0400, Chris Lalancette wrote: > For the POST, however, libdeltacloud uses > http://localhost:3001/api;driver=ec2/firewalls, but it does not get properly > translated: > > 127.0.0.1 - - [15/Aug/2011 13:57:52] "POST /api;driver=ec2/firewalls > HTTP/1.1" 404 434 0.0135
Nice catch. > It seems like we could fix this either by fixing the URLs that the API returns > initially (so that they look like > http://localhost:3001/api/firewalls;driver=ec2), > or by changing the rackup(?) stuff to do the translation for POST like it > already does for GET. Thoughts? Attached are two patches to fix rack_matrix_params; the issue was that for POST the path rewriting magic that strips matrix params and ultimately allows Sinatra's request dispatching to do the right thing was never run. Unfortunately, there's no tests in these patches - if anybody wants to add some, I'd be very grateful. Feel free to commit if this works for others. David
>From bf709535bc65b24be196948a37219d7b76cb0878 Mon Sep 17 00:00:00 2001 From: David Lutterkort <[email protected]> Date: Mon, 15 Aug 2011 15:32:11 -0700 Subject: [PATCH 1/2] * lib/sinatra/rack_matrix_params.rb: fix indentation, no functional change Signed-off-by: David Lutterkort <[email protected]> --- server/lib/sinatra/rack_matrix_params.rb | 46 +++++++++++++++--------------- 1 files changed, 23 insertions(+), 23 deletions(-) diff --git a/server/lib/sinatra/rack_matrix_params.rb b/server/lib/sinatra/rack_matrix_params.rb index df8855a..e659987 100644 --- a/server/lib/sinatra/rack_matrix_params.rb +++ b/server/lib/sinatra/rack_matrix_params.rb @@ -46,20 +46,20 @@ module Rack uri_components = env['REQUEST_URI'].split('/') matrix_params = {} uri_components.each do |component| - sub_components, value = component.split(/\;(\w+)\=/), nil - next unless sub_components.first # Skip subcomponent if it's empty (usually /) - while param=sub_components.pop do - if value - matrix_params[sub_components.first] ||= {} - matrix_params[sub_components.first].merge!( - param => value - ) - value=nil - next - else - value = param.gsub(/\?.*$/, '') - end - end + sub_components, value = component.split(/\;(\w+)\=/), nil + next unless sub_components.first # Skip subcomponent if it's empty (usually /) + while param=sub_components.pop do + if value + matrix_params[sub_components.first] ||= {} + matrix_params[sub_components.first].merge!( + param => value + ) + value=nil + next + else + value = param.gsub(/\?.*$/, '') + end + end end # If request method is POST, simply include matrix params in form_hash @@ -71,16 +71,16 @@ module Rack env['REQUEST_URI'] = env['REQUEST_PATH'] env['REQUEST_PATH'] = env['PATH_INFO'] end - # Rewrite current path and query string and strip all matrix params from it + # Rewrite current path and query string and strip all matrix params from it env['REQUEST_PATH'] = env['REQUEST_PATH'].gsub(/;([^\/]*)/, '').gsub(/\?(.*)$/, '') - env['PATH_INFO'] = env['REQUEST_PATH'] - env['QUERY_STRING'].gsub!(/;([^\/]*)/, '') - new_params = matrix_params.collect do |component, params| - params.collect { |k,v| "#{component}[#{k}]=#{CGI::escape(v.to_s)}" } - end.flatten - # Add matrix params as a regular GET params - env['QUERY_STRING'] += '&' if not env['QUERY_STRING'].empty? - env['QUERY_STRING'] += "#{new_params.join('&')}" + env['PATH_INFO'] = env['REQUEST_PATH'] + env['QUERY_STRING'].gsub!(/;([^\/]*)/, '') + new_params = matrix_params.collect do |component, params| + params.collect { |k,v| "#{component}[#{k}]=#{CGI::escape(v.to_s)}" } + end.flatten + # Add matrix params as a regular GET params + env['QUERY_STRING'] += '&' if not env['QUERY_STRING'].empty? + env['QUERY_STRING'] += "#{new_params.join('&')}" end @app.call(env) end -- 1.7.6
>From 394a9aeea78a0df2ac5e0090761fd58795a125fe Mon Sep 17 00:00:00 2001 From: David Lutterkort <[email protected]> Date: Mon, 15 Aug 2011 16:22:25 -0700 Subject: [PATCH 2/2] * lib/sinatra/rack_matrix_params.rb: rewrite paths for all reqs We didn't rewrite PATH_INFO, REQUEST_PATH and REQUEST_URI for POST requests, which made dispatching fail if matrix parameters were present. Signed-off-by: David Lutterkort <[email protected]> --- server/lib/sinatra/rack_matrix_params.rb | 51 ++++++++++++++++++++---------- 1 files changed, 34 insertions(+), 17 deletions(-) diff --git a/server/lib/sinatra/rack_matrix_params.rb b/server/lib/sinatra/rack_matrix_params.rb index e659987..13c8f95 100644 --- a/server/lib/sinatra/rack_matrix_params.rb +++ b/server/lib/sinatra/rack_matrix_params.rb @@ -62,25 +62,42 @@ module Rack end end - # If request method is POST, simply include matrix params in form_hash - env['rack.request.form_hash'].merge!(matrix_params) if env['rack.request.form_hash'] + # Two things need to happen to make matrix params work: + # (1) the parameters need to be appended to the 'normal' params + # for the request. 'Normal' really depends on the content + # type of the request, which does not seem accessible from + # Middleware, so we use the existence of + # rack.request.form_hash in the environment to distinguish + # between basic and application/x-www-form-urlencoded + # requests + # (2) the parameters need to be stripped from the appropriate + # path related env variables, so that request dispatching + # does not trip over them - # For other methods it's a way complicated ;-) - if env['REQUEST_METHOD']!='POST' and not matrix_params.keys.empty? - if env['REQUEST_PATH'] == '/' - env['REQUEST_URI'] = env['REQUEST_PATH'] - env['REQUEST_PATH'] = env['PATH_INFO'] + # (1) Rewrite current path by stripping all matrix params from it + if env['REQUEST_PATH'] == '/' + env['REQUEST_URI'] = env['REQUEST_PATH'] + env['REQUEST_PATH'] = env['PATH_INFO'] + end + env['REQUEST_PATH'] = env['REQUEST_PATH'].gsub(/;([^\/]*)/, '').gsub(/\?(.*)$/, '') + env['PATH_INFO'] = env['REQUEST_PATH'] + + # (2) Append the matrix params to the 'normal' request params + # FIXME: Make this work for multipart/form-data + if env['rack.request.form_hash'] + # application/x-www-form-urlencoded, most likely a POST + env['rack.request.form_hash'].merge!(matrix_params) + else + # For other methods it's more complicated + if env['REQUEST_METHOD']!='POST' and not matrix_params.keys.empty? + env['QUERY_STRING'].gsub!(/;([^\/]*)/, '') + new_params = matrix_params.collect do |component, params| + params.collect { |k,v| "#{component}[#{k}]=#{CGI::escape(v.to_s)}" } + end.flatten + # Add matrix params as a regular GET params + env['QUERY_STRING'] += '&' if not env['QUERY_STRING'].empty? + env['QUERY_STRING'] += "#{new_params.join('&')}" end - # Rewrite current path and query string and strip all matrix params from it - env['REQUEST_PATH'] = env['REQUEST_PATH'].gsub(/;([^\/]*)/, '').gsub(/\?(.*)$/, '') - env['PATH_INFO'] = env['REQUEST_PATH'] - env['QUERY_STRING'].gsub!(/;([^\/]*)/, '') - new_params = matrix_params.collect do |component, params| - params.collect { |k,v| "#{component}[#{k}]=#{CGI::escape(v.to_s)}" } - end.flatten - # Add matrix params as a regular GET params - env['QUERY_STRING'] += '&' if not env['QUERY_STRING'].empty? - env['QUERY_STRING'] += "#{new_params.join('&')}" end @app.call(env) end -- 1.7.6
