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.
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 > >>> + �[email protected](: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 --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
