Please review pull request #757: Bug/2.7.x/10146 don't allow dash in variables opened by (daniel-pittman)
Description:
This resolves #10146, by enforcing the rule that - is an illegal character in variable names.
This will make it impossible to access a scoped variable inside a class with the (illegal, but not enforced) - character in the class name.
- Opened: Fri May 11 18:12:25 UTC 2012
- Based on: puppetlabs:2.7.x (7d3e09801ced1dc31276c6338de7af35f58774de)
- Requested merge: daniel-pittman:bug/2.7.x/10146-don't-allow-dash-in-variables (b26699a2b89f3724b4322a3723d2c700fb7e4dd3)
Diff follows:
diff --git a/lib/puppet/parser/lexer.rb b/lib/puppet/parser/lexer.rb
index cb1ff45..a27b94c 100644
--- a/lib/puppet/parser/lexer.rb
+++ b/lib/puppet/parser/lexer.rb
@@ -240,11 +240,11 @@ def (TOKENS[:DQCONT]).acceptable?(context={})
end
#:startdoc:
- TOKENS.add_token :DOLLAR_VAR, %r{\$(::)?([-\w]+::)*[-\w]+} do |lexer, value|
+ TOKENS.add_token :DOLLAR_VAR, %r{\$(::)?(\w+::)*\w+} do |lexer, value|
[TOKENS[:VARIABLE],value[1..-1]]
end
- TOKENS.add_token :VARIABLE, %r{(::)?([-\w]+::)*[-\w]+}
+ TOKENS.add_token :VARIABLE, %r{(::)?(\w+::)*\w+}
#:stopdoc: # Issue #4161
def (TOKENS[:VARIABLE]).acceptable?(context={})
[:DQPRE,:DQMID].include? context[:after]
diff --git a/spec/lib/puppet_spec/fixtures.rb b/spec/lib/puppet_spec/fixtures.rb
index 7f6bc2a..4623526 100755
--- a/spec/lib/puppet_spec/fixtures.rb
+++ b/spec/lib/puppet_spec/fixtures.rb
@@ -1,3 +1,5 @@
+require 'pathname'
+
module PuppetSpec::Fixtures
def fixtures(*rest)
File.join(PuppetSpec::FIXTURE_DIR, *rest)
@@ -11,18 +13,23 @@ def my_fixture_dir
fail "sorry, I couldn't work out your path from the caller stack!"
end
def my_fixture(name)
- file = File.join(my_fixture_dir, name)
+ file = (Pathname(my_fixture_dir) + name).relative_path_from(Pathname.getwd).to_s
+
unless File.readable? file then
fail Puppet::DevError, "fixture '#{name}' for #{my_fixture_dir} is not readable"
end
return file
end
def my_fixtures(glob = '*', flags = 0)
- files = Dir.glob(File.join(my_fixture_dir, glob), flags)
+ pwd = Pathname.getwd
+ files = Pathname.glob(Pathname(my_fixture_dir) + glob, flags).
+ map {|f| f.relative_path_from(pwd).to_s }
+
unless files.length > 0 then
fail Puppet::DevError, "fixture '#{glob}' for #{my_fixture_dir} had no files!"
end
- block_given? and files.each do |file| yield file end
+
+ block_given? and files.each {|file| yield file }
files
end
end
diff --git a/spec/unit/parser/lexer_spec.rb b/spec/unit/parser/lexer_spec.rb
index 48f7304..3dcae64 100755
--- a/spec/unit/parser/lexer_spec.rb
+++ b/spec/unit/parser/lexer_spec.rb
@@ -406,6 +406,44 @@
end
end
+shared_examples_for "variable names in the lexer" do |prefix|
+ # Watch out - a regex might match a *prefix* on these, not just the whole
+ # word, so make sure you don't have false positive or negative results based
+ # on that.
+ legal = %w{f foo f::b foo::b f::bar foo::bar 3 foo3 3foo}
+ illegal = %w{f- f-o -f f::-o f::o- f::o-o}
+
+ ["", "::"].each do |global_scope|
+ legal.each do |name|
+ var = prefix + global_scope + name
+ it "should accept #{var.inspect} as a valid variable name" do
+ (subject.regex.match(var) || [])[0].should == var
+ end
+ end
+
+ illegal.each do |name|
+ var = prefix + global_scope + name
+ it "should NOT accept #{var.inspect} as a valid variable name" do
+ (subject.regex.match(var) || [])[0].should_not == var
+ end
+ end
+ end
+end
+
+describe Puppet::Parser::Lexer::TOKENS[:DOLLAR_VAR] do
+ its(:skip_text) { should be_false }
+ its(:incr_line) { should be_false }
+
+ it_should_behave_like "variable names in the lexer", '$'
+end
+
+describe Puppet::Parser::Lexer::TOKENS[:VARIABLE] do
+ its(:skip_text) { should be_false }
+ its(:incr_line) { should be_false }
+
+ it_should_behave_like "variable names in the lexer", ''
+end
+
def tokens_scanned_from(s)
lexer = Puppet::Parser::Lexer.new
lexer.string = s
@@ -663,11 +701,16 @@ def tokens_scanned_from(s)
end
it "should correctly lex variables" do
- ["$variable", "$::variable", "$qualified::variable", "$further::qualified::variable", "$hyphenated-variable", "$-variable-with-leading-dash"].each do |string|
+ ["$variable", "$::variable", "$qualified::variable", "$further::qualified::variable"].each do |string|
tokens_scanned_from(string).should be_like([:VARIABLE,string.sub(/^\$/,'')])
end
end
+ it "should end variables at `-`" do
+ tokens_scanned_from('$hyphenated-variable').
+ should be_like [:VARIABLE, "hyphenated"], [:MINUS, '-'], [:NAME, 'variable']
+ end
+
it "should not include whitespace in a variable" do
tokens_scanned_from("$foo bar").should_not be_like([:VARIABLE, "foo bar"])
end
@@ -681,7 +724,7 @@ def tokens_scanned_from(s)
it "should correctly lex #{file}" do
lexer = Puppet::Parser::Lexer.new
lexer.file = file
- lambda { lexer.fullscan }.should_not raise_error
+ expect { lexer.fullscan }.should_not raise_error
end
end
end
-- 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.
