Repository navigation
feat(go): refuse undeclared environment variables in generated parsers #1612
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
091a5b0
feat(go): refuse undeclared environment variables in generated Parse
jdx f75438e
fix(go): match usage-lib's Windows case folding and empty prefix in u…
jdx 04c1c1c
fix(go): sort and dedup unknown_env names by one Windows case key
jdx bcc299c
test(go): fold case clearing EX_ variables and check exact request codes
jdx 581fad9
fix(go): enforce unknown_env instead of refusing it, and match lib on…
jdx ea159ff
fix(go): refuse unknown_env with a mount in Go generation
jdx ca65f6e
docs(go): say generate skips the unknown_env and mount check, and pla…
jdx File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| package argv | ||
|
|
||
| import ( | ||
| "os" | ||
| "runtime" | ||
| "slices" | ||
| "strings" | ||
| "unicode/utf8" | ||
| ) | ||
|
|
||
| // UnknownEnv is the variables in names that start with prefix and are not in | ||
| // known, sorted and each once. | ||
| // | ||
| // A spec's `unknown_env` asks for this: a misspelled or stale variable under the | ||
| // CLI's own prefix is read by nobody, and the program runs on its defaults. known | ||
| // is every name the spec declares anywhere in its command tree, and its allow | ||
| // list; the generator collects it, since the whole tree counts and not only the | ||
| // command that ran — a variable meant for `serve` is not a typo when the user runs | ||
| // `publish`. | ||
| // | ||
| // names is passed in rather than read from the process for the reason [Fill] | ||
| // takes its lookup: a test of a parse should not be a test of the machine it runs | ||
| // on. [EnvNames] reads the process. On Windows, where variable names are | ||
| // case-insensitive, so is the comparison, and a variable set in two cases is | ||
| // reported once. | ||
| func UnknownEnv(prefix string, known, names []string) []string { | ||
| return unknownEnv(prefix, known, names, runtime.GOOS == "windows") | ||
| } | ||
|
|
||
| // unknownEnv is [UnknownEnv] with the platform as a parameter, so the Windows | ||
| // rules can be tested anywhere. | ||
| func unknownEnv(prefix string, known, names []string, fold bool) []string { | ||
| key := func(name string) string { | ||
| if fold { | ||
| return envKey(name) | ||
| } | ||
| return name | ||
| } | ||
| // An empty prefix owns nothing: the spec refuses one, and a hand-built call | ||
| // that passes one must not make PATH an error. | ||
| if prefix == "" { | ||
| return nil | ||
| } | ||
| var unknown []string | ||
| for _, name := range names { | ||
| // The prefix is matched against the valid UTF-8 head of the name, as | ||
| // usage-lib does: a prefix ending in U+FFFD must not claim a name whose | ||
| // invalid byte only reads as one. | ||
| if !strings.HasPrefix(key(validHead(name)), key(prefix)) { | ||
| continue | ||
| } | ||
| // A name that is not UTF-8 is never a declared one. | ||
| if utf8.ValidString(name) && slices.ContainsFunc(known, func(k string) bool { return key(k) == key(name) }) { | ||
| continue | ||
| } | ||
| unknown = append(unknown, name) | ||
| } | ||
| // On Windows EX_TYPO and ex_typo are one variable, and listing it twice would | ||
| // read as two mistakes. Names that are not UTF-8 are the same only byte for | ||
| // byte, since folding would turn every invalid byte into U+FFFD. | ||
| same := func(a, b string) bool { | ||
| if utf8.ValidString(a) && utf8.ValidString(b) { | ||
| return key(a) == key(b) | ||
| } | ||
| return a == b | ||
| } | ||
| // Sorted by the folded spelling and then the raw bytes, so the names same | ||
| // calls equal are always adjacent. | ||
| slices.SortFunc(unknown, func(a, b string) int { | ||
| if c := strings.Compare(key(strings.ToValidUTF8(a, "�")), key(strings.ToValidUTF8(b, "�"))); c != 0 { | ||
| return c | ||
| } | ||
| return strings.Compare(a, b) | ||
| }) | ||
| return slices.CompactFunc(unknown, same) | ||
| } | ||
|
|
||
| // validHead is name up to its first byte that is not valid UTF-8. | ||
| func validHead(name string) string { | ||
| for i, r := range name { | ||
| if r == utf8.RuneError { | ||
| if _, size := utf8.DecodeRuneInString(name[i:]); size == 1 { | ||
| return name[:i] | ||
| } | ||
| } | ||
| } | ||
| return name | ||
| } | ||
|
|
||
| // RefuseUnknownEnv is [UnknownEnv] as the error a parse returns, with every | ||
| // offending variable named at once: a deployment with two typos should not need | ||
| // two restarts to find them. It returns nil when there are none. | ||
| func RefuseUnknownEnv(prefix string, known, names []string) error { | ||
| unknown := UnknownEnv(prefix, known, names) | ||
| if len(unknown) == 0 { | ||
| return nil | ||
| } | ||
| return &Error{Code: CodeUnknownEnv, Name: prefix, Names: unknown} | ||
| } | ||
|
|
||
| // EnvNames is the names of the process environment, for callers that want it. | ||
| func EnvNames() []string { | ||
| environ := os.Environ() | ||
| names := make([]string, 0, len(environ)) | ||
| for _, entry := range environ { | ||
| // Windows keeps per-drive working directories as `=C:=C:\dir`, so the | ||
| // separator is the first `=` after the name's first byte. | ||
| if i := strings.IndexByte(entry[min(1, len(entry)):], '='); i >= 0 { | ||
| entry = entry[:i+min(1, len(entry))] | ||
| } | ||
| names = append(names, entry) | ||
| } | ||
| return names | ||
| } | ||
|
|
||
| // envKey is a Windows variable name as compared: uppercased with Unicode rules, | ||
| // as usage-lib does, because Windows folds ä and Ä together too. Not | ||
| // strings.EqualFold, which also folds the Kelvin sign to K where usage-lib and | ||
| // this key do not, and which no sort order can agree with. | ||
| func envKey(name string) string { return strings.ToUpper(name) } | ||
|
|
||
| // pluralWord is the noun alone, without [plural]'s count. | ||
| func pluralWord(n int, noun string) string { | ||
| if n == 1 { | ||
| return noun | ||
| } | ||
| return noun + "s" | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| package argv | ||
|
|
||
| import ( | ||
| "reflect" | ||
| "runtime" | ||
| "testing" | ||
| ) | ||
|
|
||
| func TestUnknownEnv(t *testing.T) { | ||
| known := []string{"EX_PORT", "EX_LIBRARY"} | ||
| names := []string{"EX_TSL", "HOME", "EX_PORT", "EX_", "EX", "EX_PROT", "EX_TSL", "EXTRA"} | ||
| got := UnknownEnv("EX_", known, names) | ||
| if want := []string{"EX_", "EX_PROT", "EX_TSL"}; !reflect.DeepEqual(got, want) { | ||
| t.Fatalf("got %q, want %q", got, want) | ||
| } | ||
| if err := RefuseUnknownEnv("EX_", known, []string{"EX_PORT"}); err != nil { | ||
| t.Fatalf("nothing unknown should be a nil error, got %#v", err) | ||
| } | ||
| // Windows names are case-insensitive; elsewhere a different case is a different variable. | ||
| got = UnknownEnv("EX_", known, []string{"EX_port", "Ex_Port", "ex_other"}) | ||
| want := []string{"EX_port"} | ||
| if runtime.GOOS == "windows" { | ||
| want = []string{"ex_other"} | ||
| } | ||
| if !reflect.DeepEqual(got, want) { | ||
| t.Fatalf("case: got %q, want %q", got, want) | ||
| } | ||
| // The Windows rules, wherever this runs: one variable set in two cases is | ||
| // reported once, and the Kelvin sign is not K, so EX_K is its own name | ||
| // and stays apart from EX_K even with EX_Z sorted between them. | ||
| got = unknownEnv("EX_", nil, []string{"EX_K", "EX_Z", "EX_K", "ex_k", "EX_ä", "EX_Ä"}, true) | ||
| if want := []string{"EX_K", "EX_Z", "EX_Ä", "EX_K"}; !reflect.DeepEqual(got, want) { | ||
| t.Fatalf("windows: got %q, want %q", got, want) | ||
| } | ||
| // ß has no one-letter uppercase, so usage-lib keeps it as it is; so must this. | ||
| if got := envKey("EX_ß"); got != "EX_ß" { | ||
| t.Fatalf("ß folded to %q", got) | ||
| } | ||
| // Names that are not UTF-8: the prefix is matched on the valid head, so a | ||
| // prefix ending in U+FFFD does not claim an invalid byte; such a name is never | ||
| // a declared one; and two of them are one only when their bytes are. | ||
| for _, fold := range []bool{false, true} { | ||
| if got := unknownEnv("EX_�", nil, []string{"EX_\xff"}, fold); got != nil { | ||
| t.Fatalf("fold=%v: U+FFFD prefix claimed an invalid byte: %q", fold, got) | ||
| } | ||
| got := unknownEnv("EX_", []string{"EX_�"}, []string{"EX_\xfe", "EX_\xff", "EX_\xff"}, fold) | ||
| if want := []string{"EX_\xfe", "EX_\xff"}; !reflect.DeepEqual(got, want) { | ||
| t.Fatalf("fold=%v: non-UTF-8 names: got %q, want %q", fold, got, want) | ||
| } | ||
| } | ||
| if got := UnknownEnv("", known, []string{"PATH"}); got != nil { | ||
| t.Fatalf("an empty prefix owns nothing, got %q", got) | ||
| } | ||
| } | ||
|
|
||
| func TestUnknownEnvRendering(t *testing.T) { | ||
| err := &Error{Code: CodeUnknownEnv, Name: "EX_", Names: []string{"EX_PROT"}} | ||
| if got, want := err.Error(), "unknown environment variable under EX_: EX_PROT"; got != want { | ||
| t.Fatalf("Error(): %q", got) | ||
| } | ||
| if got, want := Render(err, nil, nil, nil), "error: unknown environment variable under `EX_`: `EX_PROT`\n\nFor more information, try `--help`.\n"; got != want { | ||
| t.Fatalf("Render: %q", got) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| package conformance | ||
|
|
||
| import "testing" | ||
|
|
||
| // TestGeneratedUnknownEnv runs a generated Parse against the real process | ||
| // environment, which is what `unknown_env` judges. | ||
| func TestGeneratedUnknownEnv(t *testing.T) { | ||
| runGeneratedFixture(t, `name "example" | ||
| bin "example" | ||
| version "1.0" | ||
| unknown_env prefix="EX_" { | ||
| allow "EX_LIBRARY" | ||
| } | ||
| flag "--port <port>" env="EX_PORT" | ||
| cmd "serve" { | ||
| flag "--tls" deprecated_env="EX_SSL" | ||
| } | ||
| cmd "publish" { | ||
| flag "--name <name>" required=#true env="EX_NAME" | ||
| } | ||
| config { | ||
| prop "jobs" env="EX_JOBS" | ||
| } | ||
| `, `package generated | ||
| import ( | ||
| "errors" | ||
| "os" | ||
| "reflect" | ||
| "strings" | ||
| "testing" | ||
| "github.com/jdx/usage/go/argv" | ||
| ) | ||
| func clearEx(t *testing.T) { | ||
| for _, entry := range os.Environ() { | ||
| // Upper-cased, since on Windows an inherited ex_typo is under EX_ too. | ||
| if name, _, _ := strings.Cut(entry, "="); strings.HasPrefix(strings.ToUpper(name), "EX_") { | ||
| t.Setenv(name, "") | ||
| os.Unsetenv(name) | ||
| } | ||
| } | ||
| } | ||
| func TestUnknownEnv(t *testing.T) { | ||
| clearEx(t) | ||
| // Declared by another command, deprecated, read by config, allowed, or outside the prefix. | ||
| for _, name := range []string{"EX_PORT", "EX_SSL", "EX_JOBS", "EX_LIBRARY", "EXTRA"} { t.Setenv(name, "1") } | ||
| t.Setenv("EX_NAME", "n") | ||
| if _, err := Parse([]string{"publish"}); err != nil { t.Fatalf("declared names: %v", err) } | ||
|
|
||
| t.Setenv("EX_TSL", "1") | ||
| t.Setenv("EX_PROT", "1") | ||
| _, err := Parse([]string{"serve"}); var req *argv.Error | ||
| if !errors.As(err, &req) || req.Code != argv.CodeUnknownEnv { t.Fatalf("typos: %v", err) } | ||
| if req.Name != "EX_" || !reflect.DeepEqual(req.Names, []string{"EX_PROT", "EX_TSL"}) { t.Fatalf("every offender, sorted: %#v", req) } | ||
| if got, want := err.Error(), "unknown environment variables under EX_: EX_PROT, EX_TSL"; got != want { t.Fatalf("message: %q", got) } | ||
|
|
||
| // Reported ahead of the requirement it may be the cause of. | ||
| os.Unsetenv("EX_NAME") | ||
| _, err = Parse([]string{"publish"}) | ||
| if !errors.As(err, &req) || req.Code != argv.CodeUnknownEnv { t.Fatalf("before requirements: %v", err) } | ||
|
|
||
| // Help and version still answer. | ||
| for _, tc := range []struct{ args []string; code argv.Code }{{[]string{"--help"}, argv.CodeHelp}, {[]string{"help", "serve"}, argv.CodeHelp}, {[]string{"--version"}, argv.CodeVersion}} { | ||
| _, err = Parse(tc.args) | ||
| if !errors.As(err, &req) || req.Code != tc.code { t.Fatalf("%v: %v", tc.args, err) } | ||
| } | ||
| } | ||
| `, "unknown-env") | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.