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) }