From 88c7401606a80184ae5fc7ff6556f2bfc03d4f35 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 21:15:40 +0000 Subject: [PATCH] fix: escape '/' inside folder names in FOLDER paths (mendixlabs/mxcli#1367) Studio Pro allows '/' in a folder name, but BuildFolderPath joined names with a bare '/' and every folder-path walker split on it. A constant in `Private - String en/de-cryption` > `Apis` was described as folder 'Private - String en/de-cryption/Apis', and replaying the --save-edits output with exec filed it three folders deep, exit 0. New package mdl/folderpath: a '/' in a segment is written `\/`, a '\' `\\`; Split is lenient (a backslash before anything else stays literal), so existing paths keep their meaning. BuildFolderPath joins with it, resolveFolder / findFolderByPath / lookupFolder / the page builder and the project tree split with it. Describers that wrote `folder '%s'` unquoted now use mdlQuote, so the escape (and apostrophes) survive both string rules. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Cj4DQ8PRitHdtF3q8pVVBQ --- ...containing-slash-split-nested-folders.json | 1 + cmd/mxcli/project_tree.go | 24 ++---- .../reference/organization/create-folder.md | 16 ++++ .../1367-folder-name-containing-slash.mdl | 38 +++++++++ mdl/executor/cmd_constants.go | 7 +- mdl/executor/cmd_export_mappings.go | 2 +- mdl/executor/cmd_folders.go | 6 +- mdl/executor/cmd_import_mappings.go | 2 +- mdl/executor/cmd_jsonstructures.go | 2 +- .../cmd_messagedefinition_documents.go | 2 +- mdl/executor/cmd_messagedefinitions.go | 2 +- mdl/executor/cmd_microflows_build.go | 6 +- mdl/executor/cmd_odata.go | 2 +- mdl/executor/cmd_pages_builder.go | 8 +- mdl/executor/cmd_published_rest.go | 2 +- mdl/executor/cmd_rest_clients.go | 2 +- mdl/executor/document_placement.go | 4 +- mdl/executor/folder_path_slash_test.go | 83 +++++++++++++++++++ mdl/executor/helpers.go | 8 +- mdl/executor/hierarchy.go | 9 +- mdl/folderpath/folderpath.go | 78 +++++++++++++++++ mdl/folderpath/folderpath_test.go | 49 +++++++++++ 22 files changed, 293 insertions(+), 60 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-10-09-folder-name-containing-slash-split-nested-folders.json create mode 100644 mdl-examples/bug-tests/1367-folder-name-containing-slash.mdl create mode 100644 mdl/executor/folder_path_slash_test.go create mode 100644 mdl/folderpath/folderpath.go create mode 100644 mdl/folderpath/folderpath_test.go diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-folder-name-containing-slash-split-nested-folders.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-folder-name-containing-slash-split-nested-folders.json new file mode 100644 index 0000000000..a89896a626 --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-10-09-folder-name-containing-slash-split-nested-folders.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-10-09", "symptom": "marketplace update --save-edits + exec corrupts constant folder paths containing a literal '/' in the folder name: EncryptionKey in `Private - String en/de-cryption` > `Apis` was replayed into `Private - String en` > `de-cryption` > `Apis`; exec exited 0.", "cause": "ContainerHierarchy.BuildFolderPath joined folder names with a bare '/', and every path walker (resolveFolder, findFolderByPath, lookupFolder, the page builder's) split on it. Studio Pro does not reserve '/' in a folder name, so the path was ambiguous. Fix: mdl/folderpath escapes '/' as `\\/` and '\\' as `\\\\` per segment and splits leniently (a backslash before anything else stays literal); the describers quote the clause with mdlQuote so the escape survives both string rules.", "file": "mdl/folderpath/folderpath.go; mdl/executor/hierarchy.go (BuildFolderPath); helpers.go, cmd_folders.go, cmd_microflows_build.go, cmd_pages_builder.go (walkers)", "insight": "The reported doctype (constant) was not the blast radius: BuildFolderPath feeds every describer and four separate split-on-'/' walkers. Fix the join and split once and grep `Split(.*folder.*\"/\")` for the walkers. A serialised path needs an escape that is not a doubled separator (`a///b` is ambiguous) and that reads the same under mdl 0 and mdl 1 string rules: `\\/` does, because mdl 0 keeps a backslash before an unknown character. Several describers also wrote `folder '%s'` without doubling apostrophes - the same round-trip gap, now mdlQuote.", "refs": ["mendixlabs/mxcli#1367"], "test": "mdl/executor/folder_path_slash_test.go TestDescribeExecRoundTrip_FolderNameContainingSlash; mdl/folderpath/folderpath_test.go"} diff --git a/cmd/mxcli/project_tree.go b/cmd/mxcli/project_tree.go index 6a8c8d6474..b6a6bdf1ce 100644 --- a/cmd/mxcli/project_tree.go +++ b/cmd/mxcli/project_tree.go @@ -10,6 +10,7 @@ import ( modelsdkbackend "github.com/mendixlabs/mxcli/mdl/backend/modelsdk" "github.com/mendixlabs/mxcli/mdl/executor" + "github.com/mendixlabs/mxcli/mdl/folderpath" "github.com/mendixlabs/mxcli/mdl/types" "github.com/mendixlabs/mxcli/model" "github.com/spf13/cobra" @@ -820,7 +821,7 @@ func getOrCreateFolder(root *TreeNode, cache map[string]*TreeNode, path string) if builtPath != "" { builtPath += "/" } - builtPath += part + builtPath += folderpath.Escape(part) if node, ok := cache[builtPath]; ok { current = node @@ -1057,23 +1058,8 @@ func buildMenuTreeNodes(parent *TreeNode, items []*types.NavMenuItem) { } } -// splitFolderPath splits a folder path like "Parent/Child" into parts. +// splitFolderPath splits a folder path like "Parent/Child" into folder names, +// reading a `\/` inside a name as part of it (mendixlabs/mxcli#1367). func splitFolderPath(path string) []string { - if path == "" { - return nil - } - var parts []string - start := 0 - for i := 0; i < len(path); i++ { - if path[i] == '/' { - if i > start { - parts = append(parts, path[start:i]) - } - start = i + 1 - } - } - if start < len(path) { - parts = append(parts, path[start:]) - } - return parts + return folderpath.Split(path) } diff --git a/docs-site/src/reference/organization/create-folder.md b/docs-site/src/reference/organization/create-folder.md index 7323340aa2..3d934fc53e 100644 --- a/docs-site/src/reference/organization/create-folder.md +++ b/docs-site/src/reference/organization/create-folder.md @@ -55,6 +55,22 @@ CREATE PAGE MyModule.Order_Edit FOLDER 'Orders' }; ``` +### A folder whose name contains `/` + +Studio Pro allows `/` inside a folder name, so in a `FOLDER '…'` path a slash +that belongs to the name is written `\/` (and a backslash `\\`). This is one +folder, `Private - String en/de-cryption`, holding `Apis`: + +```sql +CREATE CONSTANT Encryption.EncryptionKey FOLDER 'Private - String en\/de-cryption/Apis' ( + Type: String, + DefaultValue: '' +); +``` + +`DESCRIBE` writes the escape for you, so its output files the document back +where it was. A backslash before any other character is an ordinary character. + ## See Also [CREATE MODULE](create-module.md), [DROP FOLDER](drop-folder.md), [MOVE](move.md) diff --git a/mdl-examples/bug-tests/1367-folder-name-containing-slash.mdl b/mdl-examples/bug-tests/1367-folder-name-containing-slash.mdl new file mode 100644 index 0000000000..e1d85bdf16 --- /dev/null +++ b/mdl-examples/bug-tests/1367-folder-name-containing-slash.mdl @@ -0,0 +1,38 @@ +mdl 1; +-- ============================================================================ +-- A folder name containing '/' split into nested folders (mendixlabs/mxcli#1367) +-- ============================================================================ +-- +-- Symptom (before fix): "marketplace update --save-edits + exec corrupts +-- constant folder paths containing a literal '/' in the folder name". +-- The Encryption module keeps EncryptionKey in the Studio Pro folder +-- `Private - String en/de-cryption` > `Apis`. DESCRIBE (which --save-edits +-- writes) joined the folder names with a bare '/': +-- +-- create or modify constant Encryption.EncryptionKey folder 'Private - String en/de-cryption/Apis' ( +-- +-- and replaying it filed the constant in `Private - String en` > +-- `de-cryption` > `Apis`. exec exited 0. +-- +-- Fix: inside a folder-path segment a '/' is written `\/` (a backslash `\\`); +-- the path walkers read it back as part of the name (mdl/folderpath). +-- +-- Usage: +-- mxcli exec mdl-examples/bug-tests/1367-folder-name-containing-slash.mdl -p app.mpr +-- mxcli -p app.mpr -c "list folders in BugTest1367" +-- Expect two folders: `Private - String en\/de-cryption` and +-- `Private - String en\/de-cryption/Apis` (holding EncryptionKey), not three. +-- ============================================================================ + +create module BugTest1367; + +create or modify constant BugTest1367.EncryptionKey folder 'Private - String en\/de-cryption/Apis' ( + Type: String, + DefaultValue: '' +); + +-- Re-running the statement must find the same folder, not create a sibling. +create or modify constant BugTest1367.EncryptionKey folder 'Private - String en\/de-cryption/Apis' ( + Type: String, + DefaultValue: '' +); diff --git a/mdl/executor/cmd_constants.go b/mdl/executor/cmd_constants.go index abbfcdb4e5..f49614fc75 100644 --- a/mdl/executor/cmd_constants.go +++ b/mdl/executor/cmd_constants.go @@ -124,12 +124,7 @@ func outputConstantMDL(ctx *ExecContext, c *model.Constant, moduleName string) e } } // The folder is a clause right after the name (R9). - folder := "" - if h, _ := getHierarchy(ctx); h != nil { - if folderPath := h.BuildFolderPath(c.ContainerID); folderPath != "" { - folder = fmt.Sprintf(" folder '%s'", strings.ReplaceAll(folderPath, "'", "''")) - } - } + folder := describeFolderClause(ctx, c.ContainerID) // The properties are a ( Key: value ) list with Studio Pro's names // (phase 3.6, ako/mxcli#755); ExposedToClient is omitted at its default. fmt.Fprintf(ctx.Output, "create or modify constant %s.%s%s (\n", moduleName, c.Name, folder) diff --git a/mdl/executor/cmd_export_mappings.go b/mdl/executor/cmd_export_mappings.go index 6cbbdd5e1e..35fc037e7c 100644 --- a/mdl/executor/cmd_export_mappings.go +++ b/mdl/executor/cmd_export_mappings.go @@ -115,7 +115,7 @@ func describeExportMapping(ctx *ExecContext, name ast.QualifiedName) error { // Without this the description round-trips to the module root: replaying // it in a fresh project would recreate the mapping unfiled (#932). if folderPath := h.BuildFolderPath(em.ContainerID); folderPath != "" { - fmt.Fprintf(ctx.Output, " folder '%s'\n", folderPath) + fmt.Fprintf(ctx.Output, " folder %s\n", mdlQuote(ctx, folderPath)) } if em.JsonStructure != "" { diff --git a/mdl/executor/cmd_folders.go b/mdl/executor/cmd_folders.go index 05c08403b6..aae3047a41 100644 --- a/mdl/executor/cmd_folders.go +++ b/mdl/executor/cmd_folders.go @@ -10,20 +10,18 @@ import ( "github.com/mendixlabs/mxcli/mdl/ast" mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" + "github.com/mendixlabs/mxcli/mdl/folderpath" "github.com/mendixlabs/mxcli/mdl/types" "github.com/mendixlabs/mxcli/model" ) // findFolderByPath walks a folder path under a module and returns the folder ID. func findFolderByPath(ctx *ExecContext, moduleID model.ID, folderPath string, folders []*types.FolderInfo) (model.ID, error) { - parts := strings.Split(folderPath, "/") + parts := folderpath.Split(folderPath) currentContainerID := moduleID var targetFolderID model.ID for i, part := range parts { - if part == "" { - continue - } var found bool for _, f := range folders { diff --git a/mdl/executor/cmd_import_mappings.go b/mdl/executor/cmd_import_mappings.go index 7afa08bb35..64a1be8a5b 100644 --- a/mdl/executor/cmd_import_mappings.go +++ b/mdl/executor/cmd_import_mappings.go @@ -115,7 +115,7 @@ func describeImportMapping(ctx *ExecContext, name ast.QualifiedName) error { // Without this the description round-trips to the module root: replaying // it in a fresh project would recreate the mapping unfiled (#932). if folderPath := h.BuildFolderPath(im.ContainerID); folderPath != "" { - fmt.Fprintf(ctx.Output, " folder '%s'\n", folderPath) + fmt.Fprintf(ctx.Output, " folder %s\n", mdlQuote(ctx, folderPath)) } if im.JsonStructure != "" { diff --git a/mdl/executor/cmd_jsonstructures.go b/mdl/executor/cmd_jsonstructures.go index 61744dd42c..d351fe1c2b 100644 --- a/mdl/executor/cmd_jsonstructures.go +++ b/mdl/executor/cmd_jsonstructures.go @@ -101,7 +101,7 @@ func describeJsonStructure(ctx *ExecContext, name ast.QualifiedName) error { // Re-executable CREATE OR MODIFY statement fmt.Fprintf(ctx.Output, "create or modify json structure %s", qualifiedName) if folderPath := h.BuildFolderPath(js.ContainerID); folderPath != "" { - fmt.Fprintf(ctx.Output, "\n folder '%s'", folderPath) + fmt.Fprintf(ctx.Output, "\n folder %s", mdlQuote(ctx, folderPath)) } if docClause { fmt.Fprintf(ctx.Output, "\n comment '%s'", strings.ReplaceAll(js.Documentation, "'", "''")) diff --git a/mdl/executor/cmd_messagedefinition_documents.go b/mdl/executor/cmd_messagedefinition_documents.go index b5297662f1..78f0e3ba95 100644 --- a/mdl/executor/cmd_messagedefinition_documents.go +++ b/mdl/executor/cmd_messagedefinition_documents.go @@ -230,7 +230,7 @@ func describeMessageDefinitionDocument(ctx *ExecContext, d *model.MessageDefinit fmt.Fprintf(ctx.Output, "create or modify message definition %s\n", messageDocumentQN(ctx, d)) if h, err := getHierarchy(ctx); err == nil { if folder := h.BuildFolderPath(d.ContainerID); folder != "" { - fmt.Fprintf(ctx.Output, " folder '%s'\n", folder) + fmt.Fprintf(ctx.Output, " folder %s\n", mdlQuote(ctx, folder)) } } if d.Root == nil { diff --git a/mdl/executor/cmd_messagedefinitions.go b/mdl/executor/cmd_messagedefinitions.go index 3b5f18eb5b..2c236f743d 100644 --- a/mdl/executor/cmd_messagedefinitions.go +++ b/mdl/executor/cmd_messagedefinitions.go @@ -591,7 +591,7 @@ func execDescribeMessageDefinitionCollection(ctx *ExecContext, name ast.Qualifie fmt.Fprintf(ctx.Output, "create or modify message definition collection %s\n", name.String()) if h, err := getHierarchy(ctx); err == nil { if folder := h.BuildFolderPath(c.ContainerID); folder != "" { - fmt.Fprintf(ctx.Output, " folder '%s'\n", folder) + fmt.Fprintf(ctx.Output, " folder %s\n", mdlQuote(ctx, folder)) } } fmt.Fprintln(ctx.Output, "{") diff --git a/mdl/executor/cmd_microflows_build.go b/mdl/executor/cmd_microflows_build.go index ceab77f12f..ced93a3672 100644 --- a/mdl/executor/cmd_microflows_build.go +++ b/mdl/executor/cmd_microflows_build.go @@ -10,6 +10,7 @@ import ( "github.com/mendixlabs/mxcli/mdl/ast" mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" + "github.com/mendixlabs/mxcli/mdl/folderpath" "github.com/mendixlabs/mxcli/mdl/types" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/sdk/microflows" @@ -778,10 +779,7 @@ func lookupFolder(ctx *ExecContext, moduleID model.ID, folderPath string) (model return "", false } current := moduleID - for _, part := range strings.Split(folderPath, "/") { - if part == "" { - continue - } + for _, part := range folderpath.Split(folderPath) { found := false for _, f := range folders { if f.ContainerID == current && f.Name == part { diff --git a/mdl/executor/cmd_odata.go b/mdl/executor/cmd_odata.go index 362b442a44..54f5672d93 100644 --- a/mdl/executor/cmd_odata.go +++ b/mdl/executor/cmd_odata.go @@ -338,7 +338,7 @@ func outputPublishedODataServiceMDL(ctx *ExecContext, svc *model.PublishedODataS // The folder is a clause after the name (R9); `Folder:` is its alias. folder := "" if folderPath != "" { - folder = " folder " + mdlQuoted(folderPath) + folder = " folder " + mdlQuote(ctx, folderPath) } fmt.Fprintf(ctx.Output, "create or modify published odata service %s.%s%s (\n", moduleName, svc.Name, folder) diff --git a/mdl/executor/cmd_pages_builder.go b/mdl/executor/cmd_pages_builder.go index 363fa5fb62..5f98064671 100644 --- a/mdl/executor/cmd_pages_builder.go +++ b/mdl/executor/cmd_pages_builder.go @@ -12,6 +12,7 @@ import ( "github.com/mendixlabs/mxcli/mdl/ast" "github.com/mendixlabs/mxcli/mdl/backend" mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" + "github.com/mendixlabs/mxcli/mdl/folderpath" "github.com/mendixlabs/mxcli/mdl/types" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/sdk/domainmodel" @@ -350,14 +351,9 @@ func (pb *pageBuilder) resolveFolder(folderPath string) (model.ID, error) { return "", mdlerrors.NewBackend("list folders", err) } - // Split path into parts - parts := strings.Split(folderPath, "/") currentContainerID := pb.moduleID - for _, part := range parts { - if part == "" { - continue - } + for _, part := range folderpath.Split(folderPath) { // Find folder with this name under current container var foundFolder *types.FolderInfo diff --git a/mdl/executor/cmd_published_rest.go b/mdl/executor/cmd_published_rest.go index 6d2f634bb5..b2ebd4428b 100644 --- a/mdl/executor/cmd_published_rest.go +++ b/mdl/executor/cmd_published_rest.go @@ -103,7 +103,7 @@ func describePublishedRestService(ctx *ExecContext, name ast.QualifiedName) erro // The folder is a clause after the name (R9); `Folder:` is its alias. folder := "" if folderPath := h.BuildFolderPath(svc.ContainerID); folderPath != "" { - folder = " folder " + mdlQuoted(folderPath) + folder = " folder " + mdlQuote(ctx, folderPath) } fmt.Fprintf(ctx.Output, "create or modify published rest service %s%s (\n", qualifiedName, folder) fmt.Fprintf(ctx.Output, " Path: %s", mdlQuoted(svc.Path)) diff --git a/mdl/executor/cmd_rest_clients.go b/mdl/executor/cmd_rest_clients.go index 14c40e1006..503d16db6f 100644 --- a/mdl/executor/cmd_rest_clients.go +++ b/mdl/executor/cmd_rest_clients.go @@ -125,7 +125,7 @@ func outputConsumedRestServiceMDL(ctx *ExecContext, svc *model.ConsumedRestServi folder := "" if h, err := getHierarchy(ctx); err == nil && h != nil { if folderPath := h.BuildFolderPath(svc.ContainerID); folderPath != "" { - folder = " folder " + mdlQuoted(folderPath) + folder = " folder " + mdlQuote(ctx, folderPath) } } fmt.Fprintf(w, "create or modify consumed rest service %s.%s%s (\n", moduleName, svc.Name, folder) diff --git a/mdl/executor/document_placement.go b/mdl/executor/document_placement.go index 443943de5f..0941374d72 100644 --- a/mdl/executor/document_placement.go +++ b/mdl/executor/document_placement.go @@ -16,8 +16,6 @@ package executor import ( - "strings" - mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" "github.com/mendixlabs/mxcli/model" ) @@ -62,7 +60,7 @@ func describeFolderClause(ctx *ExecContext, containerID model.ID) string { if path == "" { return "" } - return " folder '" + strings.ReplaceAll(path, "'", "''") + "'" + return " folder " + mdlQuote(ctx, path) } // containerForDocument picks the container a CREATE OR MODIFY should use, in diff --git a/mdl/executor/folder_path_slash_test.go b/mdl/executor/folder_path_slash_test.go new file mode 100644 index 0000000000..97a31873e8 --- /dev/null +++ b/mdl/executor/folder_path_slash_test.go @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "reflect" + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/mdl/types" + "github.com/mendixlabs/mxcli/mdl/visitor" + "github.com/mendixlabs/mxcli/model" +) + +// mendixlabs/mxcli#1367: "marketplace update --save-edits + exec corrupts +// constant folder paths containing a literal '/' in the folder name". The +// Encryption module keeps EncryptionKey in `Private - String en/de-cryption` +// › `Apis`; the saved edit replayed it into `Private - String en` › +// `de-cryption` › `Apis`, and exec exited 0. +// +// --save-edits writes DESCRIBE output, so the test is the round trip itself: +// describe the constant, parse what was written, resolve its folder in a +// project that does not have it yet, and compare the folders created with the +// ones described. +func TestDescribeExecRoundTrip_FolderNameContainingSlash(t *testing.T) { + mod := mkModule("Encryption") + outer := nextID("fld") + inner := nextID("fld") + c := mkConstant(inner, "EncryptionKey", "String", "") + + h := mkHierarchy(mod) + withContainer(h, outer, mod.ID) + withContainer(h, inner, outer) + h.folderNames[outer] = "Private - String en/de-cryption" + h.folderNames[inner] = "Apis" + + mb := &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + ListConstantsFunc: func() ([]*model.Constant, error) { return []*model.Constant{c}, nil }, + } + ctx, buf := newMockCtx(t, withBackend(mb), withHierarchy(h)) + assertNoError(t, describeConstant(ctx, ast.QualifiedName{Module: "Encryption", Name: "EncryptionKey"})) + + prog, errs := visitor.Build(buf.String()) + if len(errs) > 0 { + t.Fatalf("describe output does not parse: %v\n%s", errs, buf.String()) + } + var folder string + for _, s := range prog.Statements { + if cs, ok := s.(*ast.CreateConstantStmt); ok { + folder = cs.Folder + } + } + if folder == "" { + t.Fatalf("no folder clause in the describe output:\n%s", buf.String()) + } + + var probe placementProbe + fresh, _ := folderMockBackend(t, mod, &probe) + var created []*types.FolderInfo + fresh.ListFoldersFunc = func() ([]*types.FolderInfo, error) { return created, nil } + fresh.CreateFolderFunc = func(f *model.Folder) error { + created = append(created, &types.FolderInfo{ID: f.ID, ContainerID: f.ContainerID, Name: f.Name}) + return nil + } + ctx2, _ := newMockCtx(t, withBackend(fresh), withHierarchy(mkHierarchy(mod))) + if _, err := resolveFolder(ctx2, mod.ID, folder); err != nil { + t.Fatal(err) + } + + var names []string + for _, f := range created { + names = append(names, f.Name) + } + want := []string{"Private - String en/de-cryption", "Apis"} + if !reflect.DeepEqual(names, want) { + t.Errorf("folder clause %q recreated folders %q, want %q", folder, names, want) + } + if len(created) == 2 && (created[0].ContainerID != mod.ID || created[1].ContainerID != created[0].ID) { + t.Errorf("folders not nested as described: %+v", created) + } +} diff --git a/mdl/executor/helpers.go b/mdl/executor/helpers.go index d1b66fef07..d187cc6d1a 100644 --- a/mdl/executor/helpers.go +++ b/mdl/executor/helpers.go @@ -11,6 +11,7 @@ import ( "github.com/mendixlabs/mxcli/mdl/ast" mdlerrors "github.com/mendixlabs/mxcli/mdl/errors" + "github.com/mendixlabs/mxcli/mdl/folderpath" "github.com/mendixlabs/mxcli/mdl/types" "github.com/mendixlabs/mxcli/model" "github.com/mendixlabs/mxcli/sdk/domainmodel" @@ -113,14 +114,9 @@ func resolveFolder(ctx *ExecContext, moduleID model.ID, folderPath string) (mode return "", mdlerrors.NewBackend("list folders", err) } - // Split path into parts - parts := strings.Split(folderPath, "/") currentContainerID := moduleID - for _, part := range parts { - if part == "" { - continue - } + for _, part := range folderpath.Split(folderPath) { // Find folder with this name under current container var foundFolder *types.FolderInfo diff --git a/mdl/executor/hierarchy.go b/mdl/executor/hierarchy.go index 357c045c56..9762553dfa 100644 --- a/mdl/executor/hierarchy.go +++ b/mdl/executor/hierarchy.go @@ -3,9 +3,8 @@ package executor import ( - "strings" - "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/mdl/folderpath" "github.com/mendixlabs/mxcli/mdl/types" "github.com/mendixlabs/mxcli/model" ) @@ -95,7 +94,9 @@ func (h *ContainerHierarchy) IsModule(id model.ID) bool { return h.moduleIDs[id] } -// BuildFolderPath builds a folder path string from container to module. +// BuildFolderPath builds a folder path string from container to module, each +// folder name escaped so a name holding '/' stays one segment +// (folderpath.Join, mendixlabs/mxcli#1367). func (h *ContainerHierarchy) BuildFolderPath(containerID model.ID) string { var parts []string current := containerID @@ -115,7 +116,7 @@ func (h *ContainerHierarchy) BuildFolderPath(containerID model.ID) string { if len(parts) == 0 { return "" } - return strings.Join(parts, "/") + return folderpath.Join(parts) } // GetQualifiedName returns the fully qualified name for a document. diff --git a/mdl/folderpath/folderpath.go b/mdl/folderpath/folderpath.go new file mode 100644 index 0000000000..b369bee9f8 --- /dev/null +++ b/mdl/folderpath/folderpath.go @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Package folderpath writes and reads the folder path of an MDL FOLDER clause. +// +// A folder path names a chain of nested folders, "Private/Apis", with '/' as +// the separator. Studio Pro does not reserve '/' in a folder *name*, so a +// folder called "Private - String en/de-cryption" is one folder, and joining +// names with a bare '/' made it indistinguishable from two. DESCRIBE wrote the +// path that way and `exec` split it, so a describe → exec round trip filed the +// document two folders deeper than it was, with no warning (mendixlabs/mxcli#1367). +// +// Inside a segment a '/' is written as `\/` and a '\' as `\\`. Reading is +// lenient about the backslash: one followed by anything other than '/' or '\' +// is kept as written, so a hand-written `folder 'A\B'` still names the folder +// `A\B` exactly as it did before escapes existed, and only `\/` and `\\` +// change meaning. +// +// This escape is the path's, not the string literal's: the clause is still +// quoted with the describer's string rule (mdlQuote) on top of it. `\/` reads +// the same under both language versions' string rules, which is why the +// separator escape is a backslash and not, say, a doubled slash — `a///b` +// would not say which side the literal slash belongs to. +package folderpath + +import "strings" + +// Join renders folder names, outermost first, as one folder path. +func Join(names []string) string { + escaped := make([]string, len(names)) + for i, n := range names { + escaped[i] = Escape(n) + } + return strings.Join(escaped, "/") +} + +// Escape renders one folder name as a path segment. +func Escape(name string) string { + if !strings.ContainsAny(name, `/\`) { + return name + } + var b strings.Builder + b.Grow(len(name) + 2) + for i := 0; i < len(name); i++ { + if c := name[i]; c == '/' || c == '\\' { + b.WriteByte('\\') + } + b.WriteByte(name[i]) + } + return b.String() +} + +// Split reads a folder path back into its folder names, outermost first. +// Empty segments ("A//B", a leading or trailing '/') are dropped, as the path +// walkers always did. +func Split(path string) []string { + var parts []string + var cur strings.Builder + flush := func() { + if cur.Len() > 0 { + parts = append(parts, cur.String()) + cur.Reset() + } + } + for i := 0; i < len(path); i++ { + c := path[i] + switch { + case c == '\\' && i+1 < len(path) && (path[i+1] == '/' || path[i+1] == '\\'): + cur.WriteByte(path[i+1]) + i++ + case c == '/': + flush() + default: + cur.WriteByte(c) + } + } + flush() + return parts +} diff --git a/mdl/folderpath/folderpath_test.go b/mdl/folderpath/folderpath_test.go new file mode 100644 index 0000000000..dc13569001 --- /dev/null +++ b/mdl/folderpath/folderpath_test.go @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: Apache-2.0 + +package folderpath + +import ( + "reflect" + "testing" +) + +func TestJoinSplitRoundTrip(t *testing.T) { + for _, names := range [][]string{ + {"Private"}, + {"Private", "Apis"}, + // mendixlabs/mxcli#1367: one folder whose name holds a slash. + {"Private - String en/de-cryption", "Apis"}, + {`A\B`}, + {`ends\`, "next"}, + {`a\/b`}, + {"/", "//"}, + } { + path := Join(names) + if got := Split(path); !reflect.DeepEqual(got, names) { + t.Errorf("Split(Join(%q)) = %q via %q", names, got, path) + } + } +} + +func TestSplitKeepsUnescapedPathsAsBefore(t *testing.T) { + for in, want := range map[string][]string{ + "": nil, + "A/B": {"A", "B"}, + "/A//B/": {"A", "B"}, + `A\B`: {`A\B`}, // a backslash before an ordinary character is literal + `C:\temp/x`: {`C:\temp`, "x"}, + } { + if got := Split(in); !reflect.DeepEqual(got, want) { + t.Errorf("Split(%q) = %q, want %q", in, got, want) + } + } +} + +func TestJoinEscapesOnlyWhatItMust(t *testing.T) { + if got := Join([]string{"Private - String en/de-cryption", "Apis"}); got != `Private - String en\/de-cryption/Apis` { + t.Errorf("got %q", got) + } + if got := Join([]string{"Private", "Apis"}); got != "Private/Apis" { + t.Errorf("a path without slashes changed: %q", got) + } +}