Terminate pointer-only descriptor traversal
This commit is contained in:
@@ -78,6 +78,9 @@ race, consumer, and documentation validation is specified in Stage 33.
|
||||
Finding: **A01**; also the `schemaPath` portion of **F06**.
|
||||
Owners: `internal/engine/schema.go`, `decode.go`, `secret.go`, and their tests.
|
||||
|
||||
Status: **complete** (A01; schema-path portion of F06). Validation passed:
|
||||
`GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'Schema|Decode|Resolution|Secret|PointerOnly' -count=1`, the specified focused check, and `GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -count=1`.
|
||||
|
||||
1. Introduce one private pointer-descriptor terminal lookup that detects a
|
||||
repeated descriptor, using a traversal-local visited set. Return failure for
|
||||
a pointer-only loop; do not reject that type during schema construction.
|
||||
|
||||
@@ -91,19 +91,30 @@ func decodeValue(node *document.Node, base reflect.Value, descriptor *typeDescri
|
||||
}
|
||||
}
|
||||
if descriptor.kind == descriptorPointer {
|
||||
var elementBase reflect.Value
|
||||
if base.IsNil() {
|
||||
elementBase = reflect.Zero(descriptor.typeOf.Elem())
|
||||
} else {
|
||||
elementBase = base.Elem()
|
||||
terminal, ok := terminalDescriptor(descriptor)
|
||||
if !ok {
|
||||
return reflect.Value{}, []decodeIssue{newDecodeIssue(CodeDecodeType, path, node, "configuration type cannot represent a non-null value")}
|
||||
}
|
||||
decoded, issues := decodeValue(node, elementBase, descriptor.element, path, schema, context)
|
||||
var pointers []*typeDescriptor
|
||||
currentBase := base
|
||||
for current := descriptor; current != terminal; current = current.element {
|
||||
pointers = append(pointers, current)
|
||||
if currentBase.IsNil() {
|
||||
currentBase = reflect.Zero(current.typeOf.Elem())
|
||||
} else {
|
||||
currentBase = currentBase.Elem()
|
||||
}
|
||||
}
|
||||
decoded, issues := decodeValue(node, currentBase, terminal, path, schema, context)
|
||||
if len(issues) != 0 {
|
||||
return reflect.Value{}, issues
|
||||
}
|
||||
result := reflect.New(descriptor.typeOf.Elem())
|
||||
for i := len(pointers) - 1; i >= 0; i-- {
|
||||
result := reflect.New(pointers[i].typeOf.Elem())
|
||||
result.Elem().Set(decoded)
|
||||
return result, nil
|
||||
decoded = result
|
||||
}
|
||||
return decoded, nil
|
||||
}
|
||||
|
||||
switch descriptor.kind {
|
||||
@@ -392,10 +403,11 @@ func decodeDynamic(node *document.Node, base reflect.Value, path Path, schema *s
|
||||
}
|
||||
|
||||
func acceptsMapping(descriptor *typeDescriptor) bool {
|
||||
for descriptor != nil && descriptor.kind == descriptorPointer {
|
||||
descriptor = descriptor.element
|
||||
terminal, ok := terminalDescriptor(descriptor)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
return descriptor != nil && (descriptor.kind == descriptorStruct || descriptor.kind == descriptorMap || descriptor.kind == descriptorDynamic)
|
||||
return terminal.kind == descriptorStruct || terminal.kind == descriptorMap || terminal.kind == descriptorDynamic
|
||||
}
|
||||
|
||||
func fieldForDecode(root reflect.Value, index []int, allocate bool) (reflect.Value, bool) {
|
||||
|
||||
112
internal/engine/pointer_descriptor_test.go
Normal file
112
internal/engine/pointer_descriptor_test.go
Normal file
@@ -0,0 +1,112 @@
|
||||
package engine
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"os"
|
||||
"os/exec"
|
||||
"reflect"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
type pointerOnlyLoop *pointerOnlyLoop
|
||||
|
||||
type pointerOnlyConfig struct {
|
||||
Loop pointerOnlyLoop `yaml:"loop"`
|
||||
}
|
||||
|
||||
func TestPointerOnlyDescriptorsTerminate(t *testing.T) {
|
||||
if mode := os.Getenv("CONFIGKIT_POINTER_DESCRIPTOR_TEST_MODE"); mode != "" {
|
||||
switch mode {
|
||||
case "decode":
|
||||
_, err := testLoad[pointerOnlyConfig](Bytes("config.yml", []byte("loop: {}\n")))
|
||||
var loadErr *LoadError
|
||||
if !errors.As(err, &loadErr) {
|
||||
t.Fatalf("load error = %v, want a safe decode failure", err)
|
||||
}
|
||||
for _, diagnostic := range loadErr.Diagnostics() {
|
||||
if diagnostic.Code == CodeDecodeType && diagnostic.Path.String() == "/loop" {
|
||||
return
|
||||
}
|
||||
}
|
||||
t.Fatalf("diagnostics = %#v, want decode type at /loop", loadErr.Diagnostics())
|
||||
case "paths":
|
||||
schema, issue := testBuildSchema(reflect.TypeFor[pointerOnlyConfig]())
|
||||
if issue != nil {
|
||||
t.Fatalf("schema issue = %#v", issue)
|
||||
}
|
||||
if _, ok := resolveSchemaPath(schema, Keys("loop", "child")); ok {
|
||||
t.Fatal("descendant below a pointer-only loop was accepted")
|
||||
}
|
||||
if _, ok := schemaSecretRoot(schema, Keys("loop", "child")); ok {
|
||||
t.Fatal("secret path below a pointer-only loop was accepted")
|
||||
}
|
||||
if acceptsMapping(schema.root.fields[schema.root.fieldByName["loop"]].descriptor) {
|
||||
t.Fatal("pointer-only loop was accepted as a mapping")
|
||||
}
|
||||
default:
|
||||
t.Fatalf("unknown test mode %q", mode)
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
for _, mode := range []string{"decode", "paths"} {
|
||||
t.Run(mode, func(t *testing.T) {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||
defer cancel()
|
||||
command := exec.CommandContext(ctx, os.Args[0], "-test.run=^TestPointerOnlyDescriptorsTerminate$")
|
||||
command.Env = append(os.Environ(), "CONFIGKIT_POINTER_DESCRIPTOR_TEST_MODE="+mode)
|
||||
output, err := command.CombinedOutput()
|
||||
if ctx.Err() != nil {
|
||||
t.Fatalf("pointer descriptor %s did not terminate: %s", mode, output)
|
||||
}
|
||||
if err != nil {
|
||||
t.Fatalf("pointer descriptor %s failed: %v, %s", mode, err, output)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestPointerOnlyNullableFieldAndRecursiveValues(t *testing.T) {
|
||||
for name, source := range map[string]Source{
|
||||
"omitted": NoSource(),
|
||||
"null": Bytes("config.yml", []byte("loop: null\n")),
|
||||
} {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
result, err := testLoad[pointerOnlyConfig](source)
|
||||
if err != nil || result.Config.Loop != nil {
|
||||
t.Fatalf("result = %#v, error = %v", result, err)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
result, err := testLoad[recursiveConfig](Bytes("config.yml", []byte("name: root\nnext:\n name: child\n next:\n name: leaf\n")))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if result.Config.Name != "root" || result.Config.Next == nil || result.Config.Next.Name != "child" || result.Config.Next.Next == nil || result.Config.Next.Next.Name != "leaf" {
|
||||
t.Fatalf("recursive value = %#v", result.Config)
|
||||
}
|
||||
|
||||
schema, issue := testBuildSchema(reflect.TypeFor[pointerOnlyConfig]())
|
||||
if issue != nil {
|
||||
t.Fatal(issue.message)
|
||||
}
|
||||
resolved, ok := resolveSchemaPath(schema, Keys("loop"))
|
||||
if !ok || resolved.descriptor.kind != descriptorPointer {
|
||||
t.Fatalf("nullable pointer field resolution = %#v, %v", resolved, ok)
|
||||
}
|
||||
|
||||
called := false
|
||||
nullableResult, err := testLoad[pointerOnlyConfig](NoSource(), Options[pointerOnlyConfig]{Definition: Definition[pointerOnlyConfig]{
|
||||
Resolve: func(config *pointerOnlyConfig, resolution *Resolution) error {
|
||||
called = true
|
||||
Normalize(resolution, Keys("loop"), &config.Loop, nil, "clear nullable field")
|
||||
return nil
|
||||
},
|
||||
}})
|
||||
if err != nil || !called || nullableResult.Config.Loop != nil {
|
||||
t.Fatalf("nullable field operation = %#v, called=%v, error=%v", nullableResult, called, err)
|
||||
}
|
||||
}
|
||||
@@ -334,8 +334,21 @@ func prependIndex(index int, nested []int) []int {
|
||||
|
||||
type schemaPath struct {
|
||||
descriptor *typeDescriptor
|
||||
field *fieldDescriptor
|
||||
dynamic bool
|
||||
}
|
||||
|
||||
// terminalDescriptor follows a pointer-only descriptor chain without entering
|
||||
// any value. Recursive pointer aliases can refer back to an earlier descriptor,
|
||||
// so callers must handle a repeated descriptor as an unaddressable chain.
|
||||
func terminalDescriptor(descriptor *typeDescriptor) (*typeDescriptor, bool) {
|
||||
visited := make(map[*typeDescriptor]struct{})
|
||||
for descriptor != nil && descriptor.kind == descriptorPointer {
|
||||
if _, exists := visited[descriptor]; exists {
|
||||
return nil, false
|
||||
}
|
||||
visited[descriptor] = struct{}{}
|
||||
descriptor = descriptor.element
|
||||
}
|
||||
return descriptor, descriptor != nil
|
||||
}
|
||||
|
||||
func resolveSchemaPath(schema *schemaDescriptor, path Path) (schemaPath, bool) {
|
||||
@@ -343,10 +356,11 @@ func resolveSchemaPath(schema *schemaDescriptor, path Path) (schemaPath, bool) {
|
||||
return schemaPath{}, false
|
||||
}
|
||||
current := schema.root
|
||||
var field *fieldDescriptor
|
||||
for _, segment := range path.segments {
|
||||
for current.kind == descriptorPointer {
|
||||
current = current.element
|
||||
var ok bool
|
||||
current, ok = terminalDescriptor(current)
|
||||
if !ok {
|
||||
return schemaPath{}, false
|
||||
}
|
||||
switch current.kind {
|
||||
case descriptorStruct:
|
||||
@@ -355,12 +369,10 @@ func resolveSchemaPath(schema *schemaDescriptor, path Path) (schemaPath, bool) {
|
||||
}
|
||||
position, ok := current.fieldByName[segment.key]
|
||||
if ok {
|
||||
field = ¤t.fields[position]
|
||||
current = field.descriptor
|
||||
current = current.fields[position].descriptor
|
||||
continue
|
||||
}
|
||||
if current.inlineMap != nil {
|
||||
field = current.inlineMap
|
||||
current = current.inlineMap.descriptor.element
|
||||
continue
|
||||
}
|
||||
@@ -369,26 +381,20 @@ func resolveSchemaPath(schema *schemaDescriptor, path Path) (schemaPath, bool) {
|
||||
if segment.kind == pathSegmentIndex {
|
||||
return schemaPath{}, false
|
||||
}
|
||||
field = nil
|
||||
current = current.element
|
||||
case descriptorSlice, descriptorArray:
|
||||
index, ok := schemaIndex(segment)
|
||||
if !ok || (current.kind == descriptorArray && index >= current.length) {
|
||||
return schemaPath{}, false
|
||||
}
|
||||
field = nil
|
||||
current = current.element
|
||||
case descriptorDynamic:
|
||||
return schemaPath{descriptor: current, field: field, dynamic: true}, true
|
||||
return schemaPath{descriptor: current}, true
|
||||
default:
|
||||
return schemaPath{}, false
|
||||
}
|
||||
}
|
||||
dynamic := current
|
||||
for dynamic.kind == descriptorPointer {
|
||||
dynamic = dynamic.element
|
||||
}
|
||||
return schemaPath{descriptor: current, field: field, dynamic: dynamic.kind == descriptorDynamic}, true
|
||||
return schemaPath{descriptor: current}, true
|
||||
}
|
||||
|
||||
func schemaIndex(segment pathSegment) (int, bool) {
|
||||
|
||||
@@ -168,8 +168,10 @@ func schemaSecretRoot(schema *schemaDescriptor, path Path) (Path, bool) {
|
||||
walked := Path{}
|
||||
for _, segment := range path.segments {
|
||||
walked.segments = append(walked.segments, segment)
|
||||
for current.kind == descriptorPointer {
|
||||
current = current.element
|
||||
var ok bool
|
||||
current, ok = terminalDescriptor(current)
|
||||
if !ok {
|
||||
return Path{}, false
|
||||
}
|
||||
switch current.kind {
|
||||
case descriptorStruct:
|
||||
|
||||
Reference in New Issue
Block a user