diff options
| -rw-r--r-- | lib/puppet/file_serving/configuration.rb | 6 | ||||
| -rw-r--r-- | lib/puppet/indirector/direct_file_server.rb | 2 | ||||
| -rw-r--r-- | lib/puppet/indirector/file_content/rest.rb | 1 | ||||
| -rw-r--r-- | lib/puppet/indirector/file_metadata/rest.rb | 1 | ||||
| -rw-r--r-- | lib/puppet/indirector/request.rb | 9 | ||||
| -rw-r--r-- | lib/puppet/indirector/rest.rb | 6 | ||||
| -rw-r--r-- | lib/puppet/network/http/handler.rb | 3 | ||||
| -rw-r--r-- | lib/puppet/util/uri_helper.rb | 22 | ||||
| -rwxr-xr-x | spec/unit/file_serving/configuration.rb | 12 | ||||
| -rwxr-xr-x | spec/unit/indirector/request.rb | 14 | ||||
| -rwxr-xr-x | spec/unit/indirector/rest.rb | 13 | ||||
| -rwxr-xr-x | spec/unit/network/http/handler.rb | 24 | ||||
| -rwxr-xr-x | spec/unit/util/uri_helper.rb | 41 |
13 files changed, 57 insertions, 97 deletions
diff --git a/lib/puppet/file_serving/configuration.rb b/lib/puppet/file_serving/configuration.rb index de5995d7e..608924c8b 100644 --- a/lib/puppet/file_serving/configuration.rb +++ b/lib/puppet/file_serving/configuration.rb @@ -9,10 +9,8 @@ require 'puppet/file_serving/mount/file' require 'puppet/file_serving/mount/modules' require 'puppet/file_serving/mount/plugins' require 'puppet/util/cacher' -require 'puppet/util/uri_helper' class Puppet::FileServing::Configuration - include Puppet::Util::URIHelper require 'puppet/file_serving/configuration/parser' class << self @@ -70,9 +68,7 @@ class Puppet::FileServing::Configuration # Reparse the configuration if necessary. readconfig - uri = key2uri(request.key) - - mount_name, path = uri.path.sub(/^\//, '').split(File::Separator, 2) + mount_name, path = request.key.split(File::Separator, 2) raise(ArgumentError, "Cannot find file: Invalid path '%s'" % mount_name) unless mount_name =~ %r{^[-\w]+$} diff --git a/lib/puppet/indirector/direct_file_server.rb b/lib/puppet/indirector/direct_file_server.rb index bcda92366..f69f9e14b 100644 --- a/lib/puppet/indirector/direct_file_server.rb +++ b/lib/puppet/indirector/direct_file_server.rb @@ -3,12 +3,10 @@ # Copyright (c) 2007. All rights reserved. require 'puppet/file_serving/terminus_helper' -require 'puppet/util/uri_helper' require 'puppet/indirector/terminus' class Puppet::Indirector::DirectFileServer < Puppet::Indirector::Terminus - include Puppet::Util::URIHelper include Puppet::FileServing::TerminusHelper def find(request) diff --git a/lib/puppet/indirector/file_content/rest.rb b/lib/puppet/indirector/file_content/rest.rb index 31df7626d..7b3cade8e 100644 --- a/lib/puppet/indirector/file_content/rest.rb +++ b/lib/puppet/indirector/file_content/rest.rb @@ -3,7 +3,6 @@ # Copyright (c) 2007. All rights reserved. require 'puppet/file_serving/content' -require 'puppet/util/uri_helper' require 'puppet/indirector/file_content' require 'puppet/indirector/rest' diff --git a/lib/puppet/indirector/file_metadata/rest.rb b/lib/puppet/indirector/file_metadata/rest.rb index 0f3d9c6fd..8cbf91049 100644 --- a/lib/puppet/indirector/file_metadata/rest.rb +++ b/lib/puppet/indirector/file_metadata/rest.rb @@ -3,7 +3,6 @@ # Copyright (c) 2007. All rights reserved. require 'puppet/file_serving/metadata' -require 'puppet/util/uri_helper' require 'puppet/indirector/file_metadata' require 'puppet/indirector/rest' diff --git a/lib/puppet/indirector/request.rb b/lib/puppet/indirector/request.rb index c6f268ab2..539cd0e62 100644 --- a/lib/puppet/indirector/request.rb +++ b/lib/puppet/indirector/request.rb @@ -1,3 +1,4 @@ +require 'uri' require 'puppet/indirector' # This class encapsulates all of the information you need to make an @@ -14,6 +15,10 @@ class Puppet::Indirector::Request ! ! authenticated end + def escaped_key + URI.escape(key) + end + # LAK:NOTE This is a messy interface to the cache, and it's only # used by the Configurer class. I decided it was better to implement # it now and refactor later, when we have a better design, than @@ -96,7 +101,7 @@ class Puppet::Indirector::Request # Just short-circuit these to full paths if uri.scheme == "file" - @key = uri.path + @key = URI.unescape(uri.path) return end @@ -111,6 +116,6 @@ class Puppet::Indirector::Request end @protocol = uri.scheme - @key = uri.path.sub(/^\//, '') + @key = URI.unescape(uri.path.sub(/^\//, '')) end end diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb index 2d0799286..ce459b905 100644 --- a/lib/puppet/indirector/rest.rb +++ b/lib/puppet/indirector/rest.rb @@ -62,12 +62,12 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus end def find(request) - deserialize network(request).get("/#{indirection.name}/#{request.key}#{query_string(request)}", headers) + deserialize network(request).get("/#{indirection.name}/#{request.escaped_key}#{query_string(request)}", headers) end def search(request) if request.key - path = "/#{indirection.name}s/#{request.key}#{query_string(request)}" + path = "/#{indirection.name}s/#{request.escaped_key}#{query_string(request)}" else path = "/#{indirection.name}s#{query_string(request)}" end @@ -79,7 +79,7 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus def destroy(request) raise ArgumentError, "DELETE does not accept options" unless request.options.empty? - deserialize network(request).delete("/#{indirection.name}/#{request.key}", headers) + deserialize network(request).delete("/#{indirection.name}/#{request.escaped_key}", headers) end def save(request) diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb index 9bc94a037..cce679864 100644 --- a/lib/puppet/network/http/handler.rb +++ b/lib/puppet/network/http/handler.rb @@ -75,6 +75,7 @@ module Puppet::Network::HTTP::Handler # Execute our find. def do_find(request, response) key = request_key(request) || raise(ArgumentError, "Could not locate lookup key in request path [#{path(request)}]") + key = URI.unescape(key) args = params(request) unless result = model.find(key, args) return do_exception(response, "Could not find %s %s" % [model.name, key], 404) @@ -93,6 +94,7 @@ module Puppet::Network::HTTP::Handler def do_search(request, response) args = params(request) if key = request_key(request) + key = URI.unescape(key) result = model.search(key, args) else result = model.search(args) @@ -110,6 +112,7 @@ module Puppet::Network::HTTP::Handler # Execute our destroy. def do_destroy(request, response) key = request_key(request) || raise(ArgumentError, "Could not locate lookup key in request path [#{path(request)}]") + key = URI.unescape(key) args = params(request) result = model.destroy(key, args) diff --git a/lib/puppet/util/uri_helper.rb b/lib/puppet/util/uri_helper.rb deleted file mode 100644 index cb9320387..000000000 --- a/lib/puppet/util/uri_helper.rb +++ /dev/null @@ -1,22 +0,0 @@ -# -# Created by Luke Kanies on 2007-10-16. -# Copyright (c) 2007. All rights reserved. - -require 'uri' -require 'puppet/util' - -# Helper methods for dealing with URIs. -module Puppet::Util::URIHelper - def key2uri(key) - # Return it directly if it's fully qualified. - if key =~ /^#{::File::SEPARATOR}/ - key = "file://" + key - end - - begin - uri = URI.parse(URI.escape(key)) - rescue => detail - raise ArgumentError, "Could not understand URI %s: %s" % [key, detail.to_s] - end - end -end diff --git a/spec/unit/file_serving/configuration.rb b/spec/unit/file_serving/configuration.rb index c2d2e5014..e62423caf 100755 --- a/spec/unit/file_serving/configuration.rb +++ b/spec/unit/file_serving/configuration.rb @@ -134,7 +134,7 @@ describe Puppet::FileServing::Configuration do @config = Puppet::FileServing::Configuration.create @config.stubs(:find_mount) - @request = stub 'request', :key => "puppet:///foo/bar/baz", :options => {} + @request = stub 'request', :key => "foo/bar/baz", :options => {} end it "should reread the configuration" do @@ -150,13 +150,13 @@ describe Puppet::FileServing::Configuration do end it "should fail if the mount name is not alpha-numeric" do - @request.expects(:key).returns "puppet:///foo&bar/asdf" + @request.expects(:key).returns "foo&bar/asdf" lambda { @config.split_path(@request) }.should raise_error(ArgumentError) end it "should support dashes in the mount name" do - @request.expects(:key).returns "puppet:///foo-bar/asdf" + @request.expects(:key).returns "foo-bar/asdf" lambda { @config.split_path(@request) }.should_not raise_error(ArgumentError) end @@ -182,7 +182,7 @@ describe Puppet::FileServing::Configuration do end it "should remove any double slashes" do - @request.stubs(:key).returns "puppet:///foo/bar//baz" + @request.stubs(:key).returns "foo/bar//baz" mount = stub 'mount', :name => "foo" @config.expects(:find_mount).returns mount @@ -190,7 +190,7 @@ describe Puppet::FileServing::Configuration do end it "should return the relative path as nil if it is an empty string" do - @request.expects(:key).returns "puppet:///foo" + @request.expects(:key).returns "foo" mount = stub 'mount', :name => "foo" @config.expects(:find_mount).returns mount @@ -198,7 +198,7 @@ describe Puppet::FileServing::Configuration do end it "should add 'modules/' to the relative path if the modules mount is used but not specified, for backward compatibility" do - @request.expects(:key).returns "puppet:///foo/bar" + @request.expects(:key).returns "foo/bar" mount = stub 'mount', :name => "modules" @config.expects(:find_mount).returns mount diff --git a/spec/unit/indirector/request.rb b/spec/unit/indirector/request.rb index dc1e4e39d..fc3ed44e6 100755 --- a/spec/unit/indirector/request.rb +++ b/spec/unit/indirector/request.rb @@ -79,10 +79,12 @@ describe Puppet::Indirector::Request do describe "and the request key is a URI" do describe "and the URI is a 'file' URI" do before do - @request = Puppet::Indirector::Request.new(:ind, :method, "file:///my/file") + @request = Puppet::Indirector::Request.new(:ind, :method, "file:///my/file with spaces") end - it "should set the request key to the full file path" do @request.key.should == "/my/file" end + it "should set the request key to the unescaped full file path" do + @request.key.should == "/my/file with spaces" + end it "should not set the protocol" do @request.protocol.should be_nil @@ -118,8 +120,8 @@ describe Puppet::Indirector::Request do Puppet::Indirector::Request.new(:ind, :method, "http://host/stuff").port.should == 80 end - it "should set the request key to the unqualified path from the URI" do - Puppet::Indirector::Request.new(:ind, :method, "http:///stuff").key.should == "stuff" + it "should set the request key to the unescaped unqualified path from the URI" do + Puppet::Indirector::Request.new(:ind, :method, "http:///stuff with spaces").key.should == "stuff with spaces" end it "should set the :uri attribute to the full URI" do @@ -171,4 +173,8 @@ describe Puppet::Indirector::Request do it "should use its indirection name and key, if it has no uri, as its string representation" do Puppet::Indirector::Request.new(:myind, :find, "key") == "/myind/key" end + + it "should be able to return the URI-escaped key" do + Puppet::Indirector::Request.new(:myind, :find, "my key").escaped_key.should == URI.escape("my key") + end end diff --git a/spec/unit/indirector/rest.rb b/spec/unit/indirector/rest.rb index aa6d51b6a..4fa30b20a 100755 --- a/spec/unit/indirector/rest.rb +++ b/spec/unit/indirector/rest.rb @@ -211,7 +211,8 @@ describe Puppet::Indirector::REST do @connection = stub('mock http connection', :get => @response) @searcher.stubs(:network).returns(@connection) # neuter the network connection - @request = stub 'request', :key => 'foo', :options => {} + # Use a key with spaces, so we can test escaping + @request = stub 'request', :escaped_key => 'foo', :options => {} end it "should call the GET http method on a network connection" do @@ -227,7 +228,7 @@ describe Puppet::Indirector::REST do @searcher.find(@request).should == 'myobject' end - it "should use the indirection name and request key to create the path" do + it "should use the indirection name and escaped request key to create the path" do should_path = "/%s/%s" % [@indirection.name.to_s, "foo"] @connection.expects(:get).with { |path, args| path == should_path }.returns(@response) @searcher.find(@request) @@ -258,7 +259,7 @@ describe Puppet::Indirector::REST do @model.stubs(:convert_from_multiple) - @request = stub 'request', :key => 'foo', :options => {} + @request = stub 'request', :escaped_key => 'foo', :options => {}, :key => "bar" end it "should call the GET http method on a network connection" do @@ -281,7 +282,7 @@ describe Puppet::Indirector::REST do @searcher.search(@request) end - it "should use the plural indirection name and request key to create the path if the request key is set" do + it "should use the plural indirection name and escaped request key to create the path if the request key is set" do should_path = "/%ss/%s" % [@indirection.name.to_s, "foo"] @connection.expects(:get).with { |path, args| path == should_path }.returns(@response) @searcher.search(@request) @@ -319,7 +320,7 @@ describe Puppet::Indirector::REST do @connection = stub('mock http connection', :delete => @response) @searcher.stubs(:network).returns(@connection) # neuter the network connection - @request = stub 'request', :key => 'foo', :options => {} + @request = stub 'request', :escaped_key => 'foo', :options => {} end it "should call the DELETE http method on a network connection" do @@ -341,7 +342,7 @@ describe Puppet::Indirector::REST do @searcher.destroy(@request).should == 'myobject' end - it "should use the indirection name and request key to create the path" do + it "should use the indirection name and escaped request key to create the path" do should_path = "/%s/%s" % [@indirection.name.to_s, "foo"] @connection.expects(:delete).with { |path, args| path == should_path }.returns(@response) @searcher.destroy(@request) diff --git a/spec/unit/network/http/handler.rb b/spec/unit/network/http/handler.rb index ed0f25121..6f1a57b8b 100755 --- a/spec/unit/network/http/handler.rb +++ b/spec/unit/network/http/handler.rb @@ -229,12 +229,20 @@ describe Puppet::Network::HTTP::Handler do Puppet::Network::FormatHandler.stubs(:format).returns @format end - it "should fail to find model if key is not specified" do + it "should fail if the key is not specified" do @handler.stubs(:request_key).returns(nil) lambda { @handler.do_find(@request, @response) }.should raise_error(ArgumentError) end + it "should use the escaped request key" do + @handler.stubs(:request_key).returns URI.escape("my key") + @model_class.expects(:find).with do |key, args| + key == "my key" + end.returns @result + @handler.do_find(@request, @response) + end + it "should use a common method for determining the request parameters" do @handler.stubs(:params).returns(:foo => :baz, :bar => :xyzzy) @model_class.expects(:find).with do |key, args| @@ -336,9 +344,9 @@ describe Puppet::Network::HTTP::Handler do @handler.do_search(@request, @response) end - it "should use a request key if one is provided" do - @handler.expects(:request_key).with(@request).returns "foo" - @model_class.expects(:search).with { |key, args| key == "foo" }.returns @result + it "should use an escaped request key if one is provided" do + @handler.expects(:request_key).with(@request).returns URI.escape("foo bar") + @model_class.expects(:search).with { |key, args| key == "foo bar" }.returns @result @handler.do_search(@request, @response) end @@ -402,6 +410,14 @@ describe Puppet::Network::HTTP::Handler do lambda { @handler.do_destroy(@request, @response) }.should raise_error(ArgumentError) end + it "should use the escaped request key to destroy the instance in the model" do + @handler.expects(:request_key).returns URI.escape("foo bar") + @model_class.expects(:destroy).with do |key, args| + key == "foo bar" + end + @handler.do_destroy(@request, @response) + end + it "should use a common method for determining the request parameters" do @handler.stubs(:params).returns(:foo => :baz, :bar => :xyzzy) @model_class.expects(:destroy).with do |key, args| diff --git a/spec/unit/util/uri_helper.rb b/spec/unit/util/uri_helper.rb deleted file mode 100755 index f454a2ced..000000000 --- a/spec/unit/util/uri_helper.rb +++ /dev/null @@ -1,41 +0,0 @@ -#!/usr/bin/env ruby -# -# Created by Luke Kanies on 2007-10-18. -# Copyright (c) 2007. All rights reserved. - -require File.dirname(__FILE__) + '/../../spec_helper' - -require 'puppet/util/uri_helper' - -describe Puppet::Util::URIHelper, " when converting a key to a URI" do - before do - @helper = Object.new - @helper.extend(Puppet::Util::URIHelper) - end - - it "should return the URI instance" do - URI.expects(:parse).with("file:///myhost/blah").returns(:myuri) - @helper.key2uri("/myhost/blah").should == :myuri - end - - it "should escape the key before parsing" do - URI.expects(:escape).with("mykey").returns("http://myhost/blah") - URI.expects(:parse).with("http://myhost/blah").returns(:myuri) - @helper.key2uri("mykey").should == :myuri - end - - it "should use the URI class to parse the key" do - URI.expects(:parse).with("http://myhost/blah").returns(:myuri) - @helper.key2uri("http://myhost/blah").should == :myuri - end - - it "should set the scheme to 'file' if the key is a fully qualified path" do - URI.expects(:parse).with("file:///myhost/blah").returns(:myuri) - @helper.key2uri("/myhost/blah").should == :myuri - end - - it "should set the host to 'nil' if the key is a fully qualified path" do - URI.expects(:parse).with("file:///myhost/blah").returns(:myuri) - @helper.key2uri("/myhost/blah").should == :myuri - end -end |
