summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--lib/puppet/file_serving/configuration.rb6
-rw-r--r--lib/puppet/indirector/direct_file_server.rb2
-rw-r--r--lib/puppet/indirector/file_content/rest.rb1
-rw-r--r--lib/puppet/indirector/file_metadata/rest.rb1
-rw-r--r--lib/puppet/indirector/request.rb9
-rw-r--r--lib/puppet/indirector/rest.rb6
-rw-r--r--lib/puppet/network/http/handler.rb3
-rw-r--r--lib/puppet/util/uri_helper.rb22
-rwxr-xr-xspec/unit/file_serving/configuration.rb12
-rwxr-xr-xspec/unit/indirector/request.rb14
-rwxr-xr-xspec/unit/indirector/rest.rb13
-rwxr-xr-xspec/unit/network/http/handler.rb24
-rwxr-xr-xspec/unit/util/uri_helper.rb41
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