From 35c34bc571d5b8f4cca5b26a56e75131f96a483a Mon Sep 17 00:00:00 2001 From: mathew murphy Date: Tue, 1 Oct 2024 16:29:36 -0500 Subject: [PATCH] Implement customization of JSON values; delinting --- .github/workflows/golangci-lint.yml | 3 +- .golangci.toml | 18 ++++ README.md | 3 + cmd/mustache/main.go | 8 +- mustache.go | 124 ++++++++++++++++++++-------- mustache_test.go | 101 ++++++++++++++++++---- partials.go | 11 ++- v1api/v1api.go | 3 +- 8 files changed, 205 insertions(+), 66 deletions(-) create mode 100644 .golangci.toml diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index a28a4f4..292a700 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -16,5 +16,4 @@ jobs: with: version: latest # skip cache because of flaky behaviors - skip-build-cache: true - skip-pkg-cache: true + skip-cache: true diff --git a/.golangci.toml b/.golangci.toml new file mode 100644 index 0000000..17443e9 --- /dev/null +++ b/.golangci.toml @@ -0,0 +1,18 @@ + +[linters] +enable = [ + "copyloopvar", + "err113", + "errchkjson", + "goconst", + "godot", + "gofmt", + "makezero", + "unconvert", + "unparam", + "usestdlibvars" +] + +[[issues.exclude-rules]] +path = "(.+)_test\\.go" +linters = [ "err113" ] diff --git a/README.md b/README.md index dc5412c..8a925a8 100644 --- a/README.md +++ b/README.md @@ -140,6 +140,9 @@ rules are used. To do this, use `.WithEscapeMode(mustache.JSON)` to set the esca JSON escaping rules are different from the rules used by Go's text/template.JSEscape, and do not guarantee that the JSON will be safe to include as part of an HTML page. In JSON mode, references to objects and slices in the template will be rendered to JSON objects and arrays. +In JSON mode, slices/arrays and maps will be rendered as JSON arrays and objects (respectively). This behavior can be +customized by setting a JSON value rendering function using `WithJSONMarshalFn`. + A third mode of `mustache.Raw` allows the use of Mustache templates to generate plain text, such as e-mail messages and console application help text. diff --git a/cmd/mustache/main.go b/cmd/mustache/main.go index 34ba0a2..d8359e7 100644 --- a/cmd/mustache/main.go +++ b/cmd/mustache/main.go @@ -101,11 +101,11 @@ func run(cmd *cobra.Command, args []string) error { func parseDataFromStdIn() (any, error) { b, err := io.ReadAll(os.Stdin) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to read data: %w", err) } var data any if err := yaml.Unmarshal(b, &data); err != nil { - return nil, err + return nil, fmt.Errorf("failed to unmarshal data: %w", err) } return data, nil } @@ -113,11 +113,11 @@ func parseDataFromStdIn() (any, error) { func parseDataFromFile(filePath string) (any, error) { b, err := os.ReadFile(filePath) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to read data: %w", err) } var data any if err := yaml.Unmarshal(b, &data); err != nil { - return nil, err + return nil, fmt.Errorf("failed to unmarshal data: %w", err) } return data, nil } diff --git a/mustache.go b/mustache.go index ec90be8..4647f38 100644 --- a/mustache.go +++ b/mustache.go @@ -13,15 +13,40 @@ import ( "unicode" ) -// RenderFn is the signature of a function which can be called from a lambda section +// sentinelError is an error type for constant sentinel errors. +type sentinelError string + +// Error implements the error interface. +func (e sentinelError) Error() string { return string(e) } + +// ErrMissingVar indicates that a variable was referred to, but its value could not be resolved. +const ErrMissingVar = sentinelError("missing variable") + +// ErrUnsafePartialName indicates that a partial had a name that was unsafe for use with file-based templates. +const ErrUnsafePartialName = sentinelError("unsafe partial name") + +// ErrPartialNotFound indicates that a partial's definition could not be located. +const ErrPartialNotFound = sentinelError("partial not found") + +// ErrNoPartialProvider indicates that a partial was referred to, but no partial provider had been set up. +const ErrNoPartialProvider = sentinelError("no partial provider") + +// RenderFn is the signature of a function which can be called from a lambda section. type RenderFn func(text string) (string, error) +// Compiler represents the overall object used to compile Mustache templates. type Compiler struct { - partial PartialProvider - outputMode EscapeMode - errorOnMissing bool + partial PartialProvider + outputMode EscapeMode + errorOnMissing bool + customJSONMarshaler JSONMarshalFn } +// JSONMarshalFn is the signature of a function which can marshal a value to JSON and +// write it to the supplied Writer. +type JSONMarshalFn func(dest io.Writer, data any) error + +// New returns a new instance of the Mustache template compiler. func New() *Compiler { return &Compiler{} } @@ -47,9 +72,20 @@ func (r *Compiler) WithErrors(b bool) *Compiler { return r } +// WithJSONMarshalFn adds a custom function to control how values are marshaled to JSON. +// The function is passed a Writer and a value, and is expected to write the marshaled +// value to the Writer. The function is expected to handle any escaping necessary. +// The default behavior with no custom marshal function is to call JSONMarshal, which +// in turn calls JSONEscape. Setting the custom marshal function to nil (the default) +// disables the feature and goes back to default behavior. +func (r *Compiler) WithJSONMarshalFn(m JSONMarshalFn) *Compiler { + r.customJSONMarshaler = m + return r +} + // CompileString compiles a Mustache template from a string. func (r *Compiler) CompileString(data string) (*Template, error) { - tmpl := Template{data, "{{", "}}", 0, 1, []any{}, false, r.partial, r.outputMode, r.errorOnMissing, r} + tmpl := Template{data, "{{", "}}", 0, 1, []any{}, false, r.partial, r.outputMode, r.errorOnMissing, r.customJSONMarshaler, r} err := tmpl.parse() if err != nil { return nil, err @@ -61,7 +97,7 @@ func (r *Compiler) CompileString(data string) (*Template, error) { func (r *Compiler) CompileFile(filename string) (*Template, error) { data, err := os.ReadFile(filename) if err != nil { - return nil, err + return nil, fmt.Errorf("can't read template file: %w", err) } return r.CompileString(string(data)) } @@ -152,17 +188,18 @@ const ( // Template represents a compiled mustache template which can be used to render data. type Template struct { - data string - otag string - ctag string - p int - curline int - elems []any - forceRaw bool - partial PartialProvider - outputMode EscapeMode - errorOnMissing bool - parent *Compiler + data string + otag string + ctag string + p int + curline int + elems []any + forceRaw bool + partial PartialProvider + outputMode EscapeMode + errorOnMissing bool + customJSONMarshaler JSONMarshalFn + parent *Compiler } type parseError struct { @@ -349,18 +386,19 @@ func (tmpl *Template) readTag(mayStandalone bool) (*tagReadingResult, error) { if !strings.Contains(SkipWhitespaceTagTypes, tag[0:1]) { standalone = false } else { - if eow == len(tmpl.data) { + switch { + case eow == len(tmpl.data): standalone = true tmpl.p = eow - } else if eow < len(tmpl.data) && tmpl.data[eow] == '\n' { + case eow < len(tmpl.data) && tmpl.data[eow] == '\n': standalone = true tmpl.p = eow + 1 tmpl.curline++ - } else if eow+1 < len(tmpl.data) && tmpl.data[eow] == '\r' && tmpl.data[eow+1] == '\n' { + case eow+1 < len(tmpl.data) && tmpl.data[eow] == '\r' && tmpl.data[eow+1] == '\n': standalone = true tmpl.p = eow + 2 tmpl.curline++ - } else { + default: standalone = false } } @@ -372,6 +410,7 @@ func (tmpl *Template) readTag(mayStandalone bool) (*tagReadingResult, error) { }, nil } +//nolint:unparam func (tmpl *Template) parsePartial(name, indent string) (*partialElement, error) { return &partialElement{ name: name, @@ -589,7 +628,7 @@ Outer: if !errorOnMissing { return reflect.Value{}, nil } - return reflect.Value{}, fmt.Errorf("missing variable %q", name) + return reflect.Value{}, fmt.Errorf("can't resolve %s: %w", name, ErrMissingVar) } func isEmpty(v reflect.Value) bool { @@ -725,7 +764,7 @@ func JSONEscape(dest io.Writer, data string) error { } } if err != nil { - return err + return fmt.Errorf("error escaping JSON: %w", err) } } return nil @@ -775,28 +814,26 @@ func (tmpl *Template) renderElement(element any, contextChain []any, buf io.Writ } if val.IsValid() { + value := val.Interface() if elem.raw { - fmt.Fprint(buf, val.Interface()) + fmt.Fprint(buf, value) } else { - s := fmt.Sprint(val.Interface()) switch tmpl.outputMode { case EscapeJSON: - // Output arrays and objects in JSON format, if in JSON mode - kind := reflect.TypeOf(val.Interface()).Kind() - if kind == reflect.Slice || kind == reflect.Array || kind == reflect.Map { - marshalledJson, err := json.Marshal(val.Interface()) - if err != nil { + if tmpl.customJSONMarshaler != nil { + if err := tmpl.customJSONMarshaler(buf, value); err != nil { + return err + } + } else { + if err := JSONMarshal(buf, value); err != nil { return err } - buf.Write(marshalledJson) - break - } - if err = JSONEscape(buf, s); err != nil { - return err } case EscapeHTML: + s := fmt.Sprint(val.Interface()) template.HTMLEscape(buf, []byte(s)) case Raw: + s := fmt.Sprint(val.Interface()) if _, err = buf.Write([]byte(s)); err != nil { return err } @@ -822,6 +859,23 @@ func (tmpl *Template) renderElement(element any, contextChain []any, buf io.Writ return nil } +// JSONMarshal is the default function for marshaling data into the template output +// when running in JSON mode. +func JSONMarshal(dest io.Writer, value any) error { + // Output arrays, slices and maps in JSON format + kind := reflect.TypeOf(value).Kind() + if kind == reflect.Slice || kind == reflect.Array || kind == reflect.Map { + marshalledJson, err := json.Marshal(value) + if err != nil { + return fmt.Errorf("error marshaling %T: %w", value, err) + } + _, err = dest.Write(marshalledJson) + return err + } + s := fmt.Sprint(value) + return JSONEscape(dest, s) +} + func (tmpl *Template) renderTemplate(contextChain []any, buf io.Writer) error { for _, elem := range tmpl.elems { if err := tmpl.renderElement(elem, contextChain, buf); err != nil { diff --git a/mustache_test.go b/mustache_test.go index 17f2523..a02ed81 100644 --- a/mustache_test.go +++ b/mustache_test.go @@ -2,13 +2,17 @@ package mustache import ( "bytes" + "errors" "fmt" + "io" "os" "path" "strings" "testing" ) +const TestString = "hello world" + type Test struct { tmpl string context any @@ -91,15 +95,15 @@ func TestTagType(t *testing.T) { var tests = []Test{ {`{{/}}`, nil, "", parseError{line: 1, message: "unmatched close tag"}}, - {`hello world`, nil, "hello world", nil}, - {`hello {{name}}`, map[string]string{"name": "world"}, "hello world", nil}, + {`hello world`, nil, TestString, nil}, + {`hello {{name}}`, map[string]string{"name": "world"}, TestString, nil}, {`{{var}}`, map[string]string{"var": "5 > 2"}, "5 > 2", nil}, {`{{{var}}}`, map[string]string{"var": "5 > 2"}, "5 > 2", nil}, {`{{var}}`, map[string]string{"var": "& \" < >"}, "& " < >", nil}, {`{{{var}}}`, map[string]string{"var": "& \" < >"}, "& \" < >", nil}, {`{{a}}{{b}}{{c}}{{d}}`, map[string]string{"a": "a", "b": "b", "c": "c", "d": "d"}, "abcd", nil}, {`0{{a}}1{{b}}23{{c}}456{{d}}89`, map[string]string{"a": "a", "b": "b", "c": "c", "d": "d"}, "0a1b23c456d89", nil}, - {`hello {{! comment }}world`, map[string]string{}, "hello world", nil}, + {`hello {{! comment }}world`, map[string]string{}, TestString, nil}, {`{{ a }}{{=<% %>=}}<%b %><%={{ }}=%>{{ c }}`, map[string]string{"a": "a", "b": "b", "c": "c"}, "abc", nil}, {`{{ a }}{{= <% %> =}}<%b %><%= {{ }}=%>{{c}}`, map[string]string{"a": "a", "b": "b", "c": "c"}, "abc", nil}, @@ -188,9 +192,9 @@ var tests = []Test{ {`{{#user}}{{#Func6}}{{#Allow}}abcd{{/Allow}}{{/Func6}}{{/user}}`, map[string]any{"user": &User{"Mike", 1}}, "abcd", nil}, // context chaining - {`hello {{#section}}{{name}}{{/section}}`, map[string]any{"section": map[string]string{"name": "world"}}, "hello world", nil}, - {`hello {{#section}}{{name}}{{/section}}`, map[string]any{"name": "bob", "section": map[string]string{"name": "world"}}, "hello world", nil}, - {`hello {{#bool}}{{#section}}{{name}}{{/section}}{{/bool}}`, map[string]any{"bool": true, "section": map[string]string{"name": "world"}}, "hello world", nil}, + {`hello {{#section}}{{name}}{{/section}}`, map[string]any{"section": map[string]string{"name": "world"}}, TestString, nil}, + {`hello {{#section}}{{name}}{{/section}}`, map[string]any{"name": "bob", "section": map[string]string{"name": "world"}}, TestString, nil}, + {`hello {{#bool}}{{#section}}{{name}}{{/section}}{{/bool}}`, map[string]any{"bool": true, "section": map[string]string{"name": "world"}}, TestString, nil}, {`{{#users}}{{canvas}}{{/users}}`, map[string]any{"canvas": "hello", "users": []User{{"Mike", 1}}}, "hello", nil}, {`{{#categories}}{{DisplayName}}{{/categories}}`, map[string][]*Category{ "categories": {&Category{"a", "b"}}, @@ -235,7 +239,7 @@ func TestBasic(t *testing.T) { if err == nil && tm != nil { output, err = tm.Render(test.tmpl, test.context) } - if err != test.err { + if !errors.Is(err, test.err) { t.Errorf("%q expected %q but got error %v", test.tmpl, test.expected, err) } else if output != test.expected { t.Errorf("%q expected %q got %q", test.tmpl, test.expected, output) @@ -303,7 +307,7 @@ func TestMissing(t *testing.T) { func TestFile(t *testing.T) { filename := path.Join(path.Join(os.Getenv("PWD"), "tests"), "test1.mustache") - expected := "hello world" + expected := TestString cmpl, err := New().CompileFile(filename) if err != nil { t.Error(err) @@ -318,7 +322,7 @@ func TestFile(t *testing.T) { func TestFRender(t *testing.T) { filename := path.Join(path.Join(os.Getenv("PWD"), "tests"), "test1.mustache") - expected := "hello world" + expected := TestString tmpl, err := New().CompileFile(filename) if err != nil { t.Fatal(err) @@ -341,7 +345,7 @@ func TestPartial(t *testing.T) { } testdir := path.Join(cwd, "tests") filename := path.Join(testdir, "test2.mustache") - expected := "hello world" + expected := TestString tmpl, err := New().WithErrors(true). WithPartials(&FileProvider{Paths: []string{testdir}, Extensions: []string{".mustache"}}). CompileFile(filename) @@ -522,7 +526,68 @@ func TestRenderJSON(t *testing.T) { } } -// Make sure bugs caught by fuzz testing don't creep back in +func TestJSONCustomMarshal(t *testing.T) { + var customMarshaler JSONMarshalFn = func(dest io.Writer, data any) error { + if ia, ok := data.([]string); ok { + dest.Write([]byte(strings.Join(ia, "-"))) + return nil + } + return JSONMarshal(dest, data) + } + data := map[string]any{ + "a": []string{"one", "two", "three"}, + "b": []int{1, 2, 3}, + } + tmpl, err := New().WithEscapeMode(EscapeJSON).WithJSONMarshalFn(customMarshaler).CompileString(`{{a}} {{b}}`) + if err != nil { + t.Error(err) + } + got, err := tmpl.Render(data) + if err != nil { + t.Error(err) + } + want := `one-two-three [1,2,3]` + if got != want { + t.Errorf("got %s expected %s", got, want) + } +} + +func TestJSONMarshal(t *testing.T) { + tests := []struct { + Input any + Output string + }{ + { + Input: []int{1, 2, 3}, + Output: `[1,2,3]`, + }, + { + Input: map[string]any{"a": "alpha", "b": "beta", "x": 12, "y": 9.4}, + Output: `{"a":"alpha","b":"beta","x":12,"y":9.4}`, + }, + { + Input: 3, + Output: "3", + }, + { + Input: "Tuesday", + Output: "Tuesday", + }, + } + for _, tst := range tests { + var buf bytes.Buffer + err := JSONMarshal(&buf, tst.Input) + if err != nil { + t.Error(err) + } + got := buf.String() + if got != tst.Output { + t.Errorf("got %s expected %s", got, tst.Output) + } + } +} + +// Make sure bugs caught by fuzz testing don't creep back in. func TestCrashers(t *testing.T) { crashers := []string{ `{{#}}{{#}}{{#}}{{#}}{{#}}{{=}}`, @@ -565,8 +630,8 @@ func TestMultiContext(t *testing.T) { if err != nil { t.Error(err) } - if output != "hello world" || output2 != "hello world" { - t.Errorf("TestMultiContext expected %q got %q", "hello world", output) + if output != TestString || output2 != "hello world" { + t.Errorf("TestMultiContext expected %q got %q", TestString, output) } } @@ -603,7 +668,7 @@ func TestLambdaError(t *testing.T) { templ := `stop_at_error.{{#lambda}}{{/lambda}}.never_here` data := make(map[string]any) data["lambda"] = func(text string, render RenderFn) (string, error) { - return "", fmt.Errorf("test err") + return "", errors.New("test err") } tmpl, err := New().CompileString(templ) if err != nil { @@ -617,10 +682,10 @@ func TestLambdaError(t *testing.T) { } var malformed = []Test{ - {`{{#a}}{{}}{{/a}}`, Data{true, "hello"}, "", fmt.Errorf("line 1: empty tag")}, - {`{{}}`, nil, "", fmt.Errorf("line 1: empty tag")}, - {`{{}`, nil, "", fmt.Errorf("line 1: unmatched open tag")}, - {`{{`, nil, "", fmt.Errorf("line 1: unmatched open tag")}, + {`{{#a}}{{}}{{/a}}`, Data{true, "hello"}, "", errors.New("line 1: empty tag")}, + {`{{}}`, nil, "", errors.New("line 1: empty tag")}, + {`{{}`, nil, "", errors.New("line 1: unmatched open tag")}, + {`{{`, nil, "", errors.New("line 1: unmatched open tag")}, // invalid syntax - https://github.com/hoisie/mustache/issues/10 {`{{#a}}{{#b}}{{/a}}{{/b}}}`, map[string]any{}, "", fmt.Errorf("line 1: interleaved closing tag: a")}, } diff --git a/partials.go b/partials.go index 98ad22e..2553a82 100644 --- a/partials.go +++ b/partials.go @@ -1,7 +1,6 @@ package mustache import ( - "errors" "fmt" "io" "os" @@ -40,7 +39,7 @@ func (fp *FileProvider) Get(name string) (string, error) { cname = strings.ReplaceAll(filepath.Clean(cname), "\\", "/") cname = strings.TrimLeft(cname, "/") if cname != name || cname == "" { - return "", fmt.Errorf("unsafe partial name passed to FileProvider: %s", name) + return "", fmt.Errorf("can't use %s: %w", name, ErrUnsafePartialName) //nolint:all } clean = cname } @@ -72,13 +71,13 @@ func (fp *FileProvider) Get(name string) (string, error) { } if f == nil { - return "", fmt.Errorf("%s: partial not found", name) // nil + return "", fmt.Errorf("%s: %w", name, ErrPartialNotFound) } defer f.Close() data, err := io.ReadAll(f) if err != nil { - return "", err + return "", fmt.Errorf("error reading partial %s: %w", name, err) } return string(data), nil @@ -107,7 +106,7 @@ var _ PartialProvider = (*StaticProvider)(nil) func (tmpl *Template) getPartials(partials PartialProvider, name, indent string) (*Template, error) { if partials == nil { - return nil, errors.New("no partial provider specified") + return nil, ErrNoPartialProvider } data, err := partials.Get(name) if err != nil { @@ -118,5 +117,5 @@ func (tmpl *Template) getPartials(partials PartialProvider, name, indent string) r := regexp.MustCompile(`(?m:^(.+)$)`) data = r.ReplaceAllString(data, indent+"$1") - return tmpl.parent.CompileString(data) //, partials) + return tmpl.parent.CompileString(data) } diff --git a/v1api/v1api.go b/v1api/v1api.go index 27b20d5..0ed3ca9 100644 --- a/v1api/v1api.go +++ b/v1api/v1api.go @@ -3,6 +3,7 @@ package v1api import ( + "fmt" "os" "path" @@ -22,7 +23,7 @@ func ParseString(data string) (*mustache.Template, error) { func ParseStringRaw(data string, forceRaw bool) (*mustache.Template, error) { cwd, err := os.Getwd() if err != nil { - return nil, err + return nil, fmt.Errorf("couldn't parse string: %w", err) } partials := &mustache.FileProvider{ Paths: []string{cwd},