Memoize completed native link dependencies
This commit is contained in:
@@ -263,6 +263,12 @@ Finding: native part of **C02**.
|
|||||||
Owners: `internal/engine/filesystem_links.go`, `filesystem_links_test.go`,
|
Owners: `internal/engine/filesystem_links.go`, `filesystem_links_test.go`,
|
||||||
`directory_links_test.go`, and glob link tests.
|
`directory_links_test.go`, and glob link tests.
|
||||||
|
|
||||||
|
Status: **complete** (native C02). Prior completed commits: `2736ddf`,
|
||||||
|
`deb9aa5`, `ae3408a`, `ea27f17`, `08a8c25`, `877c497`, `8aba43d`.
|
||||||
|
Validation passed: `env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'Link|Directories|Glob' -count=1`,
|
||||||
|
`env GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -count=1`,
|
||||||
|
and `git diff --check`.
|
||||||
|
|
||||||
1. Extract the existing portable frame evaluator into one private resolution
|
1. Extract the existing portable frame evaluator into one private resolution
|
||||||
operation with `active` and `completed` maps. A small set of operation-local
|
operation with `active` and `completed` maps. A small set of operation-local
|
||||||
functions supplies `Lstat`, link reading, and validated target normalization.
|
functions supplies `Lstat`, link reading, and validated target normalization.
|
||||||
|
|||||||
@@ -126,6 +126,69 @@ type importLinkFrame struct {
|
|||||||
link string
|
link string
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type importLinkOperations struct {
|
||||||
|
lstat func(string) (fs.FileInfo, error)
|
||||||
|
readlink func(string) (string, error)
|
||||||
|
normalizeTarget func(string, string) (string, error)
|
||||||
|
}
|
||||||
|
|
||||||
|
func resolveImportLinkPath(name string, operations importLinkOperations) (string, error) {
|
||||||
|
frames := []importLinkFrame{{pending: strings.Split(name, "/")}}
|
||||||
|
resolved := ""
|
||||||
|
active := make(map[string]struct{})
|
||||||
|
completed := make(map[string]string)
|
||||||
|
for len(frames) > 0 {
|
||||||
|
frame := &frames[len(frames)-1]
|
||||||
|
if len(frame.pending) == 0 {
|
||||||
|
if frame.link != "" {
|
||||||
|
if resolved == "" {
|
||||||
|
resolved = "."
|
||||||
|
}
|
||||||
|
completed[frame.link] = resolved
|
||||||
|
delete(active, frame.link)
|
||||||
|
}
|
||||||
|
frames = frames[:len(frames)-1]
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
segment := frame.pending[0]
|
||||||
|
frame.pending = frame.pending[1:]
|
||||||
|
candidate := path.Join(resolved, segment)
|
||||||
|
if candidate == "" {
|
||||||
|
candidate = "."
|
||||||
|
}
|
||||||
|
info, err := operations.lstat(candidate)
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
if info.Mode()&fs.ModeSymlink == 0 {
|
||||||
|
resolved = candidate
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if target, ok := completed[candidate]; ok {
|
||||||
|
resolved = target
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
if _, exists := active[candidate]; exists {
|
||||||
|
return "", errImportDirectoryLinkCycle
|
||||||
|
}
|
||||||
|
target, err := operations.readlink(candidate)
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
targetPath, err := operations.normalizeTarget(candidate, target)
|
||||||
|
if err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
active[candidate] = struct{}{}
|
||||||
|
frames = append(frames, importLinkFrame{pending: strings.Split(targetPath, "/"), link: candidate})
|
||||||
|
resolved = ""
|
||||||
|
}
|
||||||
|
if resolved == "" {
|
||||||
|
resolved = "."
|
||||||
|
}
|
||||||
|
return resolved, nil
|
||||||
|
}
|
||||||
|
|
||||||
func resolveHostImportPath(access *filesystemAccess, name string) (string, error) {
|
func resolveHostImportPath(access *filesystemAccess, name string) (string, error) {
|
||||||
frames := []importLinkFrame{{pending: strings.Split(filepath.ToSlash(name), "/")}}
|
frames := []importLinkFrame{{pending: strings.Split(filepath.ToSlash(name), "/")}}
|
||||||
resolved := ""
|
resolved := ""
|
||||||
@@ -189,43 +252,17 @@ func resolveNativeImportPath(access *filesystemAccess, filesystem fs.ReadLinkFS,
|
|||||||
if !fs.ValidPath(name) || !access.contains(name) {
|
if !fs.ValidPath(name) || !access.contains(name) {
|
||||||
return "", fs.ErrInvalid
|
return "", fs.ErrInvalid
|
||||||
}
|
}
|
||||||
frames := []importLinkFrame{{pending: strings.Split(name, "/")}}
|
return resolveImportLinkPath(name, importLinkOperations{
|
||||||
resolved := ""
|
lstat: func(candidate string) (fs.FileInfo, error) {
|
||||||
activeLinks := make(map[string]struct{})
|
|
||||||
for len(frames) > 0 {
|
|
||||||
frame := &frames[len(frames)-1]
|
|
||||||
if len(frame.pending) == 0 {
|
|
||||||
delete(activeLinks, frame.link)
|
|
||||||
frames = frames[:len(frames)-1]
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
segment := frame.pending[0]
|
|
||||||
frame.pending = frame.pending[1:]
|
|
||||||
candidate := path.Join(resolved, segment)
|
|
||||||
if candidate == "" {
|
|
||||||
candidate = "."
|
|
||||||
}
|
|
||||||
// Ancestors of a multi-component root are resolution prefixes, not
|
// Ancestors of a multi-component root are resolution prefixes, not
|
||||||
// selected targets. Complete paths and link targets remain contained.
|
// selected targets. Complete paths and link targets remain contained.
|
||||||
if !access.contains(candidate) && !strings.HasPrefix(access.rootPath, candidate+"/") {
|
if !access.contains(candidate) && !strings.HasPrefix(access.rootPath, candidate+"/") {
|
||||||
return "", errImportPathOutsideRoot
|
return nil, errImportPathOutsideRoot
|
||||||
}
|
|
||||||
info, err := filesystem.Lstat(candidate)
|
|
||||||
if err != nil {
|
|
||||||
return "", err
|
|
||||||
}
|
|
||||||
if info.Mode()&fs.ModeSymlink == 0 {
|
|
||||||
resolved = candidate
|
|
||||||
continue
|
|
||||||
}
|
|
||||||
if _, exists := activeLinks[candidate]; exists {
|
|
||||||
return "", errImportDirectoryLinkCycle
|
|
||||||
}
|
|
||||||
activeLinks[candidate] = struct{}{}
|
|
||||||
target, err := filesystem.ReadLink(candidate)
|
|
||||||
if err != nil {
|
|
||||||
return "", err
|
|
||||||
}
|
}
|
||||||
|
return filesystem.Lstat(candidate)
|
||||||
|
},
|
||||||
|
readlink: filesystem.ReadLink,
|
||||||
|
normalizeTarget: func(candidate, target string) (string, error) {
|
||||||
if target == "" || strings.HasPrefix(target, "/") {
|
if target == "" || strings.HasPrefix(target, "/") {
|
||||||
return "", errImportPathOutsideRoot
|
return "", errImportPathOutsideRoot
|
||||||
}
|
}
|
||||||
@@ -233,13 +270,9 @@ func resolveNativeImportPath(access *filesystemAccess, filesystem fs.ReadLinkFS,
|
|||||||
if !fs.ValidPath(targetPath) || !access.contains(targetPath) {
|
if !fs.ValidPath(targetPath) || !access.contains(targetPath) {
|
||||||
return "", errImportPathOutsideRoot
|
return "", errImportPathOutsideRoot
|
||||||
}
|
}
|
||||||
frames = append(frames, importLinkFrame{pending: strings.Split(targetPath, "/"), link: candidate})
|
return targetPath, nil
|
||||||
resolved = ""
|
},
|
||||||
}
|
})
|
||||||
if resolved == "" {
|
|
||||||
resolved = "."
|
|
||||||
}
|
|
||||||
return resolved, nil
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func isImportLinkCycle(err error) bool {
|
func isImportLinkCycle(err error) bool {
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package engine
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"errors"
|
"errors"
|
||||||
|
"fmt"
|
||||||
"io/fs"
|
"io/fs"
|
||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
@@ -45,3 +46,147 @@ func TestImportLinkResolutionTracksActiveExpansions(t *testing.T) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestNativeLinkResolutionMemoizesCompletedDependencies(t *testing.T) {
|
||||||
|
// The small graph stays within host symlink-depth limits while exceeding the
|
||||||
|
// bounded linear operation allowance without completed-target reuse.
|
||||||
|
const linkCount = 5
|
||||||
|
root := t.TempDir()
|
||||||
|
configurationRoot := filepath.Join(root, "workspace", "config")
|
||||||
|
for _, directory := range []string{
|
||||||
|
filepath.Join(configurationRoot, "graph"),
|
||||||
|
filepath.Join(configurationRoot, "selected"),
|
||||||
|
} {
|
||||||
|
if err := os.MkdirAll(directory, 0o700); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
for index := 0; index < linkCount; index++ {
|
||||||
|
name := fmt.Sprintf("s%02d", index)
|
||||||
|
target := "."
|
||||||
|
if index > 0 {
|
||||||
|
previous := fmt.Sprintf("s%02d", index-1)
|
||||||
|
target = previous + "/" + previous
|
||||||
|
}
|
||||||
|
if err := os.Symlink(target, filepath.Join(configurationRoot, "graph", name)); err != nil {
|
||||||
|
t.Skipf("symlinks unavailable: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if err := os.WriteFile(filepath.Join(configurationRoot, "graph", "item.yml"), []byte("value: linked\n"), 0o600); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := os.Symlink(fmt.Sprintf("../graph/s%02d", linkCount-1), filepath.Join(configurationRoot, "selected", "entry")); err != nil {
|
||||||
|
t.Skipf("symlinks unavailable: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, selector := range []string{"directory", "glob"} {
|
||||||
|
t.Run(selector, func(t *testing.T) {
|
||||||
|
base := os.DirFS(root)
|
||||||
|
if _, ok := base.(fs.ReadLinkFS); !ok {
|
||||||
|
t.Skip("host directory filesystem does not expose link inspection")
|
||||||
|
}
|
||||||
|
filesystem := newCountingReadLinkFS(base, linkCount*20+40)
|
||||||
|
access := newNativeAccess(filesystem, "workspace/config", &namespaceToken{})
|
||||||
|
if _, err := access.canonical("workspace/config/selected"); err != nil {
|
||||||
|
t.Fatalf("selected directory canonicalization failed before selection: %v", err)
|
||||||
|
}
|
||||||
|
var diagnostics []Diagnostic
|
||||||
|
if selector == "directory" {
|
||||||
|
_, diagnostics = selectDirectoryWithDiagnostics(t, access, "workspace/config", " - dir: selected\n follow: true\n")
|
||||||
|
} else {
|
||||||
|
_, diagnostics = selectGlobForTestWithDiagnostics(t, access, "workspace/config", " - glob: 'selected/*/*.yml'\n follow: true\n")
|
||||||
|
}
|
||||||
|
if len(diagnostics) != 0 {
|
||||||
|
t.Fatalf("selection diagnostics = %#v after %d link operations", diagnostics, filesystem.operations)
|
||||||
|
}
|
||||||
|
if filesystem.operations == 0 || filesystem.operations > filesystem.limit {
|
||||||
|
t.Fatalf("link-resolution operations = %d, want 1..%d", filesystem.operations, filesystem.limit)
|
||||||
|
}
|
||||||
|
if filesystem.readLinks < linkCount {
|
||||||
|
t.Fatalf("link reads = %d, want the selector to resolve the repeated dependency graph", filesystem.readLinks)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestNativeLinkResolutionRejectsEscapesAndSkipsWhenFollowingIsDisabled(t *testing.T) {
|
||||||
|
root := t.TempDir()
|
||||||
|
configurationRoot := filepath.Join(root, "workspace", "config")
|
||||||
|
selected := filepath.Join(configurationRoot, "selected")
|
||||||
|
if err := os.MkdirAll(selected, 0o700); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := os.Symlink("../../../outside", filepath.Join(selected, "escape")); err != nil {
|
||||||
|
t.Skipf("symlinks unavailable: %v", err)
|
||||||
|
}
|
||||||
|
base := os.DirFS(root)
|
||||||
|
if _, ok := base.(fs.ReadLinkFS); !ok {
|
||||||
|
t.Skip("host directory filesystem does not expose link inspection")
|
||||||
|
}
|
||||||
|
filesystem := newCountingReadLinkFS(base, 100)
|
||||||
|
access := newNativeAccess(filesystem, "workspace/config", &namespaceToken{})
|
||||||
|
if _, err := resolveNativeImportPath(access, filesystem.links, "workspace/config/selected/escape"); !errors.Is(err, errImportPathOutsideRoot) {
|
||||||
|
t.Fatalf("escaping native link error = %v, want outside-root rejection", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
filesystem.readLinks = 0
|
||||||
|
_, diagnostics := selectDirectoryWithDiagnostics(t, access, "workspace/config", " - dir: selected\n follow: false\n")
|
||||||
|
if len(diagnostics) != 0 || filesystem.readLinks != 0 {
|
||||||
|
t.Fatalf("follow-disabled selection diagnostics=%#v readlink calls=%d", diagnostics, filesystem.readLinks)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
type countingReadLinkFS struct {
|
||||||
|
filesystem fs.FS
|
||||||
|
links fs.ReadLinkFS
|
||||||
|
operations int
|
||||||
|
readLinks int
|
||||||
|
limit int
|
||||||
|
}
|
||||||
|
|
||||||
|
func newCountingReadLinkFS(filesystem fs.FS, limit int) *countingReadLinkFS {
|
||||||
|
links, ok := filesystem.(fs.ReadLinkFS)
|
||||||
|
if !ok {
|
||||||
|
panic("test filesystem does not support link inspection")
|
||||||
|
}
|
||||||
|
return &countingReadLinkFS{filesystem: filesystem, links: links, limit: limit}
|
||||||
|
}
|
||||||
|
|
||||||
|
func (f *countingReadLinkFS) Open(name string) (fs.File, error) { return f.filesystem.Open(name) }
|
||||||
|
|
||||||
|
func (f *countingReadLinkFS) Stat(name string) (fs.FileInfo, error) {
|
||||||
|
if statter, ok := f.filesystem.(fs.StatFS); ok {
|
||||||
|
return statter.Stat(name)
|
||||||
|
}
|
||||||
|
return fs.Stat(f.filesystem, name)
|
||||||
|
}
|
||||||
|
|
||||||
|
func (f *countingReadLinkFS) ReadDir(name string) ([]fs.DirEntry, error) {
|
||||||
|
if reader, ok := f.filesystem.(fs.ReadDirFS); ok {
|
||||||
|
return reader.ReadDir(name)
|
||||||
|
}
|
||||||
|
return fs.ReadDir(f.filesystem, name)
|
||||||
|
}
|
||||||
|
|
||||||
|
func (f *countingReadLinkFS) count() error {
|
||||||
|
f.operations++
|
||||||
|
if f.operations > f.limit {
|
||||||
|
return errors.New("native link operation limit exceeded")
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
func (f *countingReadLinkFS) Lstat(name string) (fs.FileInfo, error) {
|
||||||
|
if err := f.count(); err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
return f.links.Lstat(name)
|
||||||
|
}
|
||||||
|
|
||||||
|
func (f *countingReadLinkFS) ReadLink(name string) (string, error) {
|
||||||
|
if err := f.count(); err != nil {
|
||||||
|
return "", err
|
||||||
|
}
|
||||||
|
f.readLinks++
|
||||||
|
return f.links.ReadLink(name)
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user