From 5c5384916e8f954f3ea66148ecceb3732584588e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= <bjorn.erik.pedersen@gmail.com> Date: Mon, 13 Aug 2018 20:50:07 +0200 Subject: [PATCH] tpl/tplimpl: Reimplement the ".Params tolower" template transformer All `.Params` are stored lowercase, but it should work to access them `.Page.camelCase` etc. There was, however, some holes in the logic with the old transformer. This commit fixes that by applying a blacklist instead of the old whitelist logic. `.Param` is a very distinct key. The original case will be kept in `.Data.Params.myParam`, but other than that it will be lowercased. Fixes #5068 --- tpl/tplimpl/template_ast_transformers.go | 178 ++++++++++-------- tpl/tplimpl/template_ast_transformers_test.go | 47 +++++ 2 files changed, 150 insertions(+), 75 deletions(-) diff --git a/tpl/tplimpl/template_ast_transformers.go b/tpl/tplimpl/template_ast_transformers.go index bbd0f28a4..c396a1f41 100644 --- a/tpl/tplimpl/template_ast_transformers.go +++ b/tpl/tplimpl/template_ast_transformers.go @@ -24,15 +24,14 @@ import ( // decl keeps track of the variable mappings, i.e. $mysite => .Site etc. type decl map[string]string -var paramsPaths = [][]string{ - {"Params"}, - {"Site", "Params"}, +const ( + paramsIdentifier = "Params" +) - // Site and Pag referenced from shortcodes - {"Page", "Site", "Params"}, - {"Page", "Params"}, - - {"Site", "Language", "Params"}, +// Containers that may contain Params that we will not touch. +var reservedContainers = map[string]bool{ + // Aka .Site.Data.Params which must stay case sensitive. + "Data": true, } type templateContext struct { @@ -155,6 +154,7 @@ func (c *templateContext) updateIdentsIfNeeded(idents []string) { for i := index; i < len(idents); i++ { idents[i] = strings.ToLower(idents[i]) } + } // indexOfReplacementStart will return the index of where to start doing replacement, @@ -167,31 +167,101 @@ func (d decl) indexOfReplacementStart(idents []string) int { return -1 } - first := idents[0] - firstIsVar := first[0] == '$' - - if l == 1 && !firstIsVar { - // This can not be a Params.x - return -1 - } - - if !firstIsVar { - found := false - for _, paramsPath := range paramsPaths { - if first == paramsPath[0] { - found = true - break - } - } - if !found { + if l == 1 { + first := idents[0] + if first == "" || first == paramsIdentifier || first[0] == '$' { + // This can not be a Params.x return -1 } } + var lookFurther bool + var needsVarExpansion bool + for _, ident := range idents { + if ident[0] == '$' { + lookFurther = true + needsVarExpansion = true + break + } else if ident == paramsIdentifier { + lookFurther = true + break + } + } + + if !lookFurther { + return -1 + } + + var resolvedIdents []string + + if !needsVarExpansion { + resolvedIdents = idents + } else { + var ok bool + resolvedIdents, ok = d.resolveVariables(idents) + if !ok { + return -1 + } + } + + var paramFound bool + for i, ident := range resolvedIdents { + if ident == paramsIdentifier { + if i > 0 { + container := resolvedIdents[i-1] + if reservedContainers[container] { + // .Data.Params.someKey + return -1 + } + if !d.isKeyword(container) { + // where $pages ".Params.toc_hide" "!=" true + return -1 + } + } + if i < len(resolvedIdents)-1 { + next := resolvedIdents[i+1] + if !d.isKeyword(next) { + return -1 + } + } + + paramFound = true + break + } + } + + if !paramFound { + return -1 + } + + var paramSeen bool + idx := -1 + for i, ident := range idents { + if ident == "" || ident[0] == '$' { + continue + } + + if ident == paramsIdentifier { + paramSeen = true + idx = -1 + + } else { + if paramSeen { + return i + } + if idx == -1 { + idx = i + } + } + } + return idx + +} + +func (d decl) resolveVariables(idents []string) ([]string, bool) { var ( - resolvedIdents []string - replacements []string - replaced []string + replacements []string + replaced []string ) // An Ident can start out as one of @@ -206,7 +276,7 @@ func (d decl) indexOfReplacementStart(idents []string) int { if i > 20 { // bail out - return -1 + return nil, false } potentialVar := replacements[i] @@ -225,7 +295,7 @@ func (d decl) indexOfReplacementStart(idents []string) int { if !ok { // Temporary range vars. We do not care about those. - return -1 + return nil, false } replacement = strings.TrimPrefix(replacement, ".") @@ -242,52 +312,10 @@ func (d decl) indexOfReplacementStart(idents []string) int { } } - resolvedIdents = append(replaced, idents[1:]...) - - for _, paramPath := range paramsPaths { - if index := indexOfFirstRealIdentAfterWords(resolvedIdents, idents, paramPath...); index != -1 { - return index - } - } - - return -1 + return append(replaced, idents[1:]...), true } -func indexOfFirstRealIdentAfterWords(resolvedIdents, idents []string, words ...string) int { - if !sliceStartsWith(resolvedIdents, words...) { - return -1 - } - - for i, ident := range idents { - if ident == "" || ident[0] == '$' { - continue - } - found := true - for _, word := range words { - if ident == word { - found = false - break - } - } - if found { - return i - } - } - - return -1 -} - -func sliceStartsWith(slice []string, words ...string) bool { - - if len(slice) < len(words) { - return false - } - - for i, word := range words { - if word != slice[i] { - return false - } - } - return true +func (d decl) isKeyword(s string) bool { + return !strings.ContainsAny(s, " -\"") } diff --git a/tpl/tplimpl/template_ast_transformers_test.go b/tpl/tplimpl/template_ast_transformers_test.go index c3cf54940..cf3caeccd 100644 --- a/tpl/tplimpl/template_ast_transformers_test.go +++ b/tpl/tplimpl/template_ast_transformers_test.go @@ -14,6 +14,7 @@ package tplimpl import ( "bytes" + "fmt" "testing" "html/template" @@ -24,6 +25,11 @@ import ( var ( testFuncs = map[string]interface{}{ "Echo": func(v interface{}) interface{} { return v }, + "where": func(seq, key interface{}, args ...interface{}) (interface{}, error) { + return map[string]interface{}{ + "ByWeight": fmt.Sprintf("%v:%v:%v", seq, key, args), + }, nil + }, } paramsData = map[string]interface{}{ @@ -31,6 +37,15 @@ var ( "Slice": []int{1, 3}, "Params": map[string]interface{}{ "lower": "P1L", + "slice": []int{1, 3}, + }, + "Pages": map[string]interface{}{ + "ByWeight": []int{1, 3}, + }, + "CurrentSection": map[string]interface{}{ + "Params": map[string]interface{}{ + "lower": "pcurrentsection", + }, }, "Site": map[string]interface{}{ "Params": map[string]interface{}{ @@ -52,12 +67,14 @@ var ( paramsTempl = ` {{ $page := . }} +{{ $pages := .Pages }} {{ $pageParams := .Params }} {{ $site := .Site }} {{ $siteParams := .Site.Params }} {{ $data := .Site.Data }} {{ $notparam := .NotParam }} +PCurrentSection: {{ .CurrentSection.Params.LOWER }} P1: {{ .Params.LOWER }} P1_2: {{ $.Params.LOWER }} P1_3: {{ $page.Params.LOWER }} @@ -109,6 +126,15 @@ RANGE: {{ . }}: {{ $.Params.LOWER }} F1: {{ printf "themes/%s-theme" .Site.Params.LOWER }} F2: {{ Echo (printf "themes/%s-theme" $lower) }} F3: {{ Echo (printf "themes/%s-theme" .Site.Params.LOWER) }} + +PSLICE: {{ range .Params.SLICE }}PSLICE{{.}}|{{ end }} + +{{ $pages := "foo" }} +{{ $pages := where $pages ".Params.toc_hide" "!=" true }} +PARAMS STRING: {{ $pages.ByWeight }} +PARAMS STRING2: {{ with $pages }}{{ .ByWeight }}{{ end }} +{{ $pages3 := where ".Params.TOC_HIDE" "!=" .Params.LOWER }} +PARAMS STRING3: {{ $pages3.ByWeight }} ` ) @@ -164,6 +190,14 @@ func TestParamsKeysToLower(t *testing.T) { require.Contains(t, result, "F2: themes/P2L-theme") require.Contains(t, result, "F3: themes/P2L-theme") + require.Contains(t, result, "PSLICE: PSLICE1|PSLICE3|") + require.Contains(t, result, "PARAMS STRING: foo:.Params.toc_hide:[!= true]") + require.Contains(t, result, "PARAMS STRING2: foo:.Params.toc_hide:[!= true]") + require.Contains(t, result, "PARAMS STRING3: .Params.TOC_HIDE:!=:[P1L]") + + // Issue #5068 + require.Contains(t, result, "PCurrentSection: pcurrentsection") + } func BenchmarkTemplateParamsKeysToLower(b *testing.B) { @@ -197,6 +231,9 @@ func TestParamsKeysToLowerVars(t *testing.T) { "Params": map[string]interface{}{ "colors": map[string]interface{}{ "blue": "Amber", + "pretty": map[string]interface{}{ + "first": "Indigo", + }, }, }, } @@ -205,8 +242,14 @@ func TestParamsKeysToLowerVars(t *testing.T) { paramsTempl = ` {{$__amber_1 := .Params.Colors}} {{$__amber_2 := $__amber_1.Blue}} +{{$__amber_3 := $__amber_1.Pretty}} +{{$__amber_4 := .Params}} + Color: {{$__amber_2}} Blue: {{ $__amber_1.Blue}} +Pretty First1: {{ $__amber_3.First}} +Pretty First2: {{ $__amber_1.Pretty.First}} +Pretty First3: {{ $__amber_4.COLORS.PRETTY.FIRST}} ` ) @@ -225,6 +268,10 @@ Blue: {{ $__amber_1.Blue}} result := b.String() require.Contains(t, result, "Color: Amber") + require.Contains(t, result, "Blue: Amber") + require.Contains(t, result, "Pretty First1: Indigo") + require.Contains(t, result, "Pretty First2: Indigo") + require.Contains(t, result, "Pretty First3: Indigo") }