diff --git a/CHANGELOG.md b/CHANGELOG.md index 53e5191..f8f92ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -102,8 +102,29 @@ because it turns other people's test suites red. That takes the plain encoder and writes the same bytes this tool wrote before, to the byte - there is a pinned hash proving it. +- **A setting value is spelled the way the format declares it.** `--set + page_size=A4` and `--set directory_entries=TRUE` used to be accepted and now + refuse with exit 4, naming the setting and the value. Write `a4` and `true`. + + Nothing else changes: the bytes of every file are what they were, and a + recipe writing `header: true` is unaffected, because a YAML boolean arrives + as `true` either way. Only a value quoted into another case is refused. + + It is a breaking change for a fourth of a reason and a fix for the rest. A + value the declaration did not contain used to pass the check and land on the + format, which then did one of four things with it: refuse in its own words, + understand it anyway, **quietly ignore it and produce the default file**, or + read it as something else entirely. `--set entry_owner=USER` on a `targz` + wrote an archive owned by nobody and reported success. Now nothing gets that + far. + ### Added +- **A `targz` manifest says what its entries claim about themselves.** Two new + keys on the file entry, `entry_mode` and `entry_owner`, written every run + rather than only when you ask for them, so a harness never has to read a + missing key as "nobody owns this". The manifest schema version is unchanged. + - **A CSV can be written in the dialect you were handed.** `--set delimiter=semicolon`, `--set line_ending=crlf` and `--set header=false` on a `csv`, separately or together. Separators are named rather than typed, so diff --git a/internal/format/archive/ownership.go b/internal/format/archive/ownership.go index e78b8d7..6e237c9 100644 --- a/internal/format/archive/ownership.go +++ b/internal/format/archive/ownership.go @@ -34,6 +34,23 @@ type Ownership struct { // All four are left at nought and empty for the unset owner. Uid, Gid int Uname, Gname string + // Stated is what the settings said, in the words they were written in, for + // the manifest to record. + // + // Carried rather than worked out again at the far end. A mode of 000 turns + // back into "0" rather than "000" through the obvious arithmetic, and an + // owner would have to be recovered from Uname, which is empty for two + // different reasons. A fact the writer already has is cheaper to pass along + // than to reconstruct - ARCHITECTURE.md 5. + Stated Stated +} + +// Stated is the pair of settings as a person wrote them. +type Stated struct { + // Mode is the three octal digits, so 644 stays 644. + Mode string + // Owner is the declared word: root, user or unset. + Owner string } // ReadOwnership reads the two settings that say what an entry records about @@ -45,7 +62,7 @@ type Ownership struct { // a setting where the number a person types is not the number the file gets is // the kind of difference nobody predicts. func ReadOwnership(id string, props map[string]string) (Ownership, error) { - own := Ownership{Mode: 0o644} + own := Ownership{Mode: 0o644, Stated: Stated{Mode: "644", Owner: OwnerUnset}} if raw := props[EntryMode]; raw != "" { mode, err := strconv.ParseInt(raw, 8, 32) @@ -61,16 +78,37 @@ func ReadOwnership(id string, props map[string]string) (Ownership, error) { } } own.Mode = mode + own.Stated.Mode = raw } - switch props[EntryOwner] { + switch raw := props[EntryOwner]; raw { case OwnerRoot: + own.Stated.Owner = OwnerRoot own.Uname, own.Gname = "root", "root" case OwnerUser: // The first ordinary account on a Linux system, which is what somebody // unpacking a fixture on their own machine most likely is. + own.Stated.Owner = OwnerUser own.Uid, own.Gid = 1000, 1000 own.Uname, own.Gname = "user", "user" + case "", OwnerUnset: + // Nothing to record, which is what this format wrote before the setting + // existed. An absent key and the declared word for absent mean the same + // archive. + default: + // There was no default branch here, and the value fell through to an + // archive owned by nobody - the same bytes as unset, reported as + // success. Measured 2026-09-02: entry_owner=USER and entry_owner=ROOT + // each produced a file byte for byte identical to the default one, with + // exit 0 and not a word about it, which is rule 6 broken outright. The + // registry now stops a misspelling before it arrives, so this branch is + // for the other door: this function is callable directly. + return Ownership{}, &format.PropertyValueError{ + Format: id, Key: EntryOwner, Value: raw, + Reason: "it takes one of: " + OwnerRoot + ", " + OwnerUnset + ", " + OwnerUser, + Remedy: "Write it the way the setting is declared, so root or user, " + + "or leave the line out and the entries carry no owner.", + } } return own, nil } diff --git a/internal/format/csvfile/dialect.go b/internal/format/csvfile/dialect.go index fa6bfa8..259517e 100644 --- a/internal/format/csvfile/dialect.go +++ b/internal/format/csvfile/dialect.go @@ -80,10 +80,14 @@ func defaultDialect() dialect { // // A value that is not in the declared set has already been refused by the // registry, which checks it against the declaration for every format at once. -// The refusals here catch what that check lets through: it compares without -// regard for case, so REALISTIC style spellings arrive here rather than being -// stopped there. Written up as O168 - the branches below are reachable through -// that door and only through it, so they are not dead code. +// +// Until 2026-09-02 that check compared without regard for case, so a REALISTIC +// style spelling walked past it and was refused here instead, in different +// words - O168. The registry compares exactly now, so nothing arriving through +// a recipe or a flag can reach these branches. They stay because this function +// is callable directly and a guard calling it is such a caller, and because a +// generator that trusts its input is one registry change away from writing a +// file nobody ordered. func parseDialect(props map[string]string) (dialect, error) { d := defaultDialect() diff --git a/internal/format/format.go b/internal/format/format.go index d551548..29540a8 100644 --- a/internal/format/format.go +++ b/internal/format/format.go @@ -385,14 +385,28 @@ func (p Property) Allows(raw string) (bad string) { } } case PropertyChoice: + // Spelled the way it is declared, not merely close to it. This used to + // fold with EqualFold, and the folding was invisible here and decisive + // three layers down: a value the declaration does not contain reached + // the generator, and each generator did something different with it. + // Measured 2026-09-02 across the eight formats with a closed set, four + // answers - a refusal in the generator's own words, a fold that + // understood it, a swallow that made the DEFAULT file and reported + // success, and a misread that demanded a password to lock an archive + // with "NONE". O168. One place to say no is the whole point of + // declaring the set here. for _, c := range p.Choices { - if strings.EqualFold(raw, c) { + if raw == c { return "" } } return "it takes one of: " + strings.Join(p.Choices, ", ") case PropertyBool: - switch strings.ToLower(raw) { + // Exact for the same reason, and it costs a user nothing: a recipe + // writing `header: True` arrives as "true" already, because the reader + // puts a YAML boolean through FormatBool. Only a hand quoted "TRUE" + // changes, and it changes into a refusal that names the setting. + switch raw { case "true", "false": default: return "it takes true or false" diff --git a/internal/format/targz/targz.go b/internal/format/targz/targz.go index 07e425c..6c07861 100644 --- a/internal/format/targz/targz.go +++ b/internal/format/targz/targz.go @@ -334,6 +334,15 @@ func describe(target int64, label string, m memo, groups []format.Content) forma // never has to read a missing key as flat. archive.Depth: m.layout.Depth, archive.DirectoryEntries: m.layout.DirEntries, + // What every entry says about itself. Neither of these reached the + // manifest until 2026-09-02, and the gap is what made a swallowed + // owner invisible: a run asking for entry_owner=USER produced an + // archive owned by nobody and a manifest identical to the one that + // asked for user, so there was nothing to disagree with. Written + // every time rather than only when stated, like depth and + // compression beside them. + archive.EntryMode: m.own.Stated.Mode, + archive.EntryOwner: m.own.Stated.Owner, // How hard the stream was squeezed. This key used to be the // constant "none", written when the format could only store - and // the comment beside it said so, which is why it is worth saying diff --git a/internal/guard/closedsets_test.go b/internal/guard/closedsets_test.go new file mode 100644 index 0000000..b95351e --- /dev/null +++ b/internal/guard/closedsets_test.go @@ -0,0 +1,185 @@ +package guard + +import ( + "errors" + "strings" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/format" + _ "github.com/donislawdev/TestingFilesGenerator/internal/format/all" + "github.com/donislawdev/TestingFilesGenerator/internal/format/archive" +) + +// A closed set is closed, and the registry is what says so. +// +// It did not. Property.Allows compared a choice with EqualFold and a boolean +// with ToLower, so a value spelled in a case the declaration does not use went +// straight past the registry and arrived at the generator - and what happened +// next was decided by whichever generator caught it. Measured on 2026-09-02 +// across all eight formats that declare a closed set, twenty one settings, +// there were FOUR different answers and not one of them was declared: +// +// refuses in its own words csv, ico, log, wav - the two voices of O168 +// folds and understands it pdf page_size, directory_entries in both archives +// swallows it and makes the targz entry_owner - USER and ROOT come out byte +// DEFAULT file, exit 0 for byte identical to unset, with nothing said +// reads it as another value zip encryption=NONE, which then demands a +// password "to lock with NONE" +// +// The third one is rule 6 broken outright: a file that is not what was ordered, +// reported as success. The fourth quotes a value the format does not have. +// +// So the fix is one place rather than twenty one: the registry stops folding, +// and no generator is ever handed a value its declaration does not contain. +// These guards are the reason that stays true for the format added tomorrow. +func TestAValueTheDeclarationDoesNotSpellIsRefusedByTheRegistry(t *testing.T) { + asked, skipped := 0, 0 + + for _, d := range format.All() { + for _, p := range d.Properties { + values := p.Choices + if p.Kind == format.PropertyBool { + values = []string{"true", "false"} + } + if p.Kind != format.PropertyChoice && p.Kind != format.PropertyBool { + continue + } + for _, v := range values { + shouted := strings.ToUpper(v) + if shouted == v { + // A value with no letters in it - a permission like 644, a + // bit depth like 16 - cannot be spelled in another case at + // all. Counted rather than passed over silently, because a + // day when every value is a number is a day this guard + // walks nothing and says so. + skipped++ + continue + } + asked++ + t.Run(d.ID+" "+p.Name+"="+shouted, func(t *testing.T) { + err := d.CheckProperties(map[string]string{p.Name: shouted}) + if err == nil { + t.Fatalf("%s took %s=%s, which its declaration spells %q. "+ + "Whatever the generator does with it next is its own "+ + "decision, and there are four of those in this tree", + d.ID, p.Name, shouted, v) + } + + // Refused is not enough. It has to be refused HERE, in the + // declaration's words, or the two voices are back with one + // of them further away. + var bad *format.PropertyValueError + if !errors.As(err, &bad) { + t.Fatalf("%s refused %s=%s with %T, not with the "+ + "registry's own refusal: %v", d.ID, p.Name, shouted, err, err) + } + if bad.Key != p.Name { + t.Errorf("the refusal names %q, and the setting is %q", bad.Key, p.Name) + } + if bad.Value != shouted { + t.Errorf("the refusal quotes %q, and the value was %q", bad.Value, shouted) + } + // The declared spelling still works, or this guard would be + // green on a registry that refuses everything. + if err := d.CheckProperties(map[string]string{p.Name: v}); err != nil { + t.Errorf("%s refuses %s=%s, which is its own declared value: %v", + d.ID, p.Name, v, err) + } + }) + } + } + } + + if asked == 0 { + t.Fatalf("no closed set carried a letter, so this guard asked nothing "+ + "(%d values had no letters to shout)", skipped) + } + t.Logf("%d values asked in a case the declaration does not use, %d had no letters", asked, skipped) +} + +// An owner the archive does not know is refused, rather than quietly becoming +// no owner at all. +// +// ReadOwnership was a switch with no default: anything that was not root or +// user fell through to the zero value, which is the same archive as unset. The +// registry now stops a misspelling before it gets here, so this is about the +// other door - the function is callable directly, and a guard calling it is +// exactly such a caller. Silence there would be a file that is not what was +// asked for, which is rule 6 whichever door it came through. +func TestAnOwnerTheArchiveDoesNotKnowIsRefusedRatherThanDropped(t *testing.T) { + for _, raw := range []string{"USER", "nobody", "root "} { + t.Run(raw, func(t *testing.T) { + own, err := archive.ReadOwnership("targz", map[string]string{archive.EntryOwner: raw}) + if err == nil { + t.Fatalf("an owner of %q was taken and turned into %+v, so the archive "+ + "is not the one that was ordered and nothing said so", raw, own) + } + var bad *format.PropertyValueError + if !errors.As(err, &bad) { + t.Fatalf("refused %q with %T, not with the refusal every other "+ + "setting uses: %v", raw, err, err) + } + if bad.Key != archive.EntryOwner { + t.Errorf("the refusal names %q, and the setting is %q", bad.Key, archive.EntryOwner) + } + }) + } + + // Every declared owner still passes, or the guard above would be satisfied + // by a function that refuses everything. + for _, owner := range []string{archive.OwnerRoot, archive.OwnerUser, archive.OwnerUnset} { + if _, err := archive.ReadOwnership("targz", map[string]string{archive.EntryOwner: owner}); err != nil { + t.Errorf("%q is a declared owner and was refused: %v", owner, err) + } + } + // And an absent setting is not a misspelling. It is the common case. + if _, err := archive.ReadOwnership("targz", map[string]string{}); err != nil { + t.Errorf("an archive with no owner setting was refused: %v", err) + } +} + +// The manifest says who owns the entries and what mode they carry. +// +// It said neither, and that is why the swallowed owner above was invisible: the +// file was wrong, the run said success, and the one document a test suite reads +// had nothing to disagree with. Measured on 2026-09-02 - a run with +// entry_owner=user and one with entry_owner=USER produced manifests that were +// identical, because both were silent. +// +// Written every time rather than only when set, like depth and compression +// beside them, so a harness never has to read a missing key as "nobody owns +// this". +func TestTheArchiveManifestSaysWhoOwnsTheEntries(t *testing.T) { + d, err := format.Get("targz") + if err != nil { + t.Fatal(err) + } + + for _, tc := range []struct { + props map[string]string + mode, wner string + }{ + {map[string]string{}, "644", archive.OwnerUnset}, + {map[string]string{archive.EntryOwner: archive.OwnerRoot}, "644", archive.OwnerRoot}, + {map[string]string{archive.EntryMode: "755", archive.EntryOwner: archive.OwnerUser}, + "755", archive.OwnerUser}, + } { + name := tc.mode + "/" + tc.wner + t.Run(name, func(t *testing.T) { + p, err := d.Generator.Plan(format.Request{ + Bytes: 20480, Seed: 3, Label: true, Properties: tc.props, + }) + if err != nil { + t.Fatalf("planning %v: %v", tc.props, err) + } + if got := p.Properties[archive.EntryMode]; got != tc.mode { + t.Errorf("the manifest says entry_mode %v, and the archive was built with %s", + got, tc.mode) + } + if got := p.Properties[archive.EntryOwner]; got != tc.wner { + t.Errorf("the manifest says entry_owner %v, and the archive was built with %s", + got, tc.wner) + } + }) + } +}