Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
42 changes: 40 additions & 2 deletions internal/format/archive/ownership.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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
}
12 changes: 8 additions & 4 deletions internal/format/csvfile/dialect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
18 changes: 16 additions & 2 deletions internal/format/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
9 changes: 9 additions & 0 deletions internal/format/targz/targz.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
185 changes: 185 additions & 0 deletions internal/guard/closedsets_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
Loading