diff options
| author | Lukasz Kasprzak <lukas@labunix.xyz> | 2026-09-14 23:39:37 +0200 |
|---|---|---|
| committer | Lukasz Kasprzak <lukas@labunix.xyz> | 2026-09-14 23:39:37 +0200 |
| commit | c09020c2fb01da24d5befbed78ce3b73b5efbe4c (patch) | |
| tree | 8cd1086588988fafaf238aed8a38a7ec85b0afa3 | |
| parent | 3990586a85e91923c0a3ae1a1f0f61420db555ac (diff) | |
| download | krino-c09020c2fb01da24d5befbed78ce3b73b5efbe4c.tar.gz krino-c09020c2fb01da24d5befbed78ce3b73b5efbe4c.zip | |
content tests are three-valued: unknown when unreadable or read in part
| -rw-r--r-- | CHANGELOG.md | 7 | ||||
| -rw-r--r-- | docs/design.md | 26 | ||||
| -rw-r--r-- | internal/cond/eval.go | 139 | ||||
| -rw-r--r-- | internal/cond/eval_test.go | 33 | ||||
| -rw-r--r-- | internal/engine/exclude_test.go | 75 | ||||
| -rw-r--r-- | internal/engine/facts.go | 9 | ||||
| -rw-r--r-- | internal/extract/extract.go | 5 | ||||
| -rw-r--r-- | internal/extract/zipxml.go | 7 | ||||
| -rw-r--r-- | internal/extract/zipxml_test.go | 6 | ||||
| -rw-r--r-- | man/krino.1 | 11 | ||||
| -rw-r--r-- | man/krino.conf.5 | 24 |
11 files changed, 283 insertions, 59 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 78dbcd8..5c4c0bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,13 @@ on (a self-extracting installer) has no content, like an image: it no longer matches a content test, is not set aside by a content exclude, and raises no warning. +- A content test that cannot read its file is unknown rather than false, + and `and`/`or`/`not` combine unknowns by three-valued logic. A rule never + matches on an unknown, so `(not (content "x"))` no longer acts on a file + krino could not read; an exclude holds on an unknown only when the text + could change its answer; `explain` shows `?`. A document read only in part + answers the keywords found in what was read, and leaves the others + unknown. - `{1}` … `{9}` keep the name's diacritics: a folded `name` test still matches `Lodz`, but `Łódź-faktura.pdf` now files under `Łódź`, not `Lodz`. - A tool's error message is shown even when it writes more than the diff --git a/docs/design.md b/docs/design.md index 35b0de0..4196894 100644 --- a/docs/design.md +++ b/docs/design.md @@ -199,10 +199,12 @@ as "excluded" in the plan, listed with the form that matched under `-v`, and traced by `explain`; `check` lists every directory's exclusions. Since no rule has run yet, `(matched)` is never true inside an exclude. -An exclude fails closed: when evaluating it reaches a content test that +An exclude fails closed: when its value depends on a content test that cannot read the file (over `max-read`, a tool missing, failing or timing -out), the exclude holds, and the file is set aside as "(content -unreadable)" with the warning. A file whose format has no text at all (an +out, or a document read only in part), the exclude holds, and the file is +set aside as "(content unreadable)" with the warning (§5.4 rule 4). One that +is false whatever the text holds, such as `(exclude (content "x") (type +txt))` on a pdf, does not. A file whose format has no text at all (an image, an archive) is not unreadable: it contains no keyword, so its content tests are simply false and raise no warning. So is a file of no known extension that starts as text and holds binary data further on, such @@ -258,10 +260,16 @@ UTF-8 is replaced by U+FFFD before folding. cheapest first: `type`, `size`, `age` and `matched`, then `name` and `path`, then `duplicate`, then `content`. Content is extracted at most once per file, and only if evaluation reaches a `content` test. -4. If a file's text cannot be extracted, `content` is false and the plan - shows a warning naming the rule that wanted it. A format with no text - (§6, "anything else") is not a failure: `content` is false, silently. Inside an `exclude` the - whole exclude then holds instead (§4.6). +4. If a file's text cannot be extracted, `content` is **unknown** and the + plan shows a warning naming the rule that wanted it. A document read only + in part (an archive entry that would not open) answers a keyword found in + what was read; one not found is unknown. `and`, `or` and `not` combine + unknowns by three-valued (Kleene) logic: `and` is false if any argument is + false, `or` true if any is true, `not` of unknown is unknown, so an + unknown only reaches the top where the text could change the answer. A + rule matches only when its condition is true; an unknown exclude holds + (§4.6). `explain` shows an unknown test as `?`. A format with no text (§6, + "anything else") is not a failure: `content` is false, silently. ### 5.5 Duplicates @@ -896,7 +904,9 @@ What that means, and the tests that hold it (plans 8 and 9): - Undo restores a trash entry only while it is still the file the run put there (size, mtime, recorded path), re-checks every file at execution time, and can finish an undo that stopped part way. -- An exclude whose content test cannot read the file holds (fails closed). +- An exclude whose value depends on a content test that cannot read the + file holds (fails closed); a rule whose value does is not matched, so + `(not (content "x"))` never acts on a file krino could not read. - A file reaches an external tool only as an absolute path, so a name starting with `-` is never read as an option. - A trash entry name from the log must be a plain name inside the Trash, diff --git a/internal/cond/eval.go b/internal/cond/eval.go index f241504..c9f7f75 100644 --- a/internal/cond/eval.go +++ b/internal/cond/eval.go @@ -34,9 +34,11 @@ type Result struct { Reasons []string // what made it true, e.g. `type pdf`, `content "acme ltd"`, `name "\bacme\b"` Warnings []string // e.g. `content unreadable: needs pdftotext, not installed` - // Unreadable is true when evaluation reached a content test that could - // not read the file. The test counts as false; an exclude uses this to - // hold anyway (review M11). + // Unreadable is true when the condition's value is unknown: it depends + // on a content test that could not read the file. Match is then false; + // an exclude holds anyway (review M11). A condition decided whatever the + // text holds - (and (content "x") (type txt)) on a pdf - is not + // unknown (plan 11). Unreadable bool } @@ -44,18 +46,29 @@ type Result struct { type Trace struct { Label string Value bool + Unknown bool // the value depends on a content test that could not read the file Err string Children []*Trace } +// tri is a three-valued truth value: a content test that cannot read its +// file is unknown, and and/or/not follow Kleene's logic, so an unknown only +// spreads where the text could change the answer. +type tri int8 + +const ( + no tri = iota + yes + unknown +) + // evalCtx accumulates state across one Eval call: the captures of the // first true, non-negated name test, and warnings de-duplicated in // first-seen order. type evalCtx struct { - captures []string - warned map[string]bool - warnings []string - unreadable bool + captures []string + warned map[string]bool + warnings []string } // warn records msg unless it has already been recorded. @@ -80,38 +93,54 @@ func (c *Cond) Eval(f Facts) Result { return Result{Match: true, Reasons: []string{"no condition"}} } ctx := &evalCtx{} - match, reasons := c.eval(c.root, f, ctx, false) - return Result{Match: match, Captures: ctx.captures, Reasons: reasons, Warnings: ctx.warnings, Unreadable: ctx.unreadable} + v, reasons := c.eval(c.root, f, ctx, false) + return Result{Match: v == yes, Captures: ctx.captures, Reasons: reasons, Warnings: ctx.warnings, Unreadable: v == unknown} } // eval evaluates one node against f, short-circuiting and/or in child // (cost-sorted) order. negated tracks whether n is reached under an odd // number of enclosing nots, so a matching name test found there does not // supply Result.Captures. -func (c *Cond) eval(n *node, f Facts, ctx *evalCtx, negated bool) (bool, []string) { +func (c *Cond) eval(n *node, f Facts, ctx *evalCtx, negated bool) (tri, []string) { switch n.kind { case kAnd: + // A false child decides; an unknown one does not, so the rest still + // run (one of them may be false). var reasons []string + v := yes for _, ch := range n.children { - ok, r := c.eval(ch, f, ctx, negated) - if !ok { - return false, nil + cv, r := c.eval(ch, f, ctx, negated) + switch cv { + case no: + return no, nil + case unknown: + v = unknown } reasons = append(reasons, r...) } - return true, reasons + if v == unknown { + return unknown, nil + } + return yes, reasons case kOr: + v := no for _, ch := range n.children { - if ok, r := c.eval(ch, f, ctx, negated); ok { - return true, r + cv, r := c.eval(ch, f, ctx, negated) + switch cv { + case yes: + return yes, r + case unknown: + v = unknown } } - return false, nil + return v, nil case kNot: child := n.children[0] - ok, _ := c.eval(child, f, ctx, !negated) - if ok { - return false, nil + switch cv, _ := c.eval(child, f, ctx, !negated); cv { + case yes: + return no, nil + case unknown: + return unknown, nil } // E4: a negated leaf reads fine as "not " plus the leaf's own // label ("not matched", "not type pdf"), but a negated and/or's @@ -123,22 +152,20 @@ func (c *Cond) eval(n *node, f Facts, ctx *evalCtx, negated bool) (bool, []strin if child.kind == kAnd || child.kind == kOr { label = "(" + child.label + " ...)" } - return true, []string{"not " + label} + return yes, []string{"not " + label} default: ok, reason, warn, caps := c.evalLeaf(n, f) - if warn != "" { - ctx.warn(warn) - if n.kind == kContent { - ctx.unreadable = true - } + ctx.warn(warn) + if warn != "" && n.kind == kContent { + return unknown, nil } if !ok { - return false, nil + return no, nil } if caps != nil && !negated && ctx.captures == nil { ctx.captures = caps } - return true, []string{reason} + return yes, []string{reason} } } @@ -288,28 +315,61 @@ func (c *Cond) explain(n *node, f Facts) *Trace { switch n.kind { case kAnd, kOr: t := &Trace{Label: n.label} - val := n.kind == kAnd // identity: and starts true, or starts false + // Kleene: and is false on any false child, or on any true one. + decide, other := no, yes + if n.kind == kOr { + decide, other = yes, no + } + v := other for _, ch := range n.children { ct := c.explain(ch, f) t.Children = append(t.Children, ct) - if n.kind == kAnd { - val = val && ct.Value - } else { - val = val || ct.Value + switch cv := ct.tri(); { + case cv == decide: + v = decide + case cv == unknown && v != decide: + v = unknown } } - t.Value = val + t.set(v) return t case kNot: ct := c.explain(n.children[0], f) - return &Trace{Label: n.label, Value: !ct.Value, Children: []*Trace{ct}} + t := &Trace{Label: n.label, Children: []*Trace{ct}} + switch ct.tri() { + case yes: + t.set(no) + case no: + t.set(yes) + default: + t.set(unknown) + } + return t default: ok, _, warn, _ := c.evalLeaf(n, f) - return &Trace{Label: n.label, Value: ok, Err: warn} + t := &Trace{Label: n.label, Value: ok, Err: warn} + if warn != "" && n.kind == kContent { + t.set(unknown) + } + return t } } -// Format writes one line per node: "yes"/"no " padded to three, two +func (t *Trace) tri() tri { + switch { + case t.Unknown: + return unknown + case t.Value: + return yes + } + return no +} + +func (t *Trace) set(v tri) { + t.Value, t.Unknown = v == yes, v == unknown +} + +// Format writes one line per node: "yes", "no" or "?" (unknown) padded to three, two // spaces, two spaces of indent per depth, the label, and " (Err)" when // Err is set. func (t *Trace) Format(w io.Writer) { @@ -318,7 +378,10 @@ func (t *Trace) Format(w io.Writer) { func (t *Trace) format(w io.Writer, depth int) { word := "no" - if t.Value { + switch { + case t.Unknown: + word = "?" + case t.Value: word = "yes" } fmt.Fprintf(w, "%-3s %s%s", word, strings.Repeat(" ", depth), t.Label) diff --git a/internal/cond/eval_test.go b/internal/cond/eval_test.go index 2e8bcfd..7d796a1 100644 --- a/internal/cond/eval_test.go +++ b/internal/cond/eval_test.go @@ -167,11 +167,11 @@ func TestExplainFormat(t *testing.T) { c, _ := Compile("d.conf", nodes(t, `(type pdf) (or (content "acme ltd") (name "\bacme\b"))`), Options{IgnoreCase: true}) var b strings.Builder c.Explain(f).Format(&b) - want := "no and\n" + + want := "? and\n" + "yes type pdf\n" + - "no or\n" + + "? or\n" + "no name \"\\bacme\\b\"\n" + - "no content \"acme ltd\" (content unreadable: needs pdftotext, not installed)\n" + "? content \"acme ltd\" (content unreadable: needs pdftotext, not installed)\n" if b.String() != want { t.Fatalf("got\n%s\nwant\n%s", b.String(), want) } @@ -257,3 +257,30 @@ func TestCapturesKeepDiacritics(t *testing.T) { t.Errorf("captures with a group that did not take part = %q, want %q", r.Captures, want) } } + +// TestUnreadableContentIsUnknown: a content test that cannot read the file +// is unknown, not false, and and/or/not combine unknowns the way Kleene's +// three-valued logic does: a condition certainly false (or true) whatever +// the text holds is decided, and only one that depends on the text is +// unknown (plan 11, re-review cache F2). A rule matches only a true +// condition; an exclude holds on true or unknown. +func TestUnreadableContentIsUnknown(t *testing.T) { + f := &fake{name: "a.pdf", rawErr: errors.New("larger than max-read")} + cases := []struct { + src string + match, unreadable bool + }{ + {`(and (content "x") (type txt))`, false, false}, + {`(or (content "x") (type pdf))`, true, false}, + {`(not (content "x"))`, false, true}, + {`(and (type pdf) (content "x"))`, false, true}, + {`(or (type txt) (content "x"))`, false, true}, + {`(content "secret") (and (content "other") (type txt))`, false, false}, + {`(not (and (content "x") (type txt)))`, true, false}, + } + for _, c := range cases { + if r := eval(t, c.src, Options{}, f); r.Match != c.match || r.Unreadable != c.unreadable { + t.Errorf("%s: Match %v Unreadable %v; want %v, %v", c.src, r.Match, r.Unreadable, c.match, c.unreadable) + } + } +} diff --git a/internal/engine/exclude_test.go b/internal/engine/exclude_test.go index e183268..ceb8442 100644 --- a/internal/engine/exclude_test.go +++ b/internal/engine/exclude_test.go @@ -3,6 +3,8 @@ package engine import ( + "archive/zip" + "bytes" "context" "os" "path/filepath" @@ -351,3 +353,76 @@ func TestPlannedDestinationKeepsDiacritics(t *testing.T) { t.Fatalf("chains = %+v; want the move into Out/Łódź", dp.Chains) } } + +// partDocx writes a docx whose body reads and whose footer uses a +// compression method Go cannot read: a partly readable document. +func partDocx(t *testing.T, path, body string) { + t.Helper() + var buf bytes.Buffer + w := zip.NewWriter(&buf) + f, err := w.Create("word/document.xml") + if err != nil { + t.Fatal(err) + } + f.Write([]byte(`<w:document xmlns:w="w"><w:body><w:p><w:r><w:t>` + body + `</w:t></w:r></w:p></w:body></w:document>`)) + raw, err := w.CreateRaw(&zip.FileHeader{Name: "word/footer1.xml", Method: 12}) + if err != nil { + t.Fatal(err) + } + raw.Write([]byte("BZh9 not really bzip2 confidential")) + if err := w.Close(); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, buf.Bytes(), 0o644); err != nil { + t.Fatal(err) + } + old := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + os.Chtimes(path, old, old) +} + +// TestPartlyReadableDocument: a docx with an unreadable part answers a +// keyword it holds in the readable part, but a keyword not found there is +// unknown: a content exclude sets the file aside, a rule warns (plan 11, +// re-review cache F3). +func TestPartlyReadableDocument(t *testing.T) { + h, dl := excludeTree(t, map[string]string{}) + os.MkdirAll(dl, 0o755) + partDocx(t, filepath.Join(dl, "part.docx"), "good body text") + main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": ` +(path "~/dl") +(exclude (content "confidential")) +(rule "all" (move "Out")) +`}) + e, errs := Load(main) + if len(errs) > 0 { + t.Fatal(errs) + } + r, err := e.Match(context.Background(), e.Dirs[0]) + if err != nil { + t.Fatal(err) + } + if len(r.Matched) != 1 || !strings.HasSuffix(r.Matched[0].Excluded, "(content unreadable)") { + t.Fatalf("matched = %+v; want part.docx set aside as unreadable", r.Matched) + } + + os.WriteFile(filepath.Join(h, ".config", "krino", "dirs", "dl.conf"), []byte(` +(path "~/dl") +(rule "body" (when (content "good body")) (move "Docs")) +(rule "other" (when (content "nowhere")) (move "Other")) +`), 0o644) + e, errs = Load(main) + if len(errs) > 0 { + t.Fatal(errs) + } + r, err = e.Match(context.Background(), e.Dirs[0]) + if err != nil { + t.Fatal(err) + } + fm := r.Matched[0] + if len(fm.Rules) != 1 || fm.Rules[0].Rule.Name != "body" { + t.Errorf("rules = %+v; want only body, found in the readable part", fm.Rules) + } + if len(fm.Warnings) == 0 { + t.Errorf("no warning for the keyword the unreadable part might hold") + } +} diff --git a/internal/engine/facts.go b/internal/engine/facts.go index ccdf18d..c510b20 100644 --- a/internal/engine/facts.go +++ b/internal/engine/facts.go @@ -118,6 +118,7 @@ type facts struct { contentDone bool // extraction was attempted contentErr error // why it failed + partialErr error // extract.ErrPartial: a keyword not found may be in the unread part answers map[string]bool // by cond.KeywordKey, once extracted } @@ -171,7 +172,7 @@ func (f *facts) ContentContains(opt cond.Options, keywords []string) (int, error return i, nil } } - return -1, nil + return -1, f.partialErr } // extract reads the file's text and answers every keyword of the directory, @@ -180,6 +181,10 @@ func (f *facts) extract(opt cond.Options, keywords []string) { f.contentDone = true text, err := f.run.e.Extract.Text(f.run.ctx, f.file.Path, f.file.Size, f.run.d.Settings.MaxRead) switch { + case errors.Is(err, extract.ErrPartial): + // Partly read: the keywords found in it are answered; one not found + // is unknown (ContentContains), and nothing is cached (plan 11). + f.partialErr = err case errors.Is(err, extract.ErrUnsupported): // A format with no text cannot contain a keyword: every answer is // no, with no warning, and the answers are cached like any other @@ -203,7 +208,7 @@ func (f *facts) extract(opt cond.Options, keywords []string) { } f.answers[k.Key()] = strings.Contains(t, k.Norm) } - if id, ok := f.cacheID(); ok { + if id, ok := f.cacheID(); ok && f.partialErr == nil { f.run.cache.Store(id, f.answers) } } diff --git a/internal/extract/extract.go b/internal/extract/extract.go index 7e3adec..0843c42 100644 --- a/internal/extract/extract.go +++ b/internal/extract/extract.go @@ -23,6 +23,11 @@ var ( // ErrUnsupported is returned when the format carries no text krino // knows how to extract. ErrUnsupported = errors.New("no text in this format") + // ErrPartial wraps the error of a document read only in part (an + // archive entry that would not open or parse). Text returns the text it + // did read along with it: a keyword found there is found, but one not + // found may be in the part that could not be read (plan 11). + ErrPartial = errors.New("part of it could not be read") // ErrTooLarge is returned when the file is larger than the configured // max-read; nothing is read in that case. ErrTooLarge = errors.New("larger than max-read") diff --git a/internal/extract/zipxml.go b/internal/extract/zipxml.go index 1b3ba82..3b59d3a 100644 --- a/internal/extract/zipxml.go +++ b/internal/extract/zipxml.go @@ -102,8 +102,11 @@ func zipText(ctx context.Context, path, ext string, maxRead int64) (string, erro } b.WriteByte('\n') } - if !wroteText && firstErr != nil { - return "", firstErr + if firstErr != nil { + if !wroteText { + return "", firstErr + } + return b.String(), fmt.Errorf("%w: %w", ErrPartial, firstErr) } return b.String(), nil } diff --git a/internal/extract/zipxml_test.go b/internal/extract/zipxml_test.go index 14ead80..30a4cf9 100644 --- a/internal/extract/zipxml_test.go +++ b/internal/extract/zipxml_test.go @@ -138,8 +138,10 @@ func TestZipLenientOnMalformedEntry(t *testing.T) { "word/document.xml": `<w:document xmlns:w="w"><w:body><w:p><w:r><w:t>good body text</w:t></w:r></w:p></w:body></w:document>`, "word/footer1.xml": `<w:ftr xmlns:w="w"><w:p><w:r><w:t>broken</w:t></w:r></w:p></w:xyz>`, }), 0) - if err != nil { - t.Fatalf("good entry alongside a malformed one: %v", err) + // Partly read: the text is kept, and ErrPartial says some of it is + // missing, so a keyword not found in it is unknown (plan 11). + if !errors.Is(err, ErrPartial) { + t.Fatalf("good entry alongside a malformed one: err %v, want ErrPartial", err) } if !strings.Contains(got, "good body text") { t.Errorf("text from the good entry was discarded: %q", got) diff --git a/man/krino.1 b/man/krino.1 index 000fd95..2761048 100644 --- a/man/krino.1 +++ b/man/krino.1 @@ -193,8 +193,15 @@ checks every included directory. .It Ic explain Ar file Evaluate every exclusion and rule of .Ar file Ns 's -directory against it and show each test's result, so a rule that should -match but does not +directory against it and show each test's result +.Po +.Ql yes , +.Ql no , +or +.Ql \&? +for a result that depends on content krino could not read +.Pc , +so a rule that should match but does not .Pq or the reverse can be diagnosed test by test. See diff --git a/man/krino.conf.5 b/man/krino.conf.5 index 26da541..c2df331 100644 --- a/man/krino.conf.5 +++ b/man/krino.conf.5 @@ -274,11 +274,14 @@ can test size and content. .Pp An .Ic exclude -fails closed: when a content test it reaches cannot read the file -.Pq over Ic max-read , No or a tool missing, failing or timing out , +fails closed: when its value depends on a content test that cannot read the +file +.Pq over Ic max-read , No a tool missing, failing or timing out, or a document read only in part , the exclude holds and the file is set aside as .Dq (content unreadable) , with a warning. +An exclude that is false whatever the text holds does not +.Pq see Sx Operators . A file whose format has no text at all, such as an image or an archive, is not unreadable: its content tests are false, with no warning. So is a file of no known extension that starts as text and holds binary @@ -365,6 +368,23 @@ and take one or more arguments; .Ic not exactly one. +.Pp +A +.Ic content +test that cannot read its file is unknown, with a warning; a document read +only in part answers the keywords found in what was read and leaves the +others unknown. +Operators combine unknowns by three-valued logic: +.Ic and +is false if any argument is false, +.Ic or +true if any is true, and +.Ic not +of unknown is unknown. +A rule matches only when its condition is true, so +.Ql (not (content \(dqx\(dq)) +never acts on a file krino could not read; an exclusion holds when its +condition is true or unknown. .Ss Tests .Bl -tag -width Ds .It Ic (type Ar t No ...) |
