deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Lalancette <clala...@redhat.com>
Subject Re: [PATCH core] Fix for Webrick incorrectly handling matrix params (DTACLOUD-34)
Date Wed, 04 May 2011 13:20:42 GMT
On 05/04/11 - 02:28:32PM, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mfojtik@redhat.com>
> 
> ---
>  server/lib/sinatra/rack_matrix_params.rb |   16 +++++++++++++++-
>  1 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/server/lib/sinatra/rack_matrix_params.rb b/server/lib/sinatra/rack_matrix_params.rb
> index a4528b0..cc038ab 100644
> --- a/server/lib/sinatra/rack_matrix_params.rb
> +++ b/server/lib/sinatra/rack_matrix_params.rb
> @@ -66,8 +66,22 @@ module Rack
>  
>        # For other methods it's a way complicated ;-)
>        if env['REQUEST_METHOD']!='POST' and not matrix_params.keys.empty?
> +        # Workaround for WEBrick 1.3.1
> +        # In WEBrick 1.3.1 variables are set incorectly like:
> +        #
> +        # GET /api;driver=ec2/hardware_profiles
> +        # 
> +        # REQUEST_PATH = http://localhost:3001/api;driver=mock/hardware_profiles
> +        # REQUEST_URI = /
> +        # PATH_INFO = /api;driver=mock/hardware_profiles

Ah, OK, so the environment is wrong.

> +        #
> +        if env['REQUEST_PATH'] == '/'
> +          env['REQUEST_URI'] = env['PATH_INFO']
> +          env['REQUEST_PATH'] = env['PATH_INFO']
> +        end
> +

But I'm not sure I would go about fixing it this way.  That is, if we think
about it, don't we really want the variables to end up being like this:

REQUEST_PATH = /api;driver=mock/hardware_profiles
REQUEST_URI = http://localhost:3001/api;driver=mock/hardware_profiles
PATH_INFO = /api;driver=mock/hardware_profiles

If we do that, then we can fix the line below to be something more like:

env['REQUEST_PATH'] = env['REQUEST_PATH'].gsub(/;([^\/]*)/, '').gsub(/\?(.*)$/, '')

Which makes more sense to me.

>  	# Rewrite current path and query string and strip all matrix params from it
> -	env['REQUEST_PATH'], env['PATH_INFO'] = env['REQUEST_URI'].gsub(/;([^\/]*)/, '').gsub(/\?(.*)$/,
'')
> +	env['REQUEST_PATH'] = env['REQUEST_URI'].gsub(/;([^\/]*)/, '').gsub(/\?(.*)$/, '')
>  	env['PATH_INFO'] = env['REQUEST_PATH']
>  	env['QUERY_STRING'].gsub!(/;([^\/]*)/, '')
>  	new_params = matrix_params.collect do |component, params|

So I've attached a counter-proposal that seems to work in some very basic
testing for me.  What do you think?

-- 
Chris Lalancette

Mime
View raw message