Use Go rules for Windows filename components
This commit is contained in:
@@ -1,6 +1,6 @@
|
|||||||
# Configuration Discovery Review Findings
|
# Configuration Discovery Review Findings
|
||||||
|
|
||||||
Status: findings 1–3 complete; findings 4–5 remain open.
|
Status: findings 1–4 complete; finding 5 remains open.
|
||||||
|
|
||||||
Reviewed commit: `e50fee2`, including the discovery implementation commits after
|
Reviewed commit: `e50fee2`, including the discovery implementation commits after
|
||||||
`424c897`. The review compared the implementation with the
|
`424c897`. The review compared the implementation with the
|
||||||
@@ -142,7 +142,14 @@ release procedure; do not introduce native macOS validation as a gate.
|
|||||||
|
|
||||||
## 4. Use Platform-Aware Windows Component Validation
|
## 4. Use Platform-Aware Windows Component Validation
|
||||||
|
|
||||||
Priority: P3. Status: open. Algorithmic audit: correctness finding.
|
Priority: P3. Status: complete. Algorithmic audit: correctness finding.
|
||||||
|
|
||||||
|
**Completed:** Windows validation now delegates reserved-name decisions to
|
||||||
|
`filepath.IsLocal` while preserving the selected component/character checks. The
|
||||||
|
manual device table was removed. Tests cover console names, platform-dependent
|
||||||
|
extension behavior, and invalid later search components; Unix tests retain their
|
||||||
|
own filename rules. Focused tests and `./scripts/check.sh` passed on Linux. The
|
||||||
|
Windows engine test binary cross-compiled successfully; it was not run natively.
|
||||||
|
|
||||||
**Location:** [discovery.go](../../internal/engine/discovery.go),
|
**Location:** [discovery.go](../../internal/engine/discovery.go),
|
||||||
`validWindowsFilenameComponent`, lines 266–281.
|
`validWindowsFilenameComponent`, lines 266–281.
|
||||||
@@ -249,8 +256,8 @@ ordinary file acquisition. It did not benchmark or reopen those choices.
|
|||||||
|
|
||||||
No unplanned quadratic algorithm or other actionable asymptotic regression was
|
No unplanned quadratic algorithm or other actionable asymptotic regression was
|
||||||
identified. Findings 4 and 5 record the additional ordinary correctness issues
|
identified. Findings 4 and 5 record the additional ordinary correctness issues
|
||||||
found while examining those algorithms; findings 1–3 remain open from the earlier
|
found while examining those algorithms; findings 1–3 were carried forward from
|
||||||
review. Existing configuration-tree algorithms outside the discovery changes
|
the earlier review. Existing configuration-tree algorithms outside the discovery changes
|
||||||
were not re-audited as new feature work.
|
were not re-audited as new feature work.
|
||||||
|
|
||||||
## Review Validation
|
## Review Validation
|
||||||
@@ -261,7 +268,7 @@ were not re-audited as new feature work.
|
|||||||
- An external temporary Go program reproduced findings 1 and 2 through the
|
- An external temporary Go program reproduced findings 1 and 2 through the
|
||||||
public API, comparing explicit file loading with discovery. No production or
|
public API, comparing explicit file loading with discovery. No production or
|
||||||
test files were modified for these reproductions.
|
test files were modified for these reproductions.
|
||||||
- Findings 1–3 remain open despite the existing suites passing.
|
- Findings 1–3 remained open at the original review despite the suites passing.
|
||||||
The broader discovery architecture is implemented: immutable root-owned
|
The broader discovery architecture is implemented: immutable root-owned
|
||||||
descriptions, incremental ordered selection during shared source acquisition,
|
descriptions, incremental ordered selection during shared source acquisition,
|
||||||
concrete selected-source propagation, and shared absent-input handling before
|
concrete selected-source propagation, and shared absent-input handling before
|
||||||
|
|||||||
@@ -258,27 +258,16 @@ func validFilenameComponent(value string) bool {
|
|||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
if runtime.GOOS == "windows" {
|
if runtime.GOOS == "windows" {
|
||||||
return validWindowsFilenameComponent(value)
|
if !filepath.IsLocal(value) || strings.ContainsAny(value, `<>:"|?*`) || strings.HasSuffix(value, ".") || strings.HasSuffix(value, " ") {
|
||||||
}
|
|
||||||
return true
|
|
||||||
}
|
|
||||||
|
|
||||||
func validWindowsFilenameComponent(value string) bool {
|
|
||||||
if strings.ContainsAny(value, `<>:"|?*`) || strings.HasSuffix(value, ".") || strings.HasSuffix(value, " ") {
|
|
||||||
return false
|
|
||||||
}
|
|
||||||
for _, character := range value {
|
|
||||||
if character < 32 {
|
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
for _, character := range value {
|
||||||
|
if character < 32 {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
base := strings.ToUpper(strings.TrimSuffix(value, filepath.Ext(value)))
|
return true
|
||||||
switch base {
|
|
||||||
case "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9", "COM¹", "COM²", "COM³", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", "LPT¹", "LPT²", "LPT³":
|
|
||||||
return false
|
|
||||||
default:
|
|
||||||
return true
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
var errUnixSystemPresetUnavailable = errors.New("Unix system directories are unavailable on this platform")
|
var errUnixSystemPresetUnavailable = errors.New("Unix system directories are unavailable on this platform")
|
||||||
|
|||||||
@@ -129,31 +129,33 @@ func TestFilenameComponentValidation(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
if runtime.GOOS == "windows" {
|
if runtime.GOOS == "windows" {
|
||||||
for _, value := range []string{"CON", "nul.txt", "name.", "bad:name"} {
|
for _, value := range []string{"CON", "CONIN$", "CONOUT$", "name.", "bad:name", "bad\x01name"} {
|
||||||
if validFilenameComponent(value) {
|
if validFilenameComponent(value) {
|
||||||
t.Errorf("Windows component %q unexpectedly accepted", value)
|
t.Errorf("Windows component %q unexpectedly accepted", value)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else {
|
// Device names with extensions depend on the Windows version. Match
|
||||||
for _, value := range []string{"name with spaces", "naïve"} {
|
// Go's platform-aware decision instead of imposing a blanket ban.
|
||||||
if !validFilenameComponent(value) {
|
for _, value := range []string{"nul.txt", "COM¹.log", "LPT³.txt", "CON.extra.yml"} {
|
||||||
t.Errorf("host-valid component %q rejected", value)
|
if got, want := validFilenameComponent(value), filepath.IsLocal(value); got != want {
|
||||||
|
t.Errorf("Windows component %q validity = %v, want Go's %v", value, got, want)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
} else {
|
||||||
|
for _, value := range []string{"CONIN$", "CONOUT$", "nul.txt"} {
|
||||||
|
if !validFilenameComponent(value) {
|
||||||
|
t.Errorf("Windows component restriction applied to this host: %q", value)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
for _, value := range []string{"name with spaces", "naïve"} {
|
||||||
|
if !validFilenameComponent(value) {
|
||||||
|
t.Errorf("host-valid component %q rejected", value)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if diagnostics := validateSearch(SearchDirectory("configs", DirectorySearchOptions{Filenames: []FilenameGroup{CustomFilenames("bad/name")}}), ""); len(diagnostics) == 0 {
|
if diagnostics := validateSearch(SearchDirectory("configs", DirectorySearchOptions{Filenames: []FilenameGroup{CustomFilenames("bad/name")}}), ""); len(diagnostics) == 0 {
|
||||||
t.Fatal("invalid custom filename was not retained for validation")
|
t.Fatal("invalid custom filename was not retained for validation")
|
||||||
}
|
}
|
||||||
for _, value := range []string{"CON", "nul.txt", "COM¹.log", "LPT³.txt", "name.", "bad:name", "bad\x01name"} {
|
|
||||||
if validWindowsFilenameComponent(value) {
|
|
||||||
t.Errorf("Windows component %q unexpectedly accepted", value)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
for _, value := range []string{"name with spaces", "naïve"} {
|
|
||||||
if !validWindowsFilenameComponent(value) {
|
|
||||||
t.Errorf("valid Windows component %q rejected", value)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestSearchCandidatesPreserveDirectoryAndGroupOrder(t *testing.T) {
|
func TestSearchCandidatesPreserveDirectoryAndGroupOrder(t *testing.T) {
|
||||||
|
|||||||
32
internal/engine/discovery_windows_test.go
Normal file
32
internal/engine/discovery_windows_test.go
Normal file
@@ -0,0 +1,32 @@
|
|||||||
|
//go:build windows
|
||||||
|
|
||||||
|
package engine
|
||||||
|
|
||||||
|
import (
|
||||||
|
"errors"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestWindowsSearchRejectsReservedComponentsBeforeAcquisition(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
root := t.TempDir()
|
||||||
|
first := writeImportFile(t, root, "first.yml", "value: first\n")
|
||||||
|
for _, name := range []string{"CONIN$", "CONOUT$"} {
|
||||||
|
for _, source := range []Source{
|
||||||
|
SearchConfig(name, Searches(Paths(first), WorkingDirectory())),
|
||||||
|
SearchConfig("", Searches(Paths(first), SearchDirectory(root, DirectorySearchOptions{
|
||||||
|
Filenames: []FilenameGroup{CustomFilenames(name)},
|
||||||
|
}))),
|
||||||
|
} {
|
||||||
|
result, err := testLoad[discoveryLoadConfig](source, Options[discoveryLoadConfig]{})
|
||||||
|
var failure *LoadError
|
||||||
|
if result != nil || !errors.As(err, &failure) || len(failure.Diagnostics()) != 1 || failure.Diagnostics()[0].Code != CodeOptionsInvalid {
|
||||||
|
t.Fatalf("reserved component %q accepted: %#v, %v", name, result, err)
|
||||||
|
}
|
||||||
|
if len(failure.Sources()) != 0 {
|
||||||
|
t.Fatalf("reserved component %q was checked after acquisition: %#v", name, failure.Sources())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user