diff --git a/README.md b/README.md index fca9ce9..8db29e8 100644 --- a/README.md +++ b/README.md @@ -931,6 +931,7 @@ not told believes the document safe to send. It refuses a region holding: | text inside a Form XObject | the bytes are not the page's, and an XObject is **shared** — cleaning one page would change every page that draws it. `RecolourText` stops at the same wall | | an image | the spec is explicit that a mask or a clip will not do and the pixels must be destroyed; re-encoding an image with them overwritten is not in this release | | an annotation | its own `/Contents` can hold the very words being redacted | +| an annotation **this document has added** and not yet written | a mark or a widget is held until the write, because each names the page *object* it sits on, so neither is in the source file — and reading only the source missed them. The workbench walked into it: highlight a line, then decide to redact it, and the text went while the note quoting it stayed. Drop the mark or the field first | | an operator only **part** of whose text is inside the region | blanking it would remove text nobody asked about, and the surviving words cannot simply be re-emitted: the bytes in the stream are font codes, and Unicode does not round-trip back to them through a CID font's map. Keeping the survivors' original bytes and replacing the removed ones with an equivalent kern is the way, and it is next | | text that **reaches outside** the region | a run is the unit that can be removed, so a producer drawing a whole line with one `Tj` makes that line the run — and redacting one word of it would take the line. That is not a leak, which is why it is tempting to allow; it is the other way a redaction betrays its caller, by removing what they did not ask about and not saying so. A region is allowed to miss a run's own box by up to two points, because a caller mapping a lit rectangle back into page space arrives through screen pixels | diff --git a/redact.go b/redact.go index d271a75..62e8ff9 100644 --- a/redact.go +++ b/redact.go @@ -128,6 +128,42 @@ func (d *Doc) checkRedactable(p *Page, r Redaction) error { r.Page) } } + // ⛔⛔ THE ANNOTATIONS THIS DOCUMENT HAS ADDED COUNT TOO, and reading only + // the source page missed them. Held until the write, because a widget and a + // markup annotation both name the page OBJECT they sit on, they are not in + // p.src at all — so a caller that marked a passage and then redacted it got + // the text removed and the annotation kept, with the marked-up words still + // in the file as that annotation's own note. The workbench walked straight + // into it: highlight a line, then decide to redact it. + for _, m := range d.marks { + if m.Page != p.number { + continue + } + for _, q := range m.Quads { + x0, y0, x1, y1 := q.bounds() + if overlaps(x0, y0, x1, y1, r.Quads) { + return fmt.Errorf("ops: page %d carries a mark this document has added across "+ + "the region, and its own note can hold the words being redacted: drop "+ + "the mark first", r.Page) + } + } + } + // ⛔ A field's places are its WIDGETS, each with its own page: one field can + // show on several pages, so the field is not the thing with a position. + for _, f := range d.addFields { + for _, wdg := range f.Widgets { + if wdg.Page != p.number { + continue + } + x0, y0 := min(wdg.Rect[0], wdg.Rect[2]), min(wdg.Rect[1], wdg.Rect[3]) + x1, y1 := max(wdg.Rect[0], wdg.Rect[2]), max(wdg.Rect[1], wdg.Rect[3]) + if overlaps(x0, y0, x1, y1, r.Quads) { + return fmt.Errorf("ops: page %d carries a form field this document has added "+ + "across the region, and a field holds a value of its own: drop the "+ + "field first", r.Page) + } + } + } page, _ := p.src.Page(p.number) if annots, ok := resolveArray(p.src, page.Get("Annots")); ok { for _, a := range annots { diff --git a/redact_test.go b/redact_test.go index 7e9856b..f7ba792 100644 --- a/redact_test.go +++ b/redact_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/go-pdfkit/extract" + "github.com/go-pdfkit/forms" "github.com/go-pdfkit/reader" ) @@ -233,7 +234,8 @@ func TestRedactingRefusesWhatItCannotRemove(t *testing.T) { assertRefused(t, pdf, overSecret(), "image") }) - // An annotation: its own contents can hold the words being redacted. + // An annotation the file already carries: its own contents can hold the + // words being redacted. t.Run("an annotation across the region", func(t *testing.T) { d, err := Open(secretPage(t, twoLines, nil)) if err != nil { @@ -250,6 +252,87 @@ func TestRedactingRefusesWhatItCannotRemove(t *testing.T) { assertRefused(t, out, overSecret(), "annotation") }) + // ⛔⛔ AND ONE THIS DOCUMENT HAS ADDED BUT NOT YET WRITTEN. Marks and + // widgets are held until the write, because each names the page OBJECT it + // sits on — so they are not in the source at all, and reading only the + // source missed them. The workbench walked straight into it: highlight a + // line, then decide to redact it, and the text went while the note that + // quoted it stayed. + t.Run("a mark this document has added", func(t *testing.T) { + d, err := Open(secretPage(t, twoLines, nil)) + if err != nil { + t.Fatal(err) + } + if err := d.Mark(Markup{Page: 1, Kind: MarkHighlight, + Quads: []Quad{QuadOf(20, 98, 60, 112)}, Note: "the secret, again"}); err != nil { + t.Fatal(err) + } + err = d.Redact(overSecret()) + if err == nil { + t.Fatal("a redaction under a mark this document had added was accepted") + } + if !strings.Contains(err.Error(), "drop the mark first") { + t.Errorf("the refusal says %q", err) + } + // A mark somewhere else on the page is nobody's business. + d2, err := Open(secretPage(t, twoLines, nil)) + if err != nil { + t.Fatal(err) + } + if err := d2.Mark(Markup{Page: 1, Kind: MarkHighlight, + Quads: []Quad{QuadOf(20, 148, 60, 162)}}); err != nil { + t.Fatal(err) + } + if err := d2.Redact(overSecret()); err != nil { + t.Errorf("a mark on another line stopped the redaction: %v", err) + } + // And a mark on another PAGE is not even looked at, however exactly it + // sits over the region's coordinates — a page number is not a position. + d3, err := Open(secretPage(t, twoLines, nil)) + if err != nil { + t.Fatal(err) + } + d3.Blank(100, 200) + if err := d3.Mark(Markup{Page: 2, Kind: MarkHighlight, + Quads: []Quad{QuadOf(15, 95, 85, 115)}}); err != nil { + t.Fatal(err) + } + if err := d3.Redact(overSecret()); err != nil { + t.Errorf("a mark on page 2 stopped a redaction on page 1: %v", err) + } + }) + + t.Run("a field this document has added", func(t *testing.T) { + d, err := Open(secretPage(t, twoLines, nil)) + if err != nil { + t.Fatal(err) + } + if err := d.AddFields(forms.Field{Name: "note", Kind: forms.Text, + Widgets: []forms.Widget{{Page: 1, Rect: [4]float64{20, 98, 60, 112}}}}); err != nil { + t.Fatal(err) + } + err = d.Redact(overSecret()) + if err == nil { + t.Fatal("a redaction under a field this document had added was accepted") + } + if !strings.Contains(err.Error(), "drop the field first") { + t.Errorf("the refusal says %q", err) + } + // A widget on another page is not in the way, wherever it sits. + d2, err := Open(secretPage(t, twoLines, nil)) + if err != nil { + t.Fatal(err) + } + d2.Blank(100, 200) + if err := d2.AddFields(forms.Field{Name: "note", Kind: forms.Text, + Widgets: []forms.Widget{{Page: 2, Rect: [4]float64{20, 98, 60, 112}}}}); err != nil { + t.Fatal(err) + } + if err := d2.Redact(overSecret()); err != nil { + t.Errorf("a field on page 2 stopped a redaction on page 1: %v", err) + } + }) + // One operator drawing both what goes and what stays. t.Run("an operator it would half-erase", func(t *testing.T) { const body = "BT /F1 10 Tf 20 100 Td [(keep) -2000 (drop)] TJ ET"