From d490839ea1504dba94c5c53b85d2da55ebf30375 Mon Sep 17 00:00:00 2001 From: argonui <92067588+argonui@users.noreply.github.com> Date: Sun, 2 Aug 2026 19:03:11 -0500 Subject: [PATCH] bundler: remove mutable Rootname global; derive+return root name UnbundleAll stored the detected entry-module name in a mutable package global (Rootname) while findNextBundledScript hardcoded "__root" in its terminating regex. The two disagreed, causing (a) silent loss of a bundle's alphabetically-last module whenever its entry was not named "__root", and (b) persistent cross-call contamination that corrupted the entry line emitted by later Bundle calls. Rootname is now an immutable const (the canonical entry name and default). UnbundleAll detects the actual entry name locally, threads it into the scan regex, and returns it; UnbundleAllXML returns the canonical name so Lua and XML unbundling stay uniform. Callers (Unbundle, the shared handler, and tests) use the returned name instead of the global. Adds bundler tests reproducing both failure modes with a foreign entry name. Fixes #95 Co-Authored-By: Claude Opus 5 --- bundler/luabundler.go | 56 +++++++++++++++++------------ bundler/luabundler_test.go | 72 +++++++++++++++++++++++++++++++++++++- bundler/xmlbundler.go | 11 +++--- bundler/xmlbundler_test.go | 2 +- handler/bundlehandler.go | 8 ++--- tests/e2e_test.go | 4 +-- 6 files changed, 119 insertions(+), 34 deletions(-) diff --git a/bundler/luabundler.go b/bundler/luabundler.go index 6271257..8b18cb1 100644 --- a/bundler/luabundler.go +++ b/bundler/luabundler.go @@ -59,10 +59,12 @@ end)(nil)` funcsuffix string = `end)` ) -// Rootname is the bundle name for the batch of raw lua -var ( - Rootname string = `__root` -) +// Rootname is the canonical bundle name for the entry-point module. luabundle +// output produced by this package always uses it, and it is the default assumed +// when a bundle does not declare a different entry. It is an immutable constant: +// the actual entry name of a foreign bundle is detected per call and returned by +// UnbundleAll, never stored globally. +const Rootname string = `__root` // IsBundled keeps regex bundling logic to this file func IsBundled(rawlua string) bool { @@ -79,7 +81,7 @@ func AnalyzeBundle(rawlua string, log func(s string, a ...interface{})) { log("script is not bundled\n") return } - results, err := UnbundleAll(rawlua) + results, _, err := UnbundleAll(rawlua) if err != nil { log("Couldn't unbundle to analyze: %v", err) return @@ -89,37 +91,47 @@ func AnalyzeBundle(rawlua string, log func(s string, a ...interface{})) { } } -// UnbundleAll takes luacode generates all bundlenames and bundles -func UnbundleAll(rawlua string) (map[string]string, error) { +// UnbundleAll takes luacode and returns every registered module keyed by name, +// alongside the name of the entry-point ("root") module. The root name is +// detected locally from the bundle's `return __bundle_require("...")` line rather +// than read from or written to package state, so concurrent or repeated calls +// cannot contaminate one another. When the input is not bundled it is returned +// as the sole module under the canonical Rootname. +func UnbundleAll(rawlua string) (map[string]string, string, error) { if !IsBundled(rawlua) { - return map[string]string{Rootname: rawlua}, nil + return map[string]string{Rootname: rawlua}, Rootname, nil } - newRootInd := regexp.MustCompile(`__bundle_require\(".*"\)`).FindStringIndex(rawlua) - if newRootInd != nil { - Rootname = rawlua[newRootInd[0]+18 : newRootInd[1]-2] + rootName := Rootname + if m := regexp.MustCompile(`__bundle_require\("(.*)"\)`).FindStringSubmatch(rawlua); m != nil { + rootName = m[1] } scripts := map[string]string{} - r, err := findNextBundledScript(rawlua) + r, err := findNextBundledScript(rawlua, rootName) for r.leftover != "" { if err != nil { - return nil, fmt.Errorf("findNextBundledScript(%s): %v", rawlua, err) + return nil, "", fmt.Errorf("findNextBundledScript(%s): %v", rawlua, err) } scripts[r.name] = r.body - r, err = findNextBundledScript(r.leftover) + r, err = findNextBundledScript(r.leftover, rootName) } - if _, ok := scripts[Rootname]; !ok { - return nil, fmt.Errorf("Failed to find root bundle") + if _, ok := scripts[rootName]; !ok { + return nil, "", fmt.Errorf("Failed to find root bundle") } - return scripts, nil + return scripts, rootName, nil } type result struct { name, body, leftover string } -func findNextBundledScript(rawlua string) (result, error) { - root := regexp.MustCompile(`(?s)__bundle_register\("(.*?)", function\(require, _LOADED, __bundle_register, __bundle_modules\)[\r\n\s]+(.*?)[\r\n ]+end\)[\n\r]+(return __bundle_require\(\"__root\"\)|__bundle_register)+`) +func findNextBundledScript(rawlua, rootName string) (result, error) { + // The final module in a bundle is terminated by the entry line + // `return __bundle_require("")`; every earlier module is + // terminated by the next `__bundle_register`. Deriving the terminator from + // the detected root name (rather than hardcoding "__root") is what lets a + // bundle with a foreign entry name be scanned without losing its last module. + root := regexp.MustCompile(`(?s)__bundle_register\("(.*?)", function\(require, _LOADED, __bundle_register, __bundle_modules\)[\r\n\s]+(.*?)[\r\n ]+end\)[\n\r]+(return __bundle_require\("` + regexp.QuoteMeta(rootName) + `"\)|__bundle_register)+`) m := root.FindStringSubmatchIndex(rawlua) if m == nil { return result{}, nil @@ -140,13 +152,13 @@ func findNextBundledScript(rawlua string) (result, error) { // Unbundle extracts the root bundle per func Unbundle(rawlua string) (string, error) { - srcmap, err := UnbundleAll(rawlua) + srcmap, rootName, err := UnbundleAll(rawlua) if err != nil { return "", err } - rt, ok := srcmap[Rootname] + rt, ok := srcmap[rootName] if !ok { - return "", fmt.Errorf("Rootname not found in unbundled map") + return "", fmt.Errorf("root module %q not found in unbundled map", rootName) } return rt, nil } diff --git a/bundler/luabundler_test.go b/bundler/luabundler_test.go index 1fbb38e..5e104ef 100644 --- a/bundler/luabundler_test.go +++ b/bundler/luabundler_test.go @@ -2,6 +2,7 @@ package bundler import ( "fmt" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -253,10 +254,13 @@ var b = '3' end) return __bundle_require("__root") ` - got, err := UnbundleAll(raw) + got, gotRoot, err := UnbundleAll(raw) if err != nil { t.Fatalf("expected no err, got %v", err) } + if gotRoot != Rootname { + t.Errorf("want root %q, got %q", Rootname, gotRoot) + } want := map[string]string{ Rootname: `require("core/AgendaDeck-other.foo") var a = '2' @@ -268,6 +272,72 @@ require("core/AgendaDeck-other.foo")`, } } +// foreignEntryBundle is a luabundle whose entry module is named "custom_entry" +// rather than the usual "__root". Its alphabetically-last module, "zzz", is the +// final registered block, so it is the block terminated by the entry line +// `return __bundle_require("custom_entry")`. +var foreignEntryBundle = metaprefix + "\n" + + `__bundle_register("custom_entry", function(require, _LOADED, __bundle_register, __bundle_modules) +require("zzz") +end) +__bundle_register("zzz", function(require, _LOADED, __bundle_register, __bundle_modules) +zzz_value = 1 +end) +return __bundle_require("custom_entry")` + +// TestUnbundleForeignEntryName covers issue #95 failure mode (a): a bundle whose +// entry module is not "__root" used to silently drop its alphabetically-last +// module, because the scan regex hardcoded "__root" in its trailing alternation. +func TestUnbundleForeignEntryName(t *testing.T) { + got, gotRoot, err := UnbundleAll(foreignEntryBundle) + if err != nil { + t.Fatalf("expected no err, got %v", err) + } + if gotRoot != "custom_entry" { + t.Errorf("expected detected root %q, got %q", "custom_entry", gotRoot) + } + want := map[string]string{ + "custom_entry": `require("zzz")`, + // "zzz" must survive: it is the alphabetically-last module and was the + // one dropped before the fix. + "zzz": "zzz_value = 1", + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("want != got:\n%v\n", diff) + } +} + +// TestNoCrossCallContamination covers issue #95 failure mode (b): unbundling a +// bundle with a foreign entry name used to rewrite a mutable package global, +// corrupting the entry name emitted by any later Bundle call. Now that the root +// name is derived locally and returned, a subsequent Bundle is unaffected. +func TestNoCrossCallContamination(t *testing.T) { + // This call rewrote the global Rootname in the buggy implementation. + if _, _, err := UnbundleAll(foreignEntryBundle); err != nil { + t.Fatalf("unbundling foreign-entry bundle: %v", err) + } + + fr := &fakeLuaReader{fs: map[string]string{"dep.ttslua": "dep_value = 2"}} + got, err := Bundle(`require("dep")`, fr) + if err != nil { + t.Fatalf("expected no err, got %v", err) + } + if !strings.Contains(got, `__bundle_register("__root",`) { + t.Errorf("later Bundle output did not register a \"__root\" module:\n%s", got) + } + if !strings.Contains(got, `return __bundle_require("__root")`) { + t.Errorf("later Bundle output did not use \"__root\" as its entry line:\n%s", got) + } + // The bundle must round-trip: its entry must resolve to a registered module. + back, backRoot, err := UnbundleAll(got) + if err != nil { + t.Fatalf("re-unbundling later Bundle output: %v", err) + } + if _, ok := back[backRoot]; !ok { + t.Errorf("entry module %q not registered in re-bundled output", backRoot) + } +} + // This is still being non-deterministic func DisabledTestSmartBundle(t *testing.T) { fr := &fakeLuaReader{ diff --git a/bundler/xmlbundler.go b/bundler/xmlbundler.go index 54cb43b..b859f52 100644 --- a/bundler/xmlbundler.go +++ b/bundler/xmlbundler.go @@ -59,8 +59,11 @@ func indentString(s string, indent string) string { return strings.Join(final, "\n") } -// UnbundleAllXML converts a bundled xml file to mapping of filenames to contents -func UnbundleAllXML(rawxml string) (map[string]string, error) { +// UnbundleAllXML converts a bundled xml file to mapping of filenames to +// contents, alongside the name of the entry-point ("root") module. XML has no +// per-bundle entry declaration, so the root is always the canonical Rootname; +// it is returned so callers can treat Lua and XML unbundling uniformly. +func UnbundleAllXML(rawxml string) (map[string]string, string, error) { type inc struct { name string start int @@ -98,10 +101,10 @@ func UnbundleAllXML(rawxml string) (map[string]string, error) { } } if len(stack) != 0 { - return nil, fmt.Errorf("Bundled xml left after finished reading file: %v", stack) + return nil, "", fmt.Errorf("Bundled xml left after finished reading file: %v", stack) } store[Rootname] = unindentAndJoin(xmlarray, "") - return store, nil + return store, Rootname, nil } func unindentAndJoin(raw []string, indent string) string { diff --git a/bundler/xmlbundler_test.go b/bundler/xmlbundler_test.go index 0f7ed83..d3fd1e0 100644 --- a/bundler/xmlbundler_test.go +++ b/bundler/xmlbundler_test.go @@ -62,7 +62,7 @@ func TestUnbundleXML(t *testing.T) { `, } - got, err := UnbundleAllXML(input) + got, _, err := UnbundleAllXML(input) if err != nil { t.Fatalf("UnbundleAllXML(): %v", err) } diff --git a/handler/bundlehandler.go b/handler/bundlehandler.go index 4bca277..90644ca 100644 --- a/handler/bundlehandler.go +++ b/handler/bundlehandler.go @@ -17,7 +17,7 @@ type Handler struct { key, keypath, extension string bundle func(string, file.TextReader) (string, error) - unbundle func(string) (map[string]string, error) + unbundle func(string) (map[string]string, string, error) } // NewLuaHandler fills in relevant info for lua bundling @@ -102,12 +102,12 @@ func (h *Handler) WhileWritingToFile(rawj map[string]interface{}, possiblefname h.key, rawscript, rawscript) } - allScripts, err := h.unbundle(script) + allScripts, rootName, err := h.unbundle(script) if err != nil { return HandleAction{}, fmt.Errorf("UnbundleAll(...): %v", err) } // root bundle is promised to exist - rootscript, _ := allScripts[bundler.Rootname] + rootscript, _ := allScripts[rootName] returnAction := HandleAction{Noop: false} if len(rootscript) > 80 { err = h.DefaultWriter.EncodeToFile(rootscript, possiblefname) @@ -120,7 +120,7 @@ func (h *Handler) WhileWritingToFile(rawj map[string]interface{}, possiblefname returnAction.Key = h.key returnAction.Value = rootscript } - delete(allScripts, bundler.Rootname) + delete(allScripts, rootName) for k, script := range allScripts { fname := k diff --git a/tests/e2e_test.go b/tests/e2e_test.go index 183890a..5957235 100644 --- a/tests/e2e_test.go +++ b/tests/e2e_test.go @@ -114,11 +114,11 @@ func TestAllReverseThenBuild(t *testing.T) { if !ok { t.Fatalf("non string found in luascript, found %T", gls) } - wantBundles, err := bundler.UnbundleAll(wlss) + wantBundles, _, err := bundler.UnbundleAll(wlss) if err != nil { t.Fatalf("unbundle want : %v", err) } - gotBundles, err := bundler.UnbundleAll(glss) + gotBundles, _, err := bundler.UnbundleAll(glss) if err != nil { t.Fatalf("unbundle got : %v", err) }