Invalid query string handling

Invalid query string handling

[Maciej Lotkowski]

Hi,

recently I stumbled upon a problem:

require 'rubygems'
require 'rack'
require 'rack/mock'


app = lambda { |env|
  Rack::Request.new(env).GET
}
app.call(Rack::MockRequest.env_for("/kiszka?foo=&foo[option]=kiszka"))

The call raises an exception due the invalid query string

TypeError: expected Hash (got String) for param `_REQUEST'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/gems/1.8/gems/
rack-1.0.1/lib/rack/utils.rb:93:in `normalize_params'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/gems/1.8/gems/
rack-1.0.1/lib/rack/utils.rb:62:in `parse_nested_query'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/gems/1.8/gems/
rack-1.0.1/lib/rack/utils.rb:60:in `each'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/gems/1.8/gems/
rack-1.0.1/lib/rack/utils.rb:60:in `parse_nested_query'
    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/gems/1.8/gems/
rack-1.0.1/lib/rack/request.rb:119:in `GET'

As a workaround I've created a middleware which creates a
Rack::Request
instance, calls GET, catches TypeError if there is any and returns
BadRequest
if an error occured. What is the "right" way to handle this? I think
it would be
better to throw some InvalidQueryString exception from
Rack::Request#GET,
wouldn't it?

--
Maciej Lotkowski

Re: Invalid query string handling

[James Tucker]

[-- Attachment #1: Type: text/plain, Size: 1439 bytes --]


On 23 Nov 2009, at 12:41, Maciej Lotkowski wrote:

> Hi,
> 
> recently I stumbled upon a problem:
> 
> require 'rubygems'
> require 'rack'
> require 'rack/mock'
> 
> 
> app = lambda { |env|
>  Rack::Request.new(env).GET
> }
> app.call(Rack::MockRequest.env_for("/kiszka?foo=&foo[option]=kiszka"))

remove foo=<empty string> from the front of your query string.

> 
> The call raises an exception due the invalid query string
> 
> TypeError: expected Hash (got String) for param `_REQUEST'
>    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/gems/1.8/gems/
> rack-1.0.1/lib/rack/utils.rb:93:in `normalize_params'
>    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/gems/1.8/gems/
> rack-1.0.1/lib/rack/utils.rb:62:in `parse_nested_query'
>    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/gems/1.8/gems/
> rack-1.0.1/lib/rack/utils.rb:60:in `each'
>    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/gems/1.8/gems/
> rack-1.0.1/lib/rack/utils.rb:60:in `parse_nested_query'
>    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/gems/1.8/gems/
> rack-1.0.1/lib/rack/request.rb:119:in `GET'
> 
> As a workaround I've created a middleware which creates a
> Rack::Request
> instance, calls GET, catches TypeError if there is any and returns
> BadRequest
> if an error occured. What is the "right" way to handle this? I think
> it would be
> better to throw some InvalidQueryString exception from
> Rack::Request#GET,
> wouldn't it?
> 
> --
> Maciej Lotkowski


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3679 bytes --]

Re: Invalid query string handling

[Magnus Holm]

Maybe we should have a Rack::BadRequest, which can be raised when needed?

//Magnus Holm



On Mon, Nov 23, 2009 at 13:41, Maciej Lotkowski
<maciej.lotkowski@gmail.com> wrote:
> Hi,
>
> recently I stumbled upon a problem:
>
> require 'rubygems'
> require 'rack'
> require 'rack/mock'
>
>
> app = lambda { |env|
>  Rack::Request.new(env).GET
> }
> app.call(Rack::MockRequest.env_for("/kiszka?foo=&foo[option]=kiszka"))
>
> The call raises an exception due the invalid query string
>
> TypeError: expected Hash (got String) for param `_REQUEST'
>    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/gems/1.8/gems/
> rack-1.0.1/lib/rack/utils.rb:93:in `normalize_params'
>    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/gems/1.8/gems/
> rack-1.0.1/lib/rack/utils.rb:62:in `parse_nested_query'
>    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/gems/1.8/gems/
> rack-1.0.1/lib/rack/utils.rb:60:in `each'
>    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/gems/1.8/gems/
> rack-1.0.1/lib/rack/utils.rb:60:in `parse_nested_query'
>    /opt/ruby-enterprise-1.8.7-2009.10/lib/ruby/gems/1.8/gems/
> rack-1.0.1/lib/rack/request.rb:119:in `GET'
>
> As a workaround I've created a middleware which creates a
> Rack::Request
> instance, calls GET, catches TypeError if there is any and returns
> BadRequest
> if an error occured. What is the "right" way to handle this? I think
> it would be
> better to throw some InvalidQueryString exception from
> Rack::Request#GET,
> wouldn't it?
>
> --
> Maciej Lotkowski
>

Re: Invalid query string handling

[Maciej Lotkowski]

On Tue, Nov 24, 2009 at 10:18 AM, James Tucker <jftucker@gmail.com> wrote:
> On 23 Nov 2009, at 12:41, Maciej Lotkowski wrote:
>
>> Hi,
>>
>> recently I stumbled upon a problem:
>>
>> require 'rubygems'
>> require 'rack'
>> require 'rack/mock'
>>
>>
>> app = lambda { |env|
>>  Rack::Request.new(env).GET
>> }
>> app.call(Rack::MockRequest.env_for("/kiszka?foo=&foo[option]=kiszka"))
>
> remove foo=<empty string> from the front of your query string.

It's not my query string, smeone just tries to flood my app with
strange requests.
But that's not the point.


On Tue, Nov 24, 2009 at 10:25 AM, Magnus Holm <judofyr@gmail.com> wrote:
> Maybe we should have a Rack::BadRequest, which can be raised when needed?

That's what I mean. TypeError isn't an exception which you can expect to raised
because of invalid request.

-- 
Maciej Lotkowski

Re: Invalid query string handling

[James Tucker]

[-- Attachment #1: Type: text/plain, Size: 1133 bytes --]


On 24 Nov 2009, at 09:41, Maciej Lotkowski wrote:

> On Tue, Nov 24, 2009 at 10:18 AM, James Tucker <jftucker@gmail.com> wrote:
>> On 23 Nov 2009, at 12:41, Maciej Lotkowski wrote:
>> 
>>> Hi,
>>> 
>>> recently I stumbled upon a problem:
>>> 
>>> require 'rubygems'
>>> require 'rack'
>>> require 'rack/mock'
>>> 
>>> 
>>> app = lambda { |env|
>>>  Rack::Request.new(env).GET
>>> }
>>> app.call(Rack::MockRequest.env_for("/kiszka?foo=&foo[option]=kiszka"))
>> 
>> remove foo=<empty string> from the front of your query string.
> 
> It's not my query string, smeone just tries to flood my app with
> strange requests.
> But that's not the point.
> 
> 
> On Tue, Nov 24, 2009 at 10:25 AM, Magnus Holm <judofyr@gmail.com> wrote:
>> Maybe we should have a Rack::BadRequest, which can be raised when needed?
> 
> That's what I mean. TypeError isn't an exception which you can expect to raised
> because of invalid request.

Combine this with the other issues in various request parsing discussions, I'm starting to think this wants to be pushed right into middleware.

> 
> -- 
> Maciej Lotkowski


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3679 bytes --]