You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by mf...@redhat.com on 2011/05/04 14:28:32 UTC
[PATCH core] Fix for Webrick incorrectly handling matrix params (DTACLOUD-34)
From: Michal Fojtik <mf...@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
+ #
+ if env['REQUEST_PATH'] == '/'
+ env['REQUEST_URI'] = env['PATH_INFO']
+ env['REQUEST_PATH'] = env['PATH_INFO']
+ end
+
# 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|
--
1.7.4.1
Re: [PATCH core] Fix for Webrick incorrectly handling matrix params (DTACLOUD-34)
Posted by Michal Fojtik <mf...@redhat.com>.
On May 5, 2011, at 1:37 PM, Chris Lalancette wrote:
> On 05/05/11 - 11:51:26AM, Michal Fojtik wrote:
>> This patch throws an error in test:mock unit test:
>>
>> 1) Error:
>> test_it_change_features_after_driver_change(DeltacloudUnitTest::ApiTest):
>> NoMethodError: private method `gsub' called for nil:NilClass
>> ./tests/../lib/sinatra/rack_matrix_params.rb:74:in `call'
>>
>> Seems like this variable (REQUEST_PATH) is empty in rack-test. I fix it using
>> this line:
>>
>> env['REQUEST_PATH'] = env['PATH_INFO'] if env['rack.test']
>>
>> If you're happy with this fix I can push it to SVN.
>
> That seems reasonable enough to me. Thanks!
OK. Pushed to SVN. Thanks for reporting that!
-- Michal
>
> --
> Chris Lalancette
------------------------------------------------------
Michal Fojtik, mfojtik@redhat.com
Deltacloud API: http://deltacloud.org
Re: [PATCH core] Fix for Webrick incorrectly handling matrix params
(DTACLOUD-34)
Posted by Chris Lalancette <cl...@redhat.com>.
On 05/05/11 - 11:51:26AM, Michal Fojtik wrote:
> This patch throws an error in test:mock unit test:
>
> 1) Error:
> test_it_change_features_after_driver_change(DeltacloudUnitTest::ApiTest):
> NoMethodError: private method `gsub' called for nil:NilClass
> ./tests/../lib/sinatra/rack_matrix_params.rb:74:in `call'
>
> Seems like this variable (REQUEST_PATH) is empty in rack-test. I fix it using
> this line:
>
> env['REQUEST_PATH'] = env['PATH_INFO'] if env['rack.test']
>
> If you're happy with this fix I can push it to SVN.
That seems reasonable enough to me. Thanks!
--
Chris Lalancette
Re: [PATCH core] Fix for Webrick incorrectly handling matrix params (DTACLOUD-34)
Posted by Michal Fojtik <mf...@redhat.com>.
On May 4, 2011, at 3:20 PM, Chris Lalancette wrote:
> On 05/04/11 - 02:28:32PM, mfojtik@redhat.com wrote:
>> From: Michal Fojtik <mf...@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?
This patch throws an error in test:mock unit test:
1) Error:
test_it_change_features_after_driver_change(DeltacloudUnitTest::ApiTest):
NoMethodError: private method `gsub' called for nil:NilClass
./tests/../lib/sinatra/rack_matrix_params.rb:74:in `call'
Seems like this variable (REQUEST_PATH) is empty in rack-test. I fix it using
this line:
env['REQUEST_PATH'] = env['PATH_INFO'] if env['rack.test']
If you're happy with this fix I can push it to SVN.
-- Michal
>
> --
> Chris Lalancette
> <fix_matrix_params.patch>
------------------------------------------------------
Michal Fojtik, mfojtik@redhat.com
Deltacloud API: http://deltacloud.org
Re: [PATCH core] Fix for Webrick incorrectly handling matrix params
(DTACLOUD-34)
Posted by Chris Lalancette <cl...@redhat.com>.
On 05/04/11 - 02:28:32PM, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mf...@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