On Jul 2, 2009, at 7:42 AM, Jordan Curzon wrote: > > We are concerned about empty strings also. A query string with two > ampersands next two each other parses to empty strings in both the > key and the value. I'll submit a new patch.
Seems like we should handle this on both sides - the client shouldn't be sending empty arguments, and the server should be able to handle empty arguments. Any idea which arguments are being sent empty? > > On Jul 2, 5:36 am, Luke Kanies <[email protected]> wrote: >> I wondered about the blank? thing, should have asked. :/ >> >> We're apparently only concerned about it being nil, not an empty >> string, so I think just checking for .nil? should be sufficient. >> >> On Jul 2, 2009, at 7:17 AM, Jordan Curzon wrote: >> >> >> >> >> >>> It appears that blank? is not always a method of String. blank? is >>> an >>> extension from rails in active_support. Would it be better to find >>> out >>> why String is missing this method in this case or to just deal >>> with it >>> and use "var.nil? || var.empty?" ? >> >>> err: undefined method `blank?' for "facts":String >> >>> /opt/puppet/lib/puppet/network/http/handler.rb:185:in >>> `decode_params' >>> /usr/local/lib/site_ruby/1.8/rubygems/custom_require.rb:31:in >>> `inject' >>> /opt/puppet/lib/puppet/network/http/handler.rb:183:in `each' >>> /opt/puppet/lib/puppet/network/http/handler.rb:183:in `inject' >>> /opt/puppet/lib/puppet/network/http/handler.rb:183:in >>> `decode_params' >>> /opt/puppet/lib/puppet/network/http/mongrel/rest.rb:27:in `params' >>> /opt/puppet/lib/puppet/network/http/handler.rb:43:in `process' >>> /usr/lib/ruby/gems/1.8/gems/mongrel-1.1.5/lib/mongrel.rb:159:in >>> `process_client' >>> /usr/lib/ruby/gems/1.8/gems/mongrel-1.1.5/lib/mongrel.rb:158:in >>> `each' >>> /usr/lib/ruby/gems/1.8/gems/mongrel-1.1.5/lib/mongrel.rb:158:in >>> `process_client' >>> /usr/lib/ruby/gems/1.8/gems/mongrel-1.1.5/lib/mongrel.rb:285:in >>> `run' >>> /usr/lib/ruby/gems/1.8/gems/mongrel-1.1.5/lib/mongrel.rb:285:in >>> `initialize' >>> /usr/lib/ruby/gems/1.8/gems/mongrel-1.1.5/lib/mongrel.rb:285:in >>> `new' >>> /usr/lib/ruby/gems/1.8/gems/mongrel-1.1.5/lib/mongrel.rb:285:in >>> `run' >>> /usr/lib/ruby/gems/1.8/gems/mongrel-1.1.5/lib/mongrel.rb:268:in >>> `initialize' >>> /usr/lib/ruby/gems/1.8/gems/mongrel-1.1.5/lib/mongrel.rb:268:in >>> `new' >>> /usr/lib/ruby/gems/1.8/gems/mongrel-1.1.5/lib/mongrel.rb:268:in >>> `run' >>> /opt/puppet/lib/puppet/network/http/mongrel.rb:22:in `listen' >>> /opt/puppet/lib/puppet/network/server.rb:131:in `listen' >>> /opt/puppet/lib/puppet/network/server.rb:146:in `start' >>> /opt/puppet/lib/puppet/daemon.rb:128:in `start' >>> /opt/puppet/lib/puppet/application/puppetmasterd.rb:96:in `main' >>> /opt/puppet/lib/puppet/application.rb:226:in `send' >>> /opt/puppet/lib/puppet/application.rb:226:in `run_command' >>> /opt/puppet/lib/puppet/application.rb:217:in `run' >>> /opt/puppet/sbin/puppetmasterd:66 >>> err: undefined method `blank?' for "facts":String >> >>> On Jul 1, 9:49 pm, Luke Kanies <[email protected]> wrote: >>>> +1 >> >>>> Awesome, thanks. >> >>>> On Jul 1, 2009, at 11:39 PM, Jordan Curzon wrote: >> >>>>> Mongrel::HttpRequest.query_parse outputs a params hash with nil >>>>> keys given certain query strings. >>>>> Network::HTTP::Handler.decode_params >>>>> needs to check the incoming values. >> >>>>> Signed-off-by: Jordan Curzon <[email protected]> >>>>> --- >>>>> lib/puppet/network/http/handler.rb | 2 ++ >>>>> spec/unit/network/http/mongrel/rest.rb | 5 +++++ >>>>> 2 files changed, 7 insertions(+), 0 deletions(-) >> >>>>> diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/ >>>>> network/ >>>>> http/handler.rb >>>>> index 9528d39..c6b809d 100644 >>>>> --- a/lib/puppet/network/http/handler.rb >>>>> +++ b/lib/puppet/network/http/handler.rb >>>>> @@ -182,6 +182,8 @@ module Puppet::Network::HTTP::Handler >>>>> def decode_params(params) >>>>> params.inject({}) do |result, ary| >>>>> param, value = ary >>>>> + next result if param.blank? >>>>> + >>>>> param = param.to_sym >> >>>>> # These shouldn't be allowed to be set by clients >>>>> diff --git a/spec/unit/network/http/mongrel/rest.rb b/spec/unit/ >>>>> network/http/mongrel/rest.rb >>>>> index 5a5d2cf..abd573a 100755 >>>>> --- a/spec/unit/network/http/mongrel/rest.rb >>>>> +++ b/spec/unit/network/http/mongrel/rest.rb >>>>> @@ -92,6 +92,11 @@ describe "Puppet::Network::HTTP::MongrelREST" >>>>> do >>>>> @request.stubs(:params).returns({}) >>>>> end >> >>>>> + it "should skip empty parameter values" do >>>>> + >>>>> @request.expects(:params).returns('QUERY_STRING' => >>>>> "&=") >>>>> + lambda { @handler.params(@request) }.should_not >>>>> raise_error >>>>> + end >>>>> + >>>>> it "should include the HTTP request parameters, with >>>>> the >>>>> keys as symbols" do >>>>> @request.expects(:params).returns('QUERY_STRING' >>>>> => >>>>> 'foo=baz&bar=xyzzy') >>>>> result = @handler.params(@request) >>>>> -- >>>>> 1.6.3.3 >> >>>> -- >>>> Men will wrangle for religion; write for it; fight for it; die for >>>> it; >>>> anything but live for it. --Charles Caleb Colton >>>> --------------------------------------------------------------------- >>>> Luke Kanies |http://reductivelabs.com|http://madstop.com >> >> -- >> I can't understand why a person will take a year to write a novel >> when >> he can easily buy one for a few dollars. -- Fred Allen >> --------------------------------------------------------------------- >> Luke Kanies |http://reductivelabs.com|http://madstop.com > > -- The Number 1 Sign You Have Nothing to Do at Work... The 4th Division of Paperclips has overrun the Pushpin Infantry and General White-Out has called for a new skirmish. --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
