From 7d4dcd64ebded41f5edcb41904d1325aeb14f65b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Sun, 23 Aug 2026 10:01:26 +0200 Subject: [PATCH] refactor(zas)!: replace reflect-based embed dispatch with a static lookup handleEmbedTags turned a mimetypes config value into a Generator method call via reflect.ValueOf(gen).MethodByName, guarded by isEmbedPluginMethod to avoid a runtime panic whenever the resolved name happened to match a real but wrong-signature method (e.g. Run). The built-in handler set is closed and known at compile time - the README documents only three names, and any other configured name already falls through to the external mzs* plugin mechanism - so the reflection bought no actual flexibility, only a panic risk that had to be guarded against by hand. Dispatch is now a small switch-based lookup (embedPlugin) returning a typed function value, so a mismatched signature is a compile error instead of a guarded runtime panic, and isEmbedPluginMethod is gone along with the reflect import. The plugin name match is now a plain lowercase comparison instead of cases.Title(language.English), which also drops the golang.org/x/text dependency entirely. The three built-in handlers (previously Markdown, Plain, Html) are unexported, since their only reason for being exported was reflection discovery. scanEmbedTargets had its own, reflection-free copy of the same Title-cased name comparison to decide whether to recurse into an embed target; it's updated to the same lowercase comparison for consistency. The documented mimetypes override mechanism is unaffected: any configured name other than markdown/plain/html (case-insensitively) still falls through to the external mzs* plugin path exactly as before, letting an operator override a built-in handler. BREAKING CHANGE: Generator's Markdown, Plain, and Html methods are now unexported (markdown, plain, html). Any external code calling them directly must be updated; they were never part of a documented public API. --- README.md | 2 +- embed_containment_test.go | 6 +-- embed_dispatch_test.go | 34 ++++----------- generate.go | 90 ++++++++++++++++----------------------- go.mod | 1 - plain_embed_test.go | 18 ++++---- 6 files changed, 58 insertions(+), 93 deletions(-) diff --git a/README.md b/README.md index 8b2cd1e..d9bc05f 100644 --- a/README.md +++ b/README.md @@ -131,7 +131,7 @@ If Zas finds an embed tag with a type attribute set to `text/yaml+myplugin`, it ``` -Maybe you are asking yourself: "Where is mzsmarkdown?". Nowhere! It is a particular case where Zas calls an exported method Markdown. I wanted to allow anyone to override internal Markdown processing if they wish. +Maybe you are asking yourself: "Where is mzsmarkdown?". Nowhere! It is a particular case where Zas has a built-in handler for it. I wanted to allow anyone to override internal Markdown processing if they wish. If you develop a new plugin, please contact me, and I will list it here :) Please, keep in mind: make it [idempotent](http://en.wikipedia.org/wiki/Idempotence). diff --git a/embed_containment_test.go b/embed_containment_test.go index 5fda1f6..cb29d99 100644 --- a/embed_containment_test.go +++ b/embed_containment_test.go @@ -27,9 +27,9 @@ type embedHandlerCase struct { } var embedHandlerCases = []embedHandlerCase{ - {"Markdown", "text/markdown", "note.md", "# Note\n\nHello from markdown.\n", (*Generator).Markdown, "Hello from markdown."}, - {"Plain", "text/plain", "note.txt", "hello from plain", (*Generator).Plain, "hello from plain"}, - {"Html", "text/html", "note.html", "

hello from html

", (*Generator).Html, "hello from html"}, + {"Markdown", "text/markdown", "note.md", "# Note\n\nHello from markdown.\n", (*Generator).markdown, "Hello from markdown."}, + {"Plain", "text/plain", "note.txt", "hello from plain", (*Generator).plain, "hello from plain"}, + {"Html", "text/html", "note.html", "

hello from html

", (*Generator).html, "hello from html"}, } func embedDocFor(t *testing.T, src, typ string) *goquery.Document { diff --git a/embed_dispatch_test.go b/embed_dispatch_test.go index 9db4441..62014f4 100644 --- a/embed_dispatch_test.go +++ b/embed_dispatch_test.go @@ -1,39 +1,21 @@ package zas import ( - "reflect" "strings" "testing" "github.com/PuerkitoBio/goquery" ) -// config data must not be able to pick an arbitrary Generator method via -// reflection and panic method.Call with a mismatched arity/signature. +// config data must not be able to pick an arbitrary Generator method for +// embed dispatch - only the closed set embedPlugin knows about. -func TestIsEmbedPluginMethod(t *testing.T) { - gen := &Generator{} - - if valid := reflect.ValueOf(gen).MethodByName("Markdown"); !isEmbedPluginMethod(valid) { - t.Fatal("isEmbedPluginMethod(Markdown) = false, want true") - } - // Run() error is a real exported Generator method, but with the wrong - // arity (0 args) for embed dispatch - must be rejected, not panic. - if wrongArity := reflect.ValueOf(gen).MethodByName("Run"); isEmbedPluginMethod(wrongArity) { - t.Fatal("isEmbedPluginMethod(Run) = true, want false") - } - if notFound := reflect.ValueOf(gen).MethodByName("DoesNotExist"); isEmbedPluginMethod(notFound) { - t.Fatal("isEmbedPluginMethod(missing method) = true, want false") - } -} - -func TestHandleEmbedTagsFallsBackOnWrongArityMethod(t *testing.T) { - // Reproduces the audit repro: mimetypes: {text/x-t: run} resolves to the - // real, exported Run() error method, which has the wrong signature for - // embed dispatch. Must fall back to the external plugin path instead of - // panicking in reflect.Value.Call. The fallback path itself is a no-op - // here (see audit A1, out of scope), so simply completing without a - // panic is the regression this guards. +func TestHandleEmbedTagsFallsBackOnUnknownPluginName(t *testing.T) { + // mimetypes: {text/x-t: run} resolves to "run", which isn't one of + // embedPlugin's three known names (it happens to also be the name of a + // real, exported, wrong-signature Generator method, Run() error - but + // that's no longer relevant: embedPlugin never looks at Generator's + // actual method set). Must fall back to the external plugin path. gen := &Generator{ Config: ConfigSection{ "mimetypes": ConfigSection{"text/x-t": "run"}, diff --git a/generate.go b/generate.go index cf0f3e2..64c26e1 100644 --- a/generate.go +++ b/generate.go @@ -27,7 +27,6 @@ import ( "os" "os/exec" "path/filepath" - "reflect" "regexp" "runtime" "strings" @@ -46,8 +45,6 @@ import ( yaml "go.yaml.in/yaml/v3" html5 "golang.org/x/net/html" "golang.org/x/net/html/atom" - "golang.org/x/text/cases" - "golang.org/x/text/language" ) var helpers = thtml.FuncMap{ @@ -1475,8 +1472,8 @@ func (gen *Generator) resolveEmbedSrc(baseDir, src string) (string, error) { } // embedTargetModTime returns the latest mtime among path's own literal -// targets, recursed the same way the Markdown and Html -// embed handlers themselves recurse (Plain never re-parses its target, and +// targets, recursed the same way the markdown and html +// embed handlers themselves recurse (plain never re-parses its target, and // neither does an external MIME-type plugin, so recursion stops there too). // baseDir resolves a relative src exactly like NewZasData/Generate set // data.embedBaseDir for path's own render. Two things are deliberately left @@ -1540,7 +1537,7 @@ func (gen *Generator) scanEmbedTargets(path, baseDir string, visited map[string] if info, statErr := os.Stat(resolved); statErr == nil && info.ModTime().After(latest) { latest = info.ModTime() } - if method := cases.Title(language.English).String(gen.resolveMIMETypePlugin(typ)); method == "Markdown" || method == "Html" { + if name := strings.ToLower(gen.resolveMIMETypePlugin(typ)); name == "markdown" || name == "html" { if sub := gen.scanEmbedTargets(resolved, filepath.Dir(resolved), visited, depth+1); sub.After(latest) { latest = sub } @@ -1548,8 +1545,8 @@ func (gen *Generator) scanEmbedTargets(path, baseDir string, visited map[string] } } -// Markdown embeds a Markdown file. -func (gen *Generator) Markdown(e *goquery.Selection, _ *goquery.Document, data *ZasData) (err error) { +// markdown embeds a Markdown file. +func (gen *Generator) markdown(e *goquery.Selection, _ *goquery.Document, data *ZasData) (err error) { if src, ok := e.Attr(atom.Src.String()); ok { resolved, err := gen.resolveEmbedSrc(data.embedBaseDir, src) if err != nil { @@ -1587,8 +1584,8 @@ func (gen *Generator) Markdown(e *goquery.Selection, _ *goquery.Document, data * return } -// Plain embeds a plain text file. -func (gen *Generator) Plain(e *goquery.Selection, _ *goquery.Document, data *ZasData) (err error) { +// plain embeds a plain text file. +func (gen *Generator) plain(e *goquery.Selection, _ *goquery.Document, data *ZasData) (err error) { if src, ok := e.Attr(atom.Src.String()); ok { resolved, err := gen.resolveEmbedSrc(data.embedBaseDir, src) if err != nil { @@ -1613,8 +1610,8 @@ func (gen *Generator) Plain(e *goquery.Selection, _ *goquery.Document, data *Zas return } -// Html embeds a HTML file. -func (gen *Generator) Html(e *goquery.Selection, _ *goquery.Document, data *ZasData) (err error) { +// html embeds a HTML file. +func (gen *Generator) html(e *goquery.Selection, _ *goquery.Document, data *ZasData) (err error) { if src, ok := e.Attr(atom.Src.String()); ok { resolved, err := gen.resolveEmbedSrc(data.embedBaseDir, src) if err != nil { @@ -1649,10 +1646,34 @@ func (gen *Generator) Html(e *goquery.Selection, _ *goquery.Document, data *ZasD return } +// embedPlugin returns the built-in embed handler named by name (the +// lowercased mimetypes config value that selected it - see +// resolveMIMETypePlugin and handleEmbedTags), and whether name matched one +// at all. This is the closed, compile-time-known set of built-in handlers: +// a switch, not a package-level map of method expressions, because a +// package-level map here would create a genuine initialization cycle +// (markdown calls parseAndReplace, which calls handleEmbedTags, which would +// need to read the very map being initialized). Each returned value already +// has the exact +// func(*Generator, *goquery.Selection, *goquery.Document, *ZasData) error +// shape, checked by the compiler - no runtime signature validation needed. +func embedPlugin(name string) (fn func(*Generator, *goquery.Selection, *goquery.Document, *ZasData) error, ok bool) { + switch name { + case "markdown": + return (*Generator).markdown, true + case "plain": + return (*Generator).plain, true + case "html": + return (*Generator).html, true + default: + return nil, false + } +} + /* * Handles tags. * - * They can be handled with MIME type plugins or internal exported methods like Markdown. + * They can be handled with MIME type plugins or built-in handlers like markdown. */ func (gen *Generator) handleEmbedTags(doc *goquery.Document, data *ZasData) (err error) { doc.Find(atom.Embed.String()).EachWithBreak(func(_ int, e *goquery.Selection) bool { @@ -1663,19 +1684,10 @@ func (gen *Generator) handleEmbedTags(doc *goquery.Document, data *ZasData) (err return false } plugin := gen.resolveMIMETypePlugin(typ) - method := reflect.ValueOf(gen).MethodByName(cases.Title(language.English).String(plugin)) - if !isEmbedPluginMethod(method) { - err = gen.handleMIMETypePlugin(e) + if fn, ok := embedPlugin(strings.ToLower(plugin)); ok { + err = fn(gen, e, doc, data) } else { - args := make([]reflect.Value, 3) - args[0] = reflect.ValueOf(e) - args[1] = reflect.ValueOf(doc) - args[2] = reflect.ValueOf(data) - r := method.Call(args) - rerr := r[0].Interface() - if ierr, ok := rerr.(error); ok { - err = ierr - } + err = gen.handleMIMETypePlugin(e) } if err != nil { return false @@ -1686,34 +1698,6 @@ func (gen *Generator) handleEmbedTags(doc *goquery.Document, data *ZasData) (err return } -/* - * Reports whether method is a valid embed-plugin dispatch target: a method - * with the exact (e *goquery.Selection, doc *goquery.Document, data *ZasData) error - * signature. Config data chooses the method name (see resolveMIMETypePlugin), - * so any exported Generator method is reachable by MethodByName and must be - * shape-checked before Call to avoid a reflect panic on arity/type mismatch. - */ -func isEmbedPluginMethod(method reflect.Value) bool { - if !method.IsValid() { - return false - } - want := []reflect.Type{ - reflect.TypeFor[*goquery.Selection](), - reflect.TypeFor[*goquery.Document](), - reflect.TypeFor[*ZasData](), - } - t := method.Type() - if t.NumIn() != len(want) || t.NumOut() != 1 { - return false - } - for i, w := range want { - if t.In(i) != w { - return false - } - } - return true -} - // pluginNameRe restricts resolved plugin names to safe exec.Command argv[0] // suffixes. Without it, a mimetypes entry like {text/x: "../../evil"} would // make exec.Command skip PATH lookup and run a path relative to the working diff --git a/go.mod b/go.mod index f7720d3..a4fcc61 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,6 @@ require ( github.com/yuin/goldmark v1.8.5 go.yaml.in/yaml/v3 v3.0.5 golang.org/x/net v0.58.0 - golang.org/x/text v0.41.0 ) require github.com/andybalholm/cascadia v1.3.3 // indirect diff --git a/plain_embed_test.go b/plain_embed_test.go index 984eb63..c41bdf2 100644 --- a/plain_embed_test.go +++ b/plain_embed_test.go @@ -24,8 +24,8 @@ func TestPlainEmbedDoesNotRenameParentTag(t *testing.T) { t.Fatal(err) } gen := &Generator{} - if err := gen.Plain(doc.Find("embed"), doc, &ZasData{}); err != nil { - t.Fatalf("Plain() error = %v, want nil", err) + if err := gen.plain(doc.Find("embed"), doc, &ZasData{}); err != nil { + t.Fatalf("plain() error = %v, want nil", err) } div := doc.Find("div") if div.Length() != 1 { @@ -47,8 +47,8 @@ func TestPlainEmbedInsertsEscapedText(t *testing.T) { t.Fatal(err) } gen := &Generator{} - if err := gen.Plain(doc.Find("embed"), doc, &ZasData{}); err != nil { - t.Fatalf("Plain() error = %v, want nil", err) + if err := gen.plain(doc.Find("embed"), doc, &ZasData{}); err != nil { + t.Fatalf("plain() error = %v, want nil", err) } div := doc.Find("div") if div.Children().Length() != 0 { @@ -71,8 +71,8 @@ func TestPlainEmbedExecutesTemplate(t *testing.T) { } gen := &Generator{} data := &ZasData{Path: "/about.html"} - if err := gen.Plain(doc.Find("embed"), doc, data); err != nil { - t.Fatalf("Plain() error = %v, want nil", err) + if err := gen.plain(doc.Find("embed"), doc, data); err != nil { + t.Fatalf("plain() error = %v, want nil", err) } if want, got := "path is /about.html", doc.Find("div").Text(); got != want { t.Fatalf("div.Text() = %q, want %q", got, want) @@ -90,10 +90,10 @@ func TestPlainEmbedRemovesEmbedTag(t *testing.T) { t.Fatal(err) } gen := &Generator{} - if err := gen.Plain(doc.Find("embed"), doc, &ZasData{}); err != nil { - t.Fatalf("Plain() error = %v, want nil", err) + if err := gen.plain(doc.Find("embed"), doc, &ZasData{}); err != nil { + t.Fatalf("plain() error = %v, want nil", err) } if doc.Find("embed").Length() != 0 { - t.Fatal("embed tag still present after Plain()") + t.Fatal("embed tag still present after plain()") } } -- 2.51.2