diff --git a/examples/lang/contains0.mcl b/examples/lang/contains0.mcl index 5f4fd61c..5f78079f 100644 --- a/examples/lang/contains0.mcl +++ b/examples/lang/contains0.mcl @@ -21,7 +21,8 @@ $x = if sys.hostname() in ["h1", "h3",] { fmt.printf("i (%s) was not chosen :(\n", sys.hostname()) } -file "/tmp/mgmt/hello-${sys.hostname()}" { +$host = sys.hostname() +file "/tmp/mgmt/hello-${host}" { state => $const.res.file.state.exists, content => $x, } diff --git a/examples/lang/escaping1.mcl b/examples/lang/escaping1.mcl new file mode 100644 index 00000000..560a1f70 --- /dev/null +++ b/examples/lang/escaping1.mcl @@ -0,0 +1,30 @@ +print "escaping1" { + msg => "\${hello}", + + Meta:autogroup => false, +} + +$hello = "hi" +print "escaping2" { + msg => "${hello}", + + Meta:autogroup => false, +} + +print "escaping3" { + msg => "\\${hello}", + + Meta:autogroup => false, +} + +print "escaping4" { + msg => "\\ ${hello}", + + Meta:autogroup => false, +} + +print "escaping5" { + msg => "\"${hello}", + + Meta:autogroup => false, +} diff --git a/examples/lang/exchange0.mcl b/examples/lang/exchange0.mcl index 1f0af4ab..9ce8e60c 100644 --- a/examples/lang/exchange0.mcl +++ b/examples/lang/exchange0.mcl @@ -12,7 +12,8 @@ import "world" $rand = random1(8) $exchanged = world.exchange("keyns", $rand) -file "/tmp/mgmt/exchange-${sys.hostname()}" { +$host = sys.hostname() +file "/tmp/mgmt/exchange-${host}" { state => $const.res.file.state.exists, content => template("Found: {{ . }}\n", $exchanged), } diff --git a/examples/lang/hostname0.mcl b/examples/lang/hostname0.mcl index 9ebb4de5..a328376f 100644 --- a/examples/lang/hostname0.mcl +++ b/examples/lang/hostname0.mcl @@ -1,6 +1,7 @@ import "sys" -file "/tmp/mgmt/${sys.hostname()}" { - content => "hello from ${sys.hostname()}!\n", +$host = sys.hostname() +file "/tmp/mgmt/${host}" { + content => "hello from ${host}!\n", state => $const.res.file.state.exists, } diff --git a/examples/lang/os.mcl b/examples/lang/os.mcl index 890f6e43..9c0a11c9 100644 --- a/examples/lang/os.mcl +++ b/examples/lang/os.mcl @@ -1,5 +1,5 @@ import "golang/os" -import "golang/exec" +import "golang/os/exec" import "fmt" $tmpdir = os.temp_dir() @@ -9,9 +9,10 @@ file "${tmpdir}/execinfo" { content => fmt.printf("mgmt is at %s\n", os.executable()), } +$home = os.getenv("HOME") file "${tmpdir}/mgmtenv" { state => $const.res.file.state.exists, - content => os.expand_env("$HOME sweet ${os.getenv(\"HOME\")}\n"), + content => os.expand_env("$HOME sweet ${home}\n"), } file "${tmpdir}/mgmtos" { @@ -19,9 +20,10 @@ file "${tmpdir}/mgmtos" { content => os.readlink("/bin"), } +$cache_dir = os.user_cache_dir() +$home_dir = os.user_home_dir() $rm = exec.look_path("rm") - file "${tmpdir}/cache" { state => $const.res.file.state.exists, - content => "Plz cache in ${os.user_cache_dir()}.\nYour home is ${os.user_home_dir()}. Remove with ${rm}\n", + content => "Plz cache in ${cache_dir}.\nYour home is ${home_dir}. Remove with ${rm}\n", } diff --git a/examples/lang/schedule0.mcl b/examples/lang/schedule0.mcl index 3783cad6..6bd64e37 100644 --- a/examples/lang/schedule0.mcl +++ b/examples/lang/schedule0.mcl @@ -16,7 +16,8 @@ $set = world.schedule("xsched", $opts) # and if you want, you can omit the options entirely: #$set = world.schedule("xsched") -file "/tmp/mgmt/scheduled-${sys.hostname()}" { +$host = sys.hostname() +file "/tmp/mgmt/scheduled-${host}" { state => $const.res.file.state.exists, content => template("set: {{ . }}\n", $set), } diff --git a/integration/basic_test.go b/integration/basic_test.go index c29c2b3f..30e8cf98 100644 --- a/integration/basic_test.go +++ b/integration/basic_test.go @@ -167,9 +167,10 @@ func TestCluster1(t *testing.T) { code := util.Code(` import "sys" $root = sys.getenv("MGMT_TEST_ROOT") + $host = sys.hostname() file "${root}/mgmt-hostname" { - content => "i am ${sys.hostname()}\n", + content => "i am ${host}\n", state => $const.res.file.state.exists, } `) @@ -192,9 +193,10 @@ func TestCluster1(t *testing.T) { code := util.Code(` import "sys" $root = sys.getenv("MGMT_TEST_ROOT") + $host = sys.hostname() file "${root}/mgmt-hostname" { - content => "i am ${sys.hostname()}\n", + content => "i am ${host}\n", state => $const.res.file.state.exists, } `) diff --git a/lang/Makefile b/lang/Makefile index 06a10364..1bf5e42c 100644 --- a/lang/Makefile +++ b/lang/Makefile @@ -22,13 +22,13 @@ OLDGOYACC := $(shell go version | grep -E 'go1.6|go1.7') all: build -build: lexer.nn.go y.go +build: lexer.nn.go y.go interpolate/parse.generated.go @# recursively run make in child dir named types @$(MAKE) --quiet -C types clean: $(MAKE) --quiet -C types clean - @rm -f lexer.nn.go y.go y.output || true + @rm -f lexer.nn.go y.go y.output interpolate/parse.generated.go || true lexer.nn.go: lexer.nex @echo "Generating: lexer..." @@ -45,5 +45,15 @@ else endif @ROOT="$$( cd "$$( dirname "$${BASH_SOURCE[0]}" )" && cd .. && pwd )" && $$ROOT/misc/header.sh 'y.go' +interpolate/parse.generated.go: interpolate/parse.rl + @echo "Generating: interpolation..." + ragel -Z -G2 -o interpolate/parse.generated.go interpolate/parse.rl + #@ROOT="$$( cd "$$( dirname "$${BASH_SOURCE[0]}" )" && cd .. && pwd )" && $$ROOT/misc/header.sh 'interpolate/parse.generated.go' + # XXX: I have no idea why I need to sed twice. I give up :P + # remove the ragel header so our header test passes + @sed -i -e "1d" 'interpolate/parse.generated.go' + @sed -i -e "1d" 'interpolate/parse.generated.go' + gofmt -s -w 'interpolate/parse.generated.go' + fuzz: @$(MAKE) --quiet -C fuzz diff --git a/lang/interpolate.go b/lang/interpolate.go index 4630021a..074fab83 100644 --- a/lang/interpolate.go +++ b/lang/interpolate.go @@ -21,12 +21,20 @@ import ( "fmt" "github.com/purpleidea/mgmt/lang/interfaces" + "github.com/purpleidea/mgmt/lang/interpolate" "github.com/purpleidea/mgmt/util/errwrap" "github.com/hashicorp/hil" hilast "github.com/hashicorp/hil/ast" ) +const ( + // UseHilInterpolation specifies that we use the legacy Hil interpolate. + // This can't properly escape a $ in the standard way. It's here in case + // someone wants to play with it and examine how the AST stuff worked... + UseHilInterpolation = false +) + // Pos represents a position in the code. // TODO: consider expanding with range characteristics. type Pos struct { @@ -35,12 +43,72 @@ type Pos struct { Filename string // optional source filename, if known } -// InterpolateStr interpolates a string and returns the representative AST. This -// particular implementation uses the hashicorp hil library and syntax to do so. +// InterpolateStr interpolates a string and returns the representative AST. func InterpolateStr(str string, pos *Pos, data *interfaces.Data) (interfaces.Expr, error) { if data.Debug { data.Logf("interpolating: %s", str) } + + if UseHilInterpolation { + return InterpolateHil(str, pos, data) + } + return InterpolateRagel(str, pos, data) +} + +// InterpolateRagel interpolates a string and returns the representative AST. It +// uses the ragel parser to perform the string interpolation. +func InterpolateRagel(str string, pos *Pos, data *interfaces.Data) (interfaces.Expr, error) { + sequence, err := interpolate.Parse(str) + if err != nil { + return nil, errwrap.Wrapf(err, "parser failed") + } + + exprs := []interfaces.Expr{} + for _, term := range sequence { + + switch t := term.(type) { + case interpolate.Literal: + expr := &ExprStr{ + V: t.Value, + } + exprs = append(exprs, expr) + + case interpolate.Variable: + expr := &ExprVar{ + Name: t.Name, + } + exprs = append(exprs, expr) + default: + return nil, fmt.Errorf("unknown term (%T): %+v", t, t) + } + } + + // If we didn't find anything of value, we got an empty string... + if len(sequence) == 0 && str == "" { // be doubly sure... + expr := &ExprStr{ + V: "", + } + exprs = append(exprs, expr) + } + + // The parser produces non-optimal results where two strings are next to + // each other, when they could be statically combined together. + simplified, err := simplifyExprList(exprs) + if err != nil { + return nil, errwrap.Wrapf(err, "expr list simplify failed") + } + + result, err := concatExprListIntoCall(simplified) + if err != nil { + return nil, errwrap.Wrapf(err, "concat expr list failed") + } + + return result, errwrap.Wrapf(result.Init(data), "init failed") +} + +// InterpolateHil interpolates a string and returns the representative AST. This +// particular implementation uses the hashicorp hil library and syntax to do so. +func InterpolateHil(str string, pos *Pos, data *interfaces.Data) (interfaces.Expr, error) { var line, column int = -1, -1 var filename string if pos != nil { @@ -246,6 +314,7 @@ func concatExprListIntoCall(exprs []interfaces.Expr) (interfaces.Expr, error) { } return &ExprCall{ + // NOTE: if we don't set the data field we need Init() called on it! Name: operatorFuncName, // concatenate the two strings with + operator Args: []interfaces.Expr{ operator, // operator first @@ -254,3 +323,42 @@ func concatExprListIntoCall(exprs []interfaces.Expr) (interfaces.Expr, error) { }, }, nil } + +// simplifyExprList takes a list of *ExprStr and *ExprVar and groups the +// sequential *ExprStr's together. If you pass it a list of Expr's that contains +// a different type of Expr, then this will error. +func simplifyExprList(exprs []interfaces.Expr) ([]interfaces.Expr, error) { + last := false + result := []interfaces.Expr{} + + for _, x := range exprs { + switch v := x.(type) { + case *ExprStr: + if !last { + last = true + result = append(result, x) + continue + } + + // combine! + expr := result[len(result)-1] // there has to be at least one + str, ok := expr.(*ExprStr) + if !ok { + // programming error + return nil, fmt.Errorf("unexpected type (%T)", expr) + } + str.V += v.V // combine! + //last = true // redundant, it's already true + // ... and don't append, we've combined! + + case *ExprVar: + last = false // the next one can't combine with me + result = append(result, x) + + default: + return nil, fmt.Errorf("unsupported type (%T)", x) + } + } + + return result, nil +} diff --git a/lang/interpolate/.gitignore b/lang/interpolate/.gitignore new file mode 100644 index 00000000..5a80a3ad --- /dev/null +++ b/lang/interpolate/.gitignore @@ -0,0 +1 @@ +parse.generated.go diff --git a/lang/interpolate/parse.rl b/lang/interpolate/parse.rl new file mode 100644 index 00000000..15a36edc --- /dev/null +++ b/lang/interpolate/parse.rl @@ -0,0 +1,151 @@ +// Mgmt +// Copyright (C) 2013-2021+ James Shubin and the project contributors +// Written by James Shubin and the project contributors +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package interpolate + +import ( + "fmt" +) + +%%{ + machine interpolate; + write data; +}%% + +// Parse performs string interpolation on the input. It returns the list of +// tokens found. It looks for variables of the format ${foo}. The curly braces +// are required. +func Parse(data string) (out Stream, _ error) { + var ( + // variables used by Ragel + cs = 0 // current state + p = 0 // current position in data + pe = len(data) + eof = pe // eof == pe if this is the last data block + + // Index in data where the currently captured string started. + idx int + + x string // The string we use for holding a temporary value. + l Literal // The string literal being read, if any. + v Variable // The variable being read, if any. + + // Current token. This is either the variable that we just read + // or the string literal. We will append it to `out` and move + // on. + t Token + ) + + %%{ + # Record the current position as the start of a string. This is + # usually used with the entry transition (>) to start capturing + # the string when a state machine is entered. + # + # fpc is the current position in the string (basically the same + # as the variable `p` but a special Ragel keyword) so after + # executing `start`, data[idx:fpc+1] is the string from when + # start was called to the current position (inclusive). + action start { idx = fpc } + + # A variable always starts with an lowercase alphabetical char + # and contains lowercase alphanumeric characters or numbers, + # underscores, and non-consecutive dots. The last char must not + # be an underscore or a dot. + # XXX: check that we don't get consecutive underscores or dots! + var_name = ( [a-z] ([a-z0-9_] | ('.' | '_') [a-z0-9_])* ) + >start + @{ + v.Name = data[idx:fpc+1] + }; + + # var is a reference to a variable. + var = '${' var_name '}' ; + + # Any special escape characters are matched here. + escaped_lit = '\\' ( 'a' | 'b' | 'f' | 'n' | 'r' | 't' | 'v' | '\\' | '"' | '$' ) + @{ + switch s := data[fpc:fpc+1]; s { + case "a": + x = "\a" + case "b": + x = "\b" + //case "e": + // x = "\e" // non-standard + case "f": + x = "\f" + case "n": + x = "\n" + case "r": + x = "\r" + case "t": + x = "\t" + case "v": + x = "\v" + case "\\": + x = "\\" + case "\"": + x = "\"" + case "$": + x = "$" + //case "0": + // x = "\x00" + default: + //x = s // in case we want to avoid erroring + // this is a programming (parser) error I think + return nil, fmt.Errorf("unhandled escape sequence token: %s", s) + } + l = Literal{Value: x} + }; + + # XXX: explicitly try and add this one? + #escape_lit = '\\\\' + #@{ + # l = Literal{Value: "\\\\"} + #}; + + # Anything followed by a '$' that is not a '{' is used as-is + # with the dollar. + dollar_lit = '$' (any - '{') + @{ + l = Literal{Value: data[fpc-1:fpc+1]} + }; + + # Literal strings that don't contain '$' or '\'. + simple_lit = (any - '$' - '\\')+ + >start + @{ + l = Literal{Value: data[idx:fpc + 1]} + }; + + lit = escaped_lit | dollar_lit | simple_lit; + + # Tokens are the two possible components in a string. Either a + # literal or a variable reference. + token = (var @{ t = v }) | (lit @{ t = l }); + + main := (token %{ out = append(out, t) })**; + + write init; + write exec; + }%% + + if cs < %%{ write first_final; }%% { + return nil, fmt.Errorf("cannot parse string: %s", data) + } + + return out, nil +} diff --git a/lang/interpolate/types.go b/lang/interpolate/types.go new file mode 100644 index 00000000..106df4b7 --- /dev/null +++ b/lang/interpolate/types.go @@ -0,0 +1,46 @@ +// Mgmt +// Copyright (C) 2013-2021+ James Shubin and the project contributors +// Written by James Shubin and the project contributors +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package interpolate + +// Stream is the list of tokens that are produced after interpolating a string. +// It is created by using the generated Parse function. +// TODO: In theory a more advanced parser could produce an AST here instead. +type Stream []Token + +// Token is the interface that every token must implement. +type Token interface { + token() +} + +// Literal is a string literal that we have found after interpolation parsing. +type Literal struct { + Value string +} + +// token ties the Literal to the Token interface. +func (Literal) token() {} + +// Variable is a variable name that we have found after interpolation parsing. +type Variable struct { + Name string +} + +// token ties the Variable to the Token interface. +func (Variable) token() {} + +// TODO: do we want to allow inline-function calls in a string? diff --git a/lang/interpolate_test.go b/lang/interpolate_test.go index 32f7c5b1..d5971023 100644 --- a/lang/interpolate_test.go +++ b/lang/interpolate_test.go @@ -30,6 +30,7 @@ import ( "github.com/davecgh/go-spew/spew" "github.com/kylelemons/godebug/pretty" + "github.com/sanity-io/litter" ) func TestInterpolate0(t *testing.T) { @@ -127,6 +128,66 @@ func TestInterpolate0(t *testing.T) { ast: ast, }) } + { + ast := &StmtProg{ + Prog: []interfaces.Stmt{ + &StmtRes{ + Kind: "test", + Name: &ExprStr{ + V: "t1", + }, + Contents: []StmtResContents{ + &StmtResField{ + Field: "stringptr", + Value: &ExprStr{ + V: "${hello}", + }, + }, + }, + }, + }, + } + testCases = append(testCases, test{ + name: "variable escaping 1", + code: ` + test "t1" { + stringptr => "\${hello}", + } + `, + fail: false, + ast: ast, + }) + } + { + ast := &StmtProg{ + Prog: []interfaces.Stmt{ + &StmtRes{ + Kind: "test", + Name: &ExprStr{ + V: "t1", + }, + Contents: []StmtResContents{ + &StmtResField{ + Field: "stringptr", + Value: &ExprStr{ + V: `\` + `$` + `{hello}`, + }, + }, + }, + }, + }, + } + testCases = append(testCases, test{ + name: "variable escaping 2", + code: ` + test "t1" { + stringptr => "` + `\\` + `\$` + "{hello}" + `", + } + `, + fail: false, + ast: ast, + }) + } names := []string{} for index, tc := range testCases { // run all the tests @@ -189,14 +250,28 @@ func TestInterpolate0(t *testing.T) { return } // double check because DeepEqual is different since the logf exists + lo := &litter.Options{ + //Compact: false, + StripPackageNames: true, + HidePrivateFields: true, + HideZeroValues: true, + //FieldExclusions: regexp.MustCompile(`^(data)$`), + //FieldFilter func(reflect.StructField, reflect.Value) bool + //HomePackage string + //Separator string + } + if lo.Sdump(iast) == lo.Sdump(exp) { // simple diff + return + } + diff := pretty.Compare(iast, exp) if diff == "" { // bonus return } t.Errorf("test #%d: AST did not match expected", index) // TODO: consider making our own recursive print function - t.Logf("test #%d: actual: \n%s", index, spew.Sdump(iast)) - t.Logf("test #%d: expected: \n%s", index, spew.Sdump(exp)) + t.Logf("test #%d: actual: \n%s", index, lo.Sdump(iast)) + t.Logf("test #%d: expected: \n%s", index, lo.Sdump(exp)) t.Logf("test #%d: diff:\n%s", index, diff) }) } @@ -341,7 +416,7 @@ func TestInterpolateBasicStmt(t *testing.T) { } resName := &ExprCall{ Name: operatorFuncName, - // incorrect sig for this function, but correct interpolation + // incorrect sig for this function, and now invalid interpolation Args: []interfaces.Expr{ &ExprStr{ V: "+", @@ -370,11 +445,12 @@ func TestInterpolateBasicStmt(t *testing.T) { }, }, } + _ = exp // historical testCases = append(testCases, test{ name: "expanded invalid resource name", ast: ast, - fail: false, - exp: exp, + fail: true, + //exp: exp, }) } @@ -524,7 +600,7 @@ func TestInterpolateBasicExpr(t *testing.T) { //} { ast := &ExprStr{ - V: "sweetie${3.14159}", // invalid but only at type check + V: "sweetie${3.14159}", // invalid } exp := &ExprCall{ Name: operatorFuncName, @@ -540,11 +616,11 @@ func TestInterpolateBasicExpr(t *testing.T) { }, }, } + _ = exp // historical testCases = append(testCases, test{ name: "float expansion", ast: ast, - fail: false, - exp: exp, + fail: true, }) } { @@ -566,11 +642,11 @@ func TestInterpolateBasicExpr(t *testing.T) { }, }, } + _ = exp // historical testCases = append(testCases, test{ name: "function expansion", ast: ast, - fail: false, - exp: exp, + fail: true, }) } { @@ -599,11 +675,11 @@ func TestInterpolateBasicExpr(t *testing.T) { }, }, } + _ = exp // historical testCases = append(testCases, test{ name: "function expansion arg", ast: ast, - fail: false, - exp: exp, + fail: true, }) } // FIXME: i am broken, i don't deal well with negatives for some reason diff --git a/lang/interpret_test.go b/lang/interpret_test.go index 8706261a..4795efb1 100644 --- a/lang/interpret_test.go +++ b/lang/interpret_test.go @@ -772,8 +772,7 @@ func TestAstFunc1(t *testing.T) { return } if fail1 && err != nil { - // TODO: %+v instead? - s := fmt.Sprintf("%s", err) // convert to string + s := err.Error() // convert to string if s != expstr { t.Errorf("test #%d: FAIL", index) t.Errorf("test #%d: expected different error", index) @@ -839,8 +838,7 @@ func TestAstFunc1(t *testing.T) { return } if fail2 && err != nil { - // TODO: %+v instead? - s := fmt.Sprintf("%s", err) // convert to string + s := err.Error() // convert to string if s != expstr { t.Errorf("test #%d: FAIL", index) t.Errorf("test #%d: expected different error", index) @@ -872,8 +870,7 @@ func TestAstFunc1(t *testing.T) { return } if fail3 && err != nil { - // TODO: %+v instead? - s := fmt.Sprintf("%s", err) // convert to string + s := err.Error() // convert to string if s != expstr { t.Errorf("test #%d: FAIL", index) t.Errorf("test #%d: expected different error", index) @@ -897,8 +894,7 @@ func TestAstFunc1(t *testing.T) { return } if fail4 && err != nil { // can't process graph if it's nil - // TODO: %+v instead? - s := fmt.Sprintf("%s", err) // convert to string + s := err.Error() // convert to string if s != expstr { t.Errorf("test #%d: FAIL", index) t.Errorf("test #%d: expected different error", index) @@ -977,6 +973,7 @@ func TestAstFunc2(t *testing.T) { const magicError = "# err: " const magicError1 = "err1: " const magicError9 = "err9: " // TODO: rename + const magicError8 = "err8: " // TODO: rename // TODO: move them all down by one const magicError2 = "err2: " const magicError3 = "err3: " @@ -1013,6 +1010,7 @@ func TestAstFunc2(t *testing.T) { type errs struct { fail1 bool fail9 bool // TODO: rename + fail8 bool // TODO: rename // TODO: move them all down by one fail2 bool fail3 bool @@ -1073,6 +1071,7 @@ func TestAstFunc2(t *testing.T) { errStr := "" fail1 := false fail9 := false // TODO: rename + fail8 := false // TODO: rename // TODO: move them all down by one fail2 := false fail3 := false @@ -1093,6 +1092,11 @@ func TestAstFunc2(t *testing.T) { str = errStr fail9 = true } + if strings.HasPrefix(str, magicError8) { // TODO: rename + errStr = strings.TrimPrefix(str, magicError8) + str = errStr + fail8 = true + } // TODO: move them all down by one if strings.HasPrefix(str, magicError2) { errStr = strings.TrimPrefix(str, magicError2) @@ -1130,6 +1134,7 @@ func TestAstFunc2(t *testing.T) { errs: errs{ fail1: fail1, fail9: fail9, // TODO: rename + fail8: fail8, // TODO: rename // TODO: move them all down by one fail2: fail2, fail3: fail3, @@ -1171,6 +1176,7 @@ func TestAstFunc2(t *testing.T) { src := dir + path // location of the test fail1 := errs.fail1 fail9 := errs.fail9 // TODO: rename + fail8 := errs.fail8 // TODO: rename // TODO: move them all down by one fail2 := errs.fail2 fail3 := errs.fail3 @@ -1252,8 +1258,7 @@ func TestAstFunc2(t *testing.T) { return } if fail1 && err != nil { - // TODO: %+v instead? - s := fmt.Sprintf("%s", err) // convert to string + s := err.Error() // convert to string if s != expstr { t.Errorf("test #%d: FAIL", index) t.Errorf("test #%d: expected different error", index) @@ -1305,8 +1310,7 @@ func TestAstFunc2(t *testing.T) { return } if fail9 && err != nil { - // TODO: %+v instead? - s := fmt.Sprintf("%s", err) // convert to string + s := err.Error() // convert to string if s != expstr { t.Errorf("test #%d: FAIL", index) t.Errorf("test #%d: expected different error", index) @@ -1322,9 +1326,24 @@ func TestAstFunc2(t *testing.T) { } iast, err := ast.Interpolate() - if err != nil { + if (!fail || !fail8) && err != nil { t.Errorf("test #%d: FAIL", index) - t.Errorf("test #%d: interpolate failed with: %+v", index, err) + t.Errorf("test #%d: Interpolate failed with: %+v", index, err) + return + } + if fail8 && err != nil { + s := err.Error() // convert to string + if s != expstr { + t.Errorf("test #%d: FAIL", index) + t.Errorf("test #%d: expected different error", index) + t.Logf("test #%d: err: %s", index, s) + t.Logf("test #%d: exp: %s", index, expstr) + } + return // fail happened during lex parse, don't run init/interpolate! + } + if fail8 && err == nil { + t.Errorf("test #%d: FAIL", index) + t.Errorf("test #%d: Interpolate passed, expected fail", index) return } @@ -1336,8 +1355,7 @@ func TestAstFunc2(t *testing.T) { return } if fail2 && err != nil { - // TODO: %+v instead? - s := fmt.Sprintf("%s", err) // convert to string + s := err.Error() // convert to string if s != expstr { t.Errorf("test #%d: FAIL", index) t.Errorf("test #%d: expected different error", index) @@ -1369,8 +1387,7 @@ func TestAstFunc2(t *testing.T) { return } if fail3 && err != nil { - // TODO: %+v instead? - s := fmt.Sprintf("%s", err) // convert to string + s := err.Error() // convert to string if s != expstr { t.Errorf("test #%d: FAIL", index) t.Errorf("test #%d: expected different error", index) @@ -1394,8 +1411,7 @@ func TestAstFunc2(t *testing.T) { return } if fail4 && err != nil { // can't process graph if it's nil - // TODO: %+v instead? - s := fmt.Sprintf("%s", err) // convert to string + s := err.Error() // convert to string if s != expstr { t.Errorf("test #%d: FAIL", index) t.Errorf("test #%d: expected different error", index) @@ -1501,8 +1517,7 @@ func TestAstFunc2(t *testing.T) { return } if fail5 && err != nil { // can't process graph if it's nil - // TODO: %+v instead? - s := fmt.Sprintf("%s", err) // convert to string + s := err.Error() // convert to string if s != expstr { t.Errorf("test #%d: FAIL", index) t.Errorf("test #%d: expected different error", index) @@ -1525,8 +1540,7 @@ func TestAstFunc2(t *testing.T) { return } if fail6 && err != nil { - // TODO: %+v instead? - s := fmt.Sprintf("%s", err) // convert to string + s := err.Error() // convert to string if s != expstr { t.Errorf("test #%d: FAIL", index) t.Errorf("test #%d: expected different error", index) diff --git a/lang/interpret_test/TestAstFunc2/escaping1.output b/lang/interpret_test/TestAstFunc2/escaping1.output new file mode 100644 index 00000000..53fca49c --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/escaping1.output @@ -0,0 +1,21 @@ +Vertex: test[A: ${test}] +Vertex: test[B: $] +Vertex: test[C: This is c1] +Vertex: test[D: \$] +Vertex: test[E: {}] +Vertex: test[F: hello] +Vertex: test[G: This is g1 EOF] +Vertex: test[H: {hhh} EOF] +Vertex: test[I: This is ii EOF] +Vertex: test[J: $ is a dollar sign] +Vertex: test[K: $ {zzz} EOF] +Vertex: test[L: $$This is l1 EOF] +Vertex: test[M: $ $$] +Vertex: test[N: hello " world] +Vertex: test[O: hello "" world] +Vertex: test[P: hello \ world] +Vertex: test[Q: hello \\ world] +Vertex: test[R: \This is r1 EOF] +Vertex: test[S: \$ EOF] +Vertex: test[T: newline +EOF] diff --git a/lang/interpret_test/TestAstFunc2/escaping1/main.mcl b/lang/interpret_test/TestAstFunc2/escaping1/main.mcl new file mode 100644 index 00000000..87ff5475 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/escaping1/main.mcl @@ -0,0 +1,68 @@ +# escaping examples + +test "A: \${test}" {} + +test "B: \$" {} + +$c1 = "This is c1" +test "C: ${c1}" {} + +test "D: \\\$" {} + +test "E: {}" {} + +test "F: hello" {} + +$g1 = "This is g1" +test "G: ${g1} EOF" {} + +test "H: {hhh} EOF" {} + +$i_i = "This is ii" +test "I: ${i_i} EOF" {} + +# is this okay? +test "J: $ is a dollar sign" {} + +test "K: $ {zzz} EOF" {} + +$l1 = "This is l1" +test "L: $$${l1} EOF" {} + +test "M: $ $$" {} + +test "N: hello \" world" {} + +test "O: hello \"\" world" {} + +test "P: hello \\ world" {} + +test "Q: hello \\\\ world" {} + +$r1 = "This is r1" +test "R: \\${r1} EOF" {} + +test "S: \\$ EOF" {} + +test "T: newline\nEOF" {} + +# XXX: possible bugs or misunderstood expectations: + +#test "W: \\$" {} +# got: +# exp: W: \$ + +#$x1 = "This is x1" +#test "X: $${x1} EOF" {} +# got: X: $${x1} EOF +# exp: X: $This is x1 EOF + +#$unused = "i am unused" +#$y1 = "{unused}" +#test "Y: $${y1} EOF" {} # check there isn't double parsing +# got: Y: $${y1} EOF +# exp: Y: ${unused} EOF + +#test "Z: $$$" {} +# got: +# exp: Z: $$$ EOF diff --git a/lang/interpret_test/TestAstFunc2/escaping2.output b/lang/interpret_test/TestAstFunc2/escaping2.output new file mode 100644 index 00000000..b39192c2 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/escaping2.output @@ -0,0 +1 @@ +# err: err8: parser failed: cannot parse string: X: ${foo()} diff --git a/lang/interpret_test/TestAstFunc2/escaping2/main.mcl b/lang/interpret_test/TestAstFunc2/escaping2/main.mcl new file mode 100644 index 00000000..96affb85 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/escaping2/main.mcl @@ -0,0 +1,2 @@ +# this is invalid (you can't have a function inside a var lookup) +test "X: ${foo()}" {} diff --git a/lang/interpret_test/TestAstFunc2/escaping3.output b/lang/interpret_test/TestAstFunc2/escaping3.output new file mode 100644 index 00000000..f8153001 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/escaping3.output @@ -0,0 +1 @@ +# err: err8: parser failed: cannot parse string: X: ${} diff --git a/lang/interpret_test/TestAstFunc2/escaping3/main.mcl b/lang/interpret_test/TestAstFunc2/escaping3/main.mcl new file mode 100644 index 00000000..f4088afd --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/escaping3/main.mcl @@ -0,0 +1,2 @@ +# this is invalid (you can't have an empty var lookup) +test "X: ${}" {} diff --git a/lang/interpret_test/TestAstFunc2/escaping4.output b/lang/interpret_test/TestAstFunc2/escaping4.output new file mode 100644 index 00000000..3e0057ce --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/escaping4.output @@ -0,0 +1 @@ +# err: err8: parser failed: cannot parse string: X: \z diff --git a/lang/interpret_test/TestAstFunc2/escaping4/main.mcl b/lang/interpret_test/TestAstFunc2/escaping4/main.mcl new file mode 100644 index 00000000..760de6c3 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/escaping4/main.mcl @@ -0,0 +1,2 @@ +# this is invalid (you can't escape a z, right?) +test "X: \z" {} diff --git a/lang/interpret_test/TestAstFunc2/escaping5.output b/lang/interpret_test/TestAstFunc2/escaping5.output new file mode 100644 index 00000000..150d0450 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/escaping5.output @@ -0,0 +1 @@ +# err: err8: parser failed: cannot parse string: X: there is no \j sequence diff --git a/lang/interpret_test/TestAstFunc2/escaping5/main.mcl b/lang/interpret_test/TestAstFunc2/escaping5/main.mcl new file mode 100644 index 00000000..a8f3afe7 --- /dev/null +++ b/lang/interpret_test/TestAstFunc2/escaping5/main.mcl @@ -0,0 +1,2 @@ +# this is invalid (there's no \j escape sequence) +test "X: there is no \j sequence" {} diff --git a/lang/lang_test.go b/lang/lang_test.go index 3f907e52..2cef61d3 100644 --- a/lang/lang_test.go +++ b/lang/lang_test.go @@ -224,7 +224,7 @@ func TestInterpret4(t *testing.T) { # comment 3 stringptr => "the actual field name is: StringPtr", # comment 4 int8ptr => 99, # comment 5 - comment => "☺\thello\u263a\nwo\"rld\\2\u263a", # must escape these + comment => "☺\thello\nwo\"rld\\2", # must escape these } ` graph, err := runInterpret(t, code) @@ -244,7 +244,7 @@ func TestInterpret4(t *testing.T) { x.StringPtr = &stringptr int8ptr := int8(99) x.Int8Ptr = &int8ptr - x.Comment = "☺\thello☺\nwo\"rld\\2\u263a" // must escape the escaped chars + x.Comment = "☺\thello\nwo\"rld\\2" // must escape the escaped chars expected := &pgraph.Graph{} expected.AddVertex(x) diff --git a/lang/lexer.nex b/lang/lexer.nex index aa790f98..b19d568d 100644 --- a/lang/lexer.nex +++ b/lang/lexer.nex @@ -242,26 +242,13 @@ yylex.pos(lval) // our pos s := yylex.Text() - x, err := strconv.Unquote(s) // expects quote wrapping! - if err == nil { - lval.str = x // it comes out all cleanly escaped - } else if err == strconv.ErrSyntax { - // this catches improper escape codes or lone \ - lp := yylex.cast() - lp.lexerErr = &LexParseErr{ - Err: ErrLexerStringBadEscaping, - Str: s, - Row: yylex.Line(), - Col: yylex.Column(), - } - return ERROR - } else if err != nil { + if s[0:1] != "\"" || s[len(s)-1:] != "\"" { // unhandled error - panic(fmt.Sprintf("error lexing STRING, got: %v", err)) - } else { // no chars to escape found - lval.str = s[1:len(s)-1] // remove the two quotes + panic(fmt.Sprintf("error lexing STRING, got: %s", s)) + //return ERROR // unreachable } + lval.str = s[1:len(s)-1] // remove the two quotes return STRING } /\-?[0-9]+/ diff --git a/lang/lexparse_test.go b/lang/lexparse_test.go index f0c0319e..14ff238d 100644 --- a/lang/lexparse_test.go +++ b/lang/lexparse_test.go @@ -94,6 +94,17 @@ func TestLexParse0(t *testing.T) { fail: true, }) } + { + testCases = append(testCases, test{ + name: "bad escaping 2", + code: ` + test "t1" { + str => "he\\ llo", # incorrect escaping + } + `, + fail: true, + }) + } { testCases = append(testCases, test{ name: "int overflow", diff --git a/lang/structs.go b/lang/structs.go index 36609780..37367542 100644 --- a/lang/structs.go +++ b/lang/structs.go @@ -8353,7 +8353,9 @@ func (obj *ExprVar) Apply(fn func(interfaces.Node) error) error { return fn(obj) // Init initializes this branch of the AST, and returns an error if it fails to // validate. -func (obj *ExprVar) Init(*interfaces.Data) error { return nil } +func (obj *ExprVar) Init(*interfaces.Data) error { + return langutil.ValidateVarName(obj.Name) +} // Interpolate returns a new node (aka a copy) once it has been expanded. This // generally increases the size of the AST when it is used. It calls Interpolate diff --git a/lang/util/util.go b/lang/util/util.go index 8bb4f379..2c899ce3 100644 --- a/lang/util/util.go +++ b/lang/util/util.go @@ -19,8 +19,11 @@ package util import ( "fmt" + "regexp" + "strings" "github.com/purpleidea/mgmt/lang/types" + "github.com/purpleidea/mgmt/util/errwrap" ) // HasDuplicateTypes returns an error if the list of types is not unique. @@ -89,3 +92,38 @@ func FnMatch(typ *types.Type, fns []*types.FuncValue) (int, error) { return 0, fmt.Errorf("unable to find a compatible function for type: %+v", typ) } + +// ValidateVarName returns an error if the string pattern does not match the +// format for a valid variable name. The leading dollar sign must not be passed +// in. +func ValidateVarName(name string) error { + if name == "" { + return fmt.Errorf("got empty var name") + } + + // A variable always starts with an lowercase alphabetical char and + // contains lowercase alphanumeric characters or numbers, underscores, + // and non-consecutive dots. The last char must not be an underscore or + // a dot. + // TODO: put the variable matching pattern in a const somewhere? + pattern := `^[a-z]([a-z0-9_]|(\.|_)[a-z0-9_])*$` + + matched, err := regexp.MatchString(pattern, name) + if err != nil { + return errwrap.Wrapf(err, "error matching regex") + } + if !matched { + return fmt.Errorf("invalid var name: `%s`", name) + } + + // Check that we don't get consecutive underscores or dots! + // TODO: build this into the above regexp and into the parse.rl file! + if strings.Contains(name, "..") { + return fmt.Errorf("var name contains multiple periods: `%s`", name) + } + if strings.Contains(name, "__") { + return fmt.Errorf("var name contains multiple underscores: `%s`", name) + } + + return nil +} diff --git a/lang/util/util_test.go b/lang/util/util_test.go new file mode 100644 index 00000000..04e55afb --- /dev/null +++ b/lang/util/util_test.go @@ -0,0 +1,82 @@ +// Mgmt +// Copyright (C) 2013-2021+ James Shubin and the project contributors +// Written by James Shubin and the project contributors +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +package util + +import ( + "fmt" + "sort" + "testing" +) + +func TestValidateVarName(t *testing.T) { + testCases := map[string]error{ + "": fmt.Errorf("got empty var name"), + "hello": nil, + "NOPE": fmt.Errorf("invalid var name: `NOPE`"), + "$foo": fmt.Errorf("invalid var name: `$foo`"), + ".": fmt.Errorf("invalid var name: `.`"), + "..": fmt.Errorf("invalid var name: `..`"), + "_": fmt.Errorf("invalid var name: `_`"), + "__": fmt.Errorf("invalid var name: `__`"), + "0": fmt.Errorf("invalid var name: `0`"), + "1": fmt.Errorf("invalid var name: `1`"), + "42": fmt.Errorf("invalid var name: `42`"), + "X": fmt.Errorf("invalid var name: `X`"), + "x": nil, + "x0": nil, + "x1": nil, + "x42": nil, + "x42.foo": nil, + "x42_foo": nil, + + // XXX: fix these test cases + //"x.": fmt.Errorf("invalid var name: x."), + //"x_": fmt.Errorf("invalid var name: x_"), + } + + keys := []string{} + for k := range testCases { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + e, ok := testCases[k] + if !ok { + // programming error + t.Errorf("missing test case: %s", k) + continue + } + + err := ValidateVarName(k) + if err == nil && e == nil { + continue + } + if err == nil && e != nil { + t.Errorf("key: %s did not error, expected: %s", k, e.Error()) + continue + } + if err != nil && e == nil { + t.Errorf("key: %s expected no error, got: %s", k, err.Error()) + continue + } + if err.Error() != e.Error() { + t.Errorf("key: %s did not have correct error, expected: %s", k, err.Error()) + continue + } + } +} diff --git a/misc/make-deps.sh b/misc/make-deps.sh index dbfab05f..bc31fff3 100755 --- a/misc/make-deps.sh +++ b/misc/make-deps.sh @@ -50,6 +50,7 @@ if [ -n "$YUM" ]; then $sudo_command $YUM install -y augeas-devel $sudo_command $YUM install -y ruby-devel rubygems $sudo_command $YUM install -y time + $sudo_command $YUM install -y ragel # dependencies for building packages with fpm $sudo_command $YUM install -y gcc make rpm-build libffi-devel bsdtar mkosi || true $sudo_command $YUM install -y graphviz || true # for debugging @@ -59,6 +60,7 @@ if [ -n "$APT" ]; then $sudo_command $APT install -y libaugeas-dev || true $sudo_command $APT install -y ruby ruby-dev || true $sudo_command $APT install -y libpcap0.8-dev || true + $sudo_command $APT install -y ragel || true # dependencies for building packages with fpm $sudo_command $APT install -y build-essential rpm bsdtar || true # `realpath` is a more universal alternative to `readlink -f` for absolute path resolution @@ -73,11 +75,11 @@ fi # Prevent linuxbrew installing redundant deps in CI if [ -n "$BREW" -a "$RUNNER_OS" != "Linux" ]; then # coreutils contains gtimeout, gstat, etc - $BREW install pkg-config libvirt augeas coreutils || true + $BREW install pkg-config libvirt augeas coreutils ragel || true fi if [ -n "$PACMAN" ]; then - $sudo_command $PACMAN -S --noconfirm --asdeps --needed libvirt augeas rubygems libpcap + $sudo_command $PACMAN -S --noconfirm --asdeps --needed libvirt augeas rubygems libpcap ragel fi fold_end "Install dependencies" @@ -104,6 +106,24 @@ if ! in_ci; then fi fi +if in_ci; then + # TODO: consider bumping to new package manager version + RAGEL_VERSION='6.10' # current stable version + RAGEL_TMP='/tmp/ragel/' + RAGEL_FILE="${RAGEL_TMP}ragel-${RAGEL_VERSION}.tar.gz" + RAGEL_DIR="${RAGEL_TMP}ragel-${RAGEL_VERSION}/" + mkdir -p "$RAGEL_TMP" + cd "$RAGEL_TMP" + wget "https://www.colm.net/files/ragel/ragel-${RAGEL_VERSION}.tar.gz" -O "$RAGEL_FILE" + tar -xvf "$RAGEL_FILE" + cd - + cd "$RAGEL_DIR" + ./configure --prefix=/usr/local --disable-manual + make + sudo make install + cd - +fi + # attempt to workaround old ubuntu if [ -n "$APT" -a "$goversion" -lt "$mingoversion" ]; then echo "install golang from a ppa." diff --git a/test/comment_parser.go b/test/comment_parser.go index 21627c43..80f67e30 100644 --- a/test/comment_parser.go +++ b/test/comment_parser.go @@ -24,6 +24,7 @@ import ( "go/token" "log" "os" + "regexp" "strings" "unicode" ) @@ -250,6 +251,9 @@ func IsNewStart(word string) bool { if word == "*" { // bullets return true } + if IsNumberBullet(word) { + return true + } return false } @@ -269,3 +273,12 @@ func IsSpecialLine(line string) bool { return false } + +// IsNumberBullet returns true if the word starts with a number bullet like 42). +func IsNumberBullet(word string) bool { + matched, err := regexp.MatchString(`[0-9]+\)*`, word) + if err != nil { + return false + } + return matched +} diff --git a/test/shell/exchange0.mcl b/test/shell/exchange0.mcl index 1f0af4ab..ded8a8b4 100644 --- a/test/shell/exchange0.mcl +++ b/test/shell/exchange0.mcl @@ -11,8 +11,9 @@ import "world" $rand = random1(8) $exchanged = world.exchange("keyns", $rand) +$host = sys.hostname() -file "/tmp/mgmt/exchange-${sys.hostname()}" { +file "/tmp/mgmt/exchange-${host}" { state => $const.res.file.state.exists, content => template("Found: {{ . }}\n", $exchanged), } diff --git a/test/test-gometalinter.sh b/test/test-gometalinter.sh index 9dea1ba6..3abd8faf 100755 --- a/test/test-gometalinter.sh +++ b/test/test-gometalinter.sh @@ -49,6 +49,7 @@ gml="$gml --exclude=lang/lexer.nn.go" gml="$gml --exclude=lang/y.go" gml="$gml --exclude=bindata/bindata.go" gml="$gml --exclude=lang/types/kind_stringer.go" +gml="$gml --exclude=lang/interpolate/parse.generated.go" gometalinter="$gml" # final echo "Using: $gometalinter" diff --git a/test/test-govet.sh b/test/test-govet.sh index 901648f6..10391906 100755 --- a/test/test-govet.sh +++ b/test/test-govet.sh @@ -113,6 +113,10 @@ function reflowed-comments() { return 0 fi + if [ "$1" = './lang/interpolate/parse.generated.go' ]; then + return 0 + fi + ./test/comment_parser "$1" } diff --git a/test/test-headerfmt.sh b/test/test-headerfmt.sh index 4abab17b..f7a99a2e 100755 --- a/test/test-headerfmt.sh +++ b/test/test-headerfmt.sh @@ -16,7 +16,7 @@ while IFS='' read -r line; do # find what header should look like done < "$FILE" find_files() { - git ls-files | grep '\.go$' | grep -v '^examples/' | grep -v '^test/' + git ls-files | grep -E '\.go$|\.rl$' | grep -v '^examples/' | grep -v '^test/' } bad_files=$(