From c09020c2fb01da24d5befbed78ce3b73b5efbe4c Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Mon, 14 Sep 2026 23:39:37 +0200 Subject: content tests are three-valued: unknown when unreadable or read in part --- internal/cond/eval.go | 139 +++++++++++++++++++++++++++++----------- internal/cond/eval_test.go | 33 +++++++++- internal/engine/exclude_test.go | 75 ++++++++++++++++++++++ internal/engine/facts.go | 9 ++- internal/extract/extract.go | 5 ++ internal/extract/zipxml.go | 7 +- internal/extract/zipxml_test.go | 6 +- 7 files changed, 227 insertions(+), 47 deletions(-) (limited to 'internal') 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(`` + body + ``)) + 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": `good body text`, "word/footer1.xml": `broken`, }), 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) -- cgit v1.3