diff --git a/.gitattributes b/.gitattributes index 199ea9a55b..74d8a60212 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,2 +1,9 @@ *.sh text eol=lf docker/entrypoint.sh text eol=lf executable + +# Binary fixtures must be stored byte-for-byte. Mark them binary so git +# never applies text/CRLF normalization, and so the pre-commit text fixers +# (see tools/hooks/check_files.py) skip them. OLE2 .msg files in particular +# were previously corrupted by the mixed-line-ending / end-of-file fixers. +*.msg binary + diff --git a/go.mod b/go.mod index d460ccd1e3..81a245e863 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.26.4 require ( cloud.google.com/go/storage v1.63.0 + github.com/AkmalOt/gomsg v0.0.0-20260407083308-985c3a1a76b7 github.com/DATA-DOG/go-sqlmock v1.5.2 github.com/LuxorLabs/tenki-sdk-go/sandbox v0.7.0 github.com/alibabacloud-go/agentrun-20250910/v5 v5.8.4 @@ -232,3 +233,5 @@ require ( ) replace github.com/infiniflow/infinity-go-sdk => github.com/infiniflow/infinity/go v0.0.0-20260806040857-d755c5ad25d9 + +replace github.com/AkmalOt/gomsg => github.com/xugangqiang/gomsg v0.0.0-20260407083308-985c3a1a76b7 diff --git a/go.sum b/go.sum index 25e50f4434..10b93787e2 100644 --- a/go.sum +++ b/go.sum @@ -526,6 +526,8 @@ github.com/wk8/go-ordered-map/v2 v2.1.8 h1:5h/BUHu93oj4gIdvHHHGsScSTMijfx5PeYkE/ github.com/wk8/go-ordered-map/v2 v2.1.8/go.mod h1:5nJHM5DyteebpVlHnWMV0rPz6Zp7+xBAnxjb1X5vnTw= github.com/x-cray/logrus-prefixed-formatter v0.5.2 h1:00txxvfBM9muc0jiLIEAkAcIMJzfthRT6usrui8uGmg= github.com/x-cray/logrus-prefixed-formatter v0.5.2/go.mod h1:2duySbKsL6M18s5GU7VPsoEPHyzalCE06qoARUCeBBE= +github.com/xugangqiang/gomsg v0.0.0-20260407083308-985c3a1a76b7 h1:iX9RUmlLw+Hoe78WV5vlm+JjIZUCzdRn2y6tWNxZTeY= +github.com/xugangqiang/gomsg v0.0.0-20260407083308-985c3a1a76b7/go.mod h1:TCjWm+lo7de/zYHb8rHfVSaKxOP4tZNrnRUzkqV4r28= github.com/xuri/efp v0.0.1 h1:fws5Rv3myXyYni8uwj2qKjVaRP30PdjeYe2Y6FDsCL8= github.com/xuri/efp v0.0.1/go.mod h1:ybY/Jr0T0GTCnYjKqmdwxyxn2BQf2RcQIIvex5QldPI= github.com/xuri/excelize/v2 v2.11.0 h1:HxaEFl6sRN2+8J5a8HaKq+0M4FsjBGMnWWtjOCPSG88= diff --git a/internal/parser/parser/email_parser.go b/internal/parser/parser/email_parser.go index 3fe86f5cf7..14bede0c39 100644 --- a/internal/parser/parser/email_parser.go +++ b/internal/parser/parser/email_parser.go @@ -22,24 +22,26 @@ import ( "encoding/base64" "fmt" "io" + "log" "mime" "mime/multipart" "mime/quotedprintable" "net/mail" "path/filepath" "strings" + "time" + "github.com/AkmalOt/gomsg" "golang.org/x/text/encoding" "golang.org/x/text/encoding/simplifiedchinese" "golang.org/x/text/transform" + + "ragflow/internal/utility" ) -// EmailParser parses .eml (RFC 5322 email) files into structured -// JSON or plain-text output. Mirrors Python's _email() method in -// rag/flow/parser/parser.py. -// -// .msg (Outlook) files are not supported in the Go path; callers -// receive a clear error. +// EmailParser parses .eml (RFC 5322 email) and .msg (Outlook OLE2) files +// into structured JSON or plain-text output. Mirrors Python's _email() +// method in rag/flow/parser/parser.py, whose .msg branch uses extract_msg. type EmailParser struct { fields []string outputFormat string @@ -75,32 +77,78 @@ func (p *EmailParser) ConfigureFromSetup(setup map[string]any) { } func (p *EmailParser) ParseWithResult(ctx context.Context, filename string, data []byte) ParseResult { - ext := strings.ToLower(filepath.Ext(filename)) - if ext == ".msg" { - return ParseResult{ - Err: fmt.Errorf("email: .msg (Outlook) files are not supported in the Go parser; use .eml format"), - } - } + return p.parseEmail(ctx, filename, data, 0) +} - emailContent := parseEML(bytes.NewReader(data), p.fields) +// parseEmail is ParseWithResult with a re-chunk depth so nested email +// attachments are parsed (and their body made retrievable) without unbounded +// recursion into attachments-of-attachments. +func (p *EmailParser) parseEmail(ctx context.Context, filename string, data []byte, depth int) ParseResult { + ext := strings.ToLower(filepath.Ext(filename)) + + var content map[string]any + if ext == ".msg" { + var ( + msg map[string]any + err error + ) + // gomsg.Decode parses untrusted OLE2/CFB input; guard against a + // panic from a malformed .msg so one bad email can't take down the + // ingestion worker. + func() { + defer func() { + if r := recover(); r != nil { + // Log so a genuine bug (e.g. a nil deref in parseMSG) + // is not silently masked as a "decode panicked" error. + log.Printf("email: .msg decode panicked for %q; skipping: %v", filename, r) + err = fmt.Errorf("email: .msg decode panicked: %v", r) + } + }() + msg, err = parseMSG(data, p.fields) + }() + if err != nil { + return ParseResult{Err: fmt.Errorf("email: .msg: %w", err)} + } + content = msg + } else { + content = parseEML(bytes.NewReader(data), p.fields) + } outputFormat := p.outputFormat if outputFormat == "" { outputFormat = "text" } + // Re-chunk attachments so their content becomes retrievable within the + // same document (user-oriented; mirrors Python legacy rag/app/email.py + // naive_chunk). Each attachment is re-parsed by its file extension via + // the shared parser registry; a single unparseable/skipped attachment + // never breaks the whole email. + extraItems, attachmentText := p.rechunkEmailAttachments(ctx, content, depth) + + // attachments has been consumed by rechunkEmailAttachments (which + // re-parses each attachment by extension to make its content + // retrievable). It is otherwise dead weight: jsonItemsToPages copies + // every key into a schema.Page, but buildPagesFromBytes keeps only + // text+doc_type_kwd, so carrying the full attachment payloads through to + // the chunker would only bloat the intermediate pages before being + // discarded. Drop it from the result content. + delete(content, "attachments") + if outputFormat == "json" { - emailContent["doc_type_kwd"] = "text" + content["doc_type_kwd"] = "text" + items := []map[string]any{content} + items = append(items, extraItems...) return ParseResult{ OutputFormat: "json", File: map[string]any{"name": filename}, - JSON: []map[string]any{emailContent}, + JSON: items, } } // Text output: flatten fields into a single string. var sb strings.Builder - for k, v := range emailContent { + for k, v := range content { switch val := v.(type) { case string: sb.WriteString(k) @@ -119,19 +167,20 @@ func (p *EmailParser) ParseWithResult(ctx context.Context, filename string, data } } sb.WriteString("}\n") - case []map[string]any: - for _, att := range val { - fn, _ := att["filename"].(string) - pl, _ := att["payload"].(string) - sb.WriteString(fn) - sb.WriteString(":") - sb.WriteString(pl) - sb.WriteString("\n") - } case []string: sb.WriteString(strings.Join(val, "\n")) } } + // Attachment text (re-parsed by extension) replaces the old crude + // "filename:payload" flatten, so binary attachments no longer leak + // mojibake into the searchable (indexed) text. The raw attachment + // payloads themselves are dropped after rechunk (see parseEmail), so the + // JSON output path carries only the re-parsed attachment text, never the + // original payload bytes. + if attachmentText != "" { + sb.WriteString(attachmentText) + sb.WriteString("\n") + } return ParseResult{ OutputFormat: "text", File: map[string]any{"name": filename}, @@ -270,6 +319,12 @@ func readMailBody(body io.Reader, contentType string, collectAttachments bool) ( attachments = append(attachments, map[string]any{ "filename": attachmentFilename(part), "payload": decodeMailPayload(raw, partParams["charset"]), + // raw preserves the byte-exact decoded-CTE bytes so the + // re-chunk step re-parses the original attachment instead of + // the charset-decoded string (which can differ when the + // attachment declares a CJK charset). rechunkEmailAttachments + // prefers "raw" and falls back to "payload". + "raw": string(raw), }) continue } @@ -417,3 +472,324 @@ func decodeTransform(payload []byte, decoder *encoding.Decoder) (string, error) } return "", fmt.Errorf("decode produced replacement characters") } + +// parseMSG parses an Outlook .msg (OLE2 compound document) file using the +// gomsg library, mirroring the Python flow parser's _email() .msg branch +// (rag/flow/parser/parser.py), which parses via extract_msg. The output map +// shares the same field shape as parseEML so the downstream json/text +// assembly in ParseWithResult is reused unchanged. +func parseMSG(data []byte, fields []string) (map[string]any, error) { + target := targetFieldsSet(fields) + + msg, err := gomsg.Decode(bytes.NewReader(data)) + if err != nil { + return nil, err + } + + content := map[string]any{} + + if target["from"] { + content["from"] = formatSender(msg) + } + if target["to"] { + content["to"] = formatRecipient(msg.DisplayTo) + } + if target["cc"] { + content["cc"] = formatRecipient(msg.DisplayCC) + } + if target["bcc"] { + content["bcc"] = formatRecipient(msg.DisplayBCC) + } + if target["date"] { + content["date"] = formatMsgDate(msg.Date) + } + if target["subject"] { + content["subject"] = msg.Subject + } + + // Always emit metadata to match the Python flow parser contract, which + // unconditionally builds a {message_id, in_reply_to} metadata dict for + // .msg files regardless of whether "metadata" is in the configured fields. + // Empty values are emitted as nil (JSON null) to match extract_msg's None. + content["metadata"] = map[string]any{ + "message_id": orNil(msg.MessageID), + "in_reply_to": orNil(msg.InReplyTo), + } + + if target["body"] { + // Mirror Python: prefer the plain body, fall back to the HTML body + // when the plain body is empty. The .msg branch emits only "text" + // (never "text_html"), matching the Python _email .msg contract exactly. + text := msg.Body + if strings.TrimSpace(text) == "" && len(msg.BodyHTML) > 0 { + text = string(msg.BodyHTML) + } + content["text"] = text + } + + if target["attachments"] { + // Flatten attachments, recursing into embedded .msg files. gomsg + // parses an embedded Message via Attachment.EmbeddedMessage even + // though it exposes no raw bytes for it; the embedded message body + // is surfaced as a retrievable text attachment (see msgAttachments). + content["attachments"] = msgAttachments(msg) + } + + return content, nil +} + +// msgAttachments flattens a gomsg.Message's attachments into the +// {filename, payload} shape rechunkEmailAttachments consumes. Embedded .msg +// attachments expose no raw bytes via gomsg, but gomsg does parse the embedded +// Message; we surface its body as a retrievable text attachment (named .txt so +// the re-chunk step re-parses it as plain text) and recurse into its own +// attachments, so an embedded email is no longer silently dropped. +func msgAttachments(msg *gomsg.Message) []map[string]any { + out := make([]map[string]any, 0, len(msg.Attachments)) + for _, a := range msg.Attachments { + if a.IsEmbeddedMessage() { + if em := a.EmbeddedMessage(); em != nil { + body := em.Body + if strings.TrimSpace(body) == "" { + body = string(em.BodyHTML) + } + if body != "" { + out = append(out, map[string]any{ + "filename": a.DisplayName() + ".txt", + "payload": body, + }) + } + out = append(out, msgAttachments(em)...) + continue + } + } + out = append(out, map[string]any{ + "filename": a.DisplayName(), + "payload": string(a.Data()), + }) + } + return out +} + +// primarySenderEmail prefers the SMTP address, falling back to the raw email +// address when SMTP is unavailable (e.g. an EX address type). +func primarySenderEmail(msg *gomsg.Message) string { + if msg.SenderSMTP != "" { + return msg.SenderSMTP + } + return msg.SenderEmail +} + +// formatSender renders the .msg sender the way extract_msg's "sender" string +// is displayed: "Display Name " when a distinct display name is present, +// otherwise "" (matching extract_msg's angular-bracket form for an +// address with no display name). +func formatSender(msg *gomsg.Message) string { + email := primarySenderEmail(msg) + if msg.SenderName != "" && msg.SenderName != email { + return msg.SenderName + " <" + email + ">" + } + if email != "" { + return "<" + email + ">" + } + return msg.SenderName +} + +// formatRecipient renders a display recipient string the way extract_msg does: +// a bare single email address is wrapped in angle brackets, while a string +// that already contains an address form (display name, or multiple recipients) +// is returned unchanged. +func formatRecipient(display string) string { + if display == "" { + return "" + } + if strings.Contains(display, "<") { + return display + } + if !strings.ContainsAny(display, ",;") && strings.Contains(display, "@") { + return "<" + display + ">" + } + return display +} + +// orNil maps an empty string to nil so it serializes as JSON null, matching +// extract_msg's None for absent properties. +func orNil(s string) any { + if s == "" { + return nil + } + return s +} + +// formatMsgDate renders an Outlook .msg date the way extract_msg does: +// strftime("%Y-%m-%d %H:%M:%S%z"), which emits the zone without a colon +// (e.g. "2018-03-24 00:06:29+0800"). Go's -0700 layout reproduces that +// (the -07:00 layout would wrongly insert a colon). A zero time (date +// missing from the .msg) maps to nil so it serializes as JSON null, +// matching extract_msg's None, instead of a bogus sentinel such as +// "0001-01-01 00:00:00+0000". +func formatMsgDate(t time.Time) any { + if t.IsZero() { + return nil + } + return t.Format("2006-01-02 15:04:05-0700") +} + +// recoverParse runs fn, converting a panic from an untrusted attachment +// parser (e.g. a corrupt PDF/DOCX hitting a native CGO backend) into a zero +// ParseResult with panicked=true, so the caller can skip that one attachment +// instead of failing the whole email. This mirrors the recover around the +// .msg parseMSG call in parseEmail. +func recoverParse(fn func() ParseResult) (res ParseResult, panicked bool) { + defer func() { + if r := recover(); r != nil { + panicked = true + } + }() + return fn(), false +} + +// rechunkEmailAttachments re-parses each email attachment by its file +// extension and folds the extracted text back into the same document so +// attachment content becomes retrievable. This mirrors the user-oriented +// behaviour of Python's legacy rag/app/email.py, which re-chunks every +// attachment via naive_chunk and merges the resulting chunks into the same +// document. +// +// KNOWN LIMITATION: re-chunk uses the default-config parser returned by +// GetParser(ft) (and, for a nested email, only the top-level p.fields — no +// tenant/setup language, parse_method, or OCR/VLM model). The extracted +// attachment text may therefore differ from ingesting the same file as a +// standalone document through the pipeline's tenant/setup-configured +// parser. This is a deliberate, best-effort approximation for making +// attachment content retrievable (the same simplification Python's +// email.py naive_chunk makes), not a correctness bug; threading the full +// tenant/setup config into re-chunk is intentionally out of scope here. +// +// Attachments whose extension has no text-oriented parser (images, audio, +// video) are skipped: they carry no plain text in this pipeline and would +// require vision/speech models that are out of scope here. A single +// unparseable, empty, unsupported, or panicking attachment is skipped +// without failing the whole email (mirrors email.py's per-attachment +// try/except). +// +// The function returns both forms the caller needs: +// - extraItems: structured JSON items (one per non-empty text segment), +// each carrying only "text" and "doc_type_kwd", for the JSON output path. +// - text: the concatenated attachment text, for the text output path. +// +// Re-chunking stops at one level of nesting: a nested email is parsed (so its +// body becomes retrievable) but its own attachments are not re-chunked, to +// avoid unbounded recursion through attachments-of-attachments. +const maxRechunkPayloadBytes = 32 << 20 // 32 MiB + +func (p *EmailParser) rechunkEmailAttachments(ctx context.Context, content map[string]any, depth int) ([]map[string]any, string) { + if depth > 0 { + return nil, "" + } + // Honor task cancellation so a long attachment list does not keep + // re-parsing after the ingestion task has been stopped/aborted. + if ctx.Err() != nil { + return nil, "" + } + + raw, ok := content["attachments"].([]map[string]any) + if !ok || len(raw) == 0 { + return nil, "" + } + + var extra []map[string]any + var sb strings.Builder + for _, att := range raw { + fn, _ := att["filename"].(string) + payload, _ := att["payload"].(string) + // Prefer the byte-exact raw bytes (when present) so a re-parsed + // attachment is not silently corrupted by a prior charset decode. + if rawPayload, ok := att["raw"].(string); ok && rawPayload != "" { + payload = rawPayload + } + if fn == "" || payload == "" { + // Empty payload (e.g. an embedded .msg with no exposed bytes) or + // a missing filename — nothing to re-parse. + continue + } + if len(payload) > maxRechunkPayloadBytes { + // Too large to re-parse inline; re-chunking re-runs the heavy + // parsers (PDF/OCR/...), which would otherwise dominate or stall + // ingestion on a single huge attachment. The attachment remains + // referenced in metadata. + continue + } + ft := utility.GetFileType(fn) + switch ft { + case utility.FileTypeOTHER, utility.FileTypeVISUAL, + utility.FileTypeAURAL, utility.FileTypeVIDEO, utility.FileTypeFOLDER: + // No plain-text parser in this pipeline; skip rather than call + // a vision/speech model that is out of scope. + continue + } + var res ParseResult + var panicked bool + if ft == utility.FileTypeEMAIL { + // Reuse the top-level field configuration (including "body") so a + // nested .eml/.msg is parsed for its body instead of with a fresh, + // unconfigured parser that would only emit metadata and index + // garbage. We always request JSON output for the nested parse so + // the extracted "text" field is returned clean (the text output + // path would otherwise also flatten metadata into the result). + ep := NewEmailParser() + ep.ConfigureFromSetup(map[string]any{ + "fields": p.fields, + "output_format": "json", + }) + res, panicked = recoverParse(func() ParseResult { + return ep.parseEmail(ctx, fn, []byte(payload), depth+1) + }) + } else { + np, err := GetParser(ft) + if err != nil { + continue + } + res, panicked = recoverParse(func() ParseResult { + return np.ParseWithResult(ctx, fn, []byte(payload)) + }) + } + if panicked { + // An untrusted attachment parser (e.g. a corrupt PDF/DOCX hitting + // a native CGO backend) panicked. Skip just this attachment so one + // bad file can't fail the whole email — mirrors the .msg + // parseMSG recover. + log.Printf("email: attachment %q re-parse panicked; skipping", fn) + } + if panicked || res.Err != nil { + continue + } + + var texts []string + if res.OutputFormat == "json" && len(res.JSON) > 0 { + for _, it := range res.JSON { + if t, ok := it["text"].(string); ok { + if t = strings.TrimSpace(t); t != "" { + texts = append(texts, t) + } + } + } + } else if res.Text != "" { + if t := strings.TrimSpace(res.Text); t != "" { + texts = append(texts, t) + } + } + + for _, t := range texts { + extra = append(extra, map[string]any{ + "text": t, + "doc_type_kwd": "text", + }) + if sb.Len() > 0 { + sb.WriteString("\n") + } + sb.WriteString(t) + } + } + return extra, sb.String() +} diff --git a/internal/parser/parser/email_parser_test.go b/internal/parser/parser/email_parser_test.go index aeeb83095c..dd9436aa8a 100644 --- a/internal/parser/parser/email_parser_test.go +++ b/internal/parser/parser/email_parser_test.go @@ -17,9 +17,15 @@ package parser import ( + "bytes" + "context" "encoding/base64" + "mime/multipart" + "net/textproto" + "os" "strings" "testing" + "time" ) func TestEmailParser_EmlJSON(t *testing.T) { @@ -110,15 +116,122 @@ func TestEmailParser_EmlText(t *testing.T) { } } -func TestEmailParser_MsgNotSupported(t *testing.T) { +// TestEmailParser_MsgSupported parses a real Outlook .msg fixture and verifies +// the Go output aligns with the Python flow parser _email() .msg branch +// (rag/flow/parser/parser.py). Replaces the old "MsgNotSupported" test now that +// .msg is supported via gomsg. +func TestEmailParser_MsgSupported(t *testing.T) { ctx := t.Context() - p := NewEmailParser() - result := p.ParseWithResult(ctx, "test.msg", []byte{}) - if result.Err == nil { - t.Fatal("expected error for .msg file") + data, err := os.ReadFile("testdata/sample.msg") + if err != nil { + t.Fatalf("read fixture: %v", err) } - if !strings.Contains(result.Err.Error(), ".msg") { - t.Errorf("error should mention .msg: %v", result.Err) + + p := NewEmailParser() + p.ConfigureFromSetup(map[string]any{ + "output_format": "json", + "fields": []string{"from", "to", "cc", "bcc", "date", "subject", "body", "attachments", "metadata"}, + }) + + result := p.ParseWithResult(ctx, "sample.msg", data) + if result.Err != nil { + t.Fatalf("unexpected error: %v", result.Err) + } + if len(result.JSON) != 1 { + t.Fatalf("expected 1 JSON item, got %d", len(result.JSON)) + } + item := result.JSON[0] + + if v, ok := item["from"].(string); !ok || v != "" { + t.Errorf("from: got %q", v) + } + if v, ok := item["to"].(string); !ok || v != "" { + t.Errorf("to: got %q", v) + } + if v, ok := item["subject"].(string); !ok || v != "asdf" { + t.Errorf("subject: got %q", v) + } + if v, ok := item["date"].(string); !ok || v != "2018-03-24 00:06:29+0800" { + t.Errorf("date: got %q, want 2018-03-24 00:06:29+0800", v) + } + if v, ok := item["text"].(string); !ok || v != " \r\n\r\n" { + t.Errorf("text: got %q", v) + } + // The .msg branch must NOT emit text_html (matches Python _email .msg branch). + if _, ok := item["text_html"]; ok { + t.Error("text_html must be absent for .msg") + } + meta, ok := item["metadata"].(map[string]any) + if !ok { + t.Fatalf("metadata missing or wrong type: %T", item["metadata"]) + } + if v, ok := meta["message_id"].(string); !ok || v == "" { + t.Errorf("metadata message_id: got %q", v) + } + // Empty in_reply_to mirrors extract_msg's None -> JSON null. + if v, ok := meta["in_reply_to"]; ok && v != nil { + t.Errorf("metadata in_reply_to: got %v, want nil", v) + } + if _, ok := meta["in_reply_to"]; !ok { + t.Error("metadata in_reply_to key must be present") + } + // attachments are extracted by the .msg branch but deliberately dropped + // from the final ParseResult (consumed by rechunkEmailAttachments; + // buildPagesFromBytes keeps only text+doc_type_kwd). Verify the + // high-level result no longer carries the heavy payload... + if _, ok := item["attachments"]; ok { + t.Error("attachments must be dropped from the final ParseResult") + } + // ...and verify the .msg branch still extracts them at the parse level. + msgContent, err := parseMSG(data, []string{"from", "to", "cc", "bcc", "date", "subject", "body", "attachments", "metadata"}) + if err != nil { + t.Fatalf("parseMSG: %v", err) + } + atts, ok := msgContent["attachments"].([]map[string]any) + if !ok { + t.Fatalf("parseMSG attachments missing or wrong type: %T", msgContent["attachments"]) + } + if len(atts) != 1 { + t.Fatalf("expected 1 attachment, got %d", len(atts)) + } + if fn, _ := atts[0]["filename"].(string); fn != "5AAoPFgV-nJ965R7o-98C38840-4454-4750-9AEF-F53DB3E37548.jpg" { + t.Errorf("filename = %q", fn) + } + if pl, _ := atts[0]["payload"].(string); len(pl) != 122784 { + t.Errorf("payload length = %d, want 122784", len(pl)) + } +} + +// TestEmailParser_MsgMetadataAlwaysPresent verifies the .msg branch emits +// metadata unconditionally (matching the Python contract) even when "metadata" +// is omitted from the configured fields. +func TestEmailParser_MsgMetadataAlwaysPresent(t *testing.T) { + ctx := t.Context() + data, err := os.ReadFile("testdata/sample.msg") + if err != nil { + t.Fatalf("read fixture: %v", err) + } + + p := NewEmailParser() + p.ConfigureFromSetup(map[string]any{ + "output_format": "json", + "fields": []string{"from", "subject"}, // "metadata" intentionally absent + }) + + result := p.ParseWithResult(ctx, "sample.msg", data) + if result.Err != nil { + t.Fatalf("unexpected error: %v", result.Err) + } + item := result.JSON[0] + + if _, ok := item["metadata"].(map[string]any); !ok { + t.Fatalf("metadata must always be present for .msg, got %T", item["metadata"]) + } + // Basic fields not in fields are dropped. + for _, dropped := range []string{"to", "date", "body", "attachments"} { + if _, ok := item[dropped]; ok { + t.Errorf("%s should be dropped when not in fields", dropped) + } } } @@ -163,9 +276,16 @@ func TestEmailParser_Base64Attachment(t *testing.T) { } item := result.JSON[0] - atts, ok := item["attachments"].([]map[string]any) + // attachments are dropped from the final ParseResult (consumed by + // rechunk, then deleted); verify the high-level result is clean... + if _, ok := item["attachments"]; ok { + t.Error("attachments must be dropped from the final ParseResult") + } + // ...and verify the .eml branch still decodes the base64 attachment. + eml := parseEML(bytes.NewReader([]byte(raw)), []string{"from", "body", "attachments"}) + atts, ok := eml["attachments"].([]map[string]any) if !ok { - t.Fatalf("attachments missing or wrong type: %T", item["attachments"]) + t.Fatalf("attachments missing or wrong type: %T", eml["attachments"]) } if len(atts) != 1 { t.Fatalf("expected 1 attachment, got %d", len(atts)) @@ -242,8 +362,14 @@ func TestEmailParser_Base64AttachmentInMixedMultipart(t *testing.T) { t.Errorf("text_html: got %q, want to contain 'HTML body'", v) } - // Verify attachment is decoded from base64 - atts, ok := item["attachments"].([]map[string]any) + // attachments are dropped from the final ParseResult; verify the + // high-level result is clean... + if _, ok := item["attachments"]; ok { + t.Error("attachments must be dropped from the final ParseResult") + } + // ...and verify the .eml branch still decodes the base64 attachment. + eml := parseEML(bytes.NewReader([]byte(raw)), []string{"from", "body", "attachments"}) + atts, ok := eml["attachments"].([]map[string]any) if !ok || len(atts) != 1 { t.Fatalf("expected 1 attachment, got %d", len(atts)) } @@ -404,6 +530,219 @@ func TestEmailParser_TextHTMLAlwaysPresent(t *testing.T) { // empty list because attachment extraction was coupled to the body branch // (the "else if needAttachments" fallback set an empty slice instead of // walking the message). +// TestEmailParser_AttachmentSearchableJSON verifies the user-oriented +// behaviour: an email attachment is re-parsed by its file extension and its +// content becomes a retrievable chunk in the SAME document (mirrors Python +// legacy rag/app/email.py naive_chunk). The attachment text must appear as a +// separate JSON item, while the email body stays on the main item. +func TestEmailParser_AttachmentSearchableJSON(t *testing.T) { + ctx := t.Context() + attachmentContent := "QUOTE: the quick brown fox jumps over the lazy dog." + encoded := base64.StdEncoding.EncodeToString([]byte(attachmentContent)) + boundary := "attachboundary" + raw := strings.Join([]string{ + "From: sender@test.com", + "To: receiver@test.com", + "Subject: Attachment Test", + "Content-Type: multipart/mixed; boundary=" + boundary, + "", + "--" + boundary, + "Content-Type: text/plain; charset=utf-8", + "", + "Email body text.", + "--" + boundary, + "Content-Type: text/plain; charset=utf-8", + "Content-Disposition: attachment; filename=\"note.txt\"", + "Content-Transfer-Encoding: base64", + "", + encoded, + "--" + boundary + "--", + }, "\r\n") + + p := NewEmailParser() + p.ConfigureFromSetup(map[string]any{ + "output_format": "json", + "fields": []string{"from", "body", "attachments"}, + }) + + result := p.ParseWithResult(ctx, "test.eml", []byte(raw)) + if result.Err != nil { + t.Fatalf("unexpected error: %v", result.Err) + } + + // Body must remain on the main item. + if v, ok := result.JSON[0]["text"].(string); !ok || !strings.Contains(v, "Email body text") { + t.Errorf("body missing on main item: %v", result.JSON[0]["text"]) + } + + // Attachment text must appear as a separate retrievable JSON item. + found := false + for _, it := range result.JSON { + if txt, ok := it["text"].(string); ok && strings.Contains(txt, "quick brown fox") { + found = true + } + } + if !found { + t.Errorf("attachment text not found in JSON output:\n%#v", result.JSON) + } +} + +// TestEmailParser_AttachmentSearchableText is the text-output equivalent: +// the re-parsed attachment text must be present in result.Text. +func TestEmailParser_AttachmentSearchableText(t *testing.T) { + ctx := t.Context() + attachmentContent := "QUOTE: the quick brown fox jumps over the lazy dog." + encoded := base64.StdEncoding.EncodeToString([]byte(attachmentContent)) + boundary := "attachboundary" + raw := strings.Join([]string{ + "From: sender@test.com", + "To: receiver@test.com", + "Subject: Attachment Test", + "Content-Type: multipart/mixed; boundary=" + boundary, + "", + "--" + boundary, + "Content-Type: text/plain; charset=utf-8", + "", + "Email body text.", + "--" + boundary, + "Content-Type: text/plain; charset=utf-8", + "Content-Disposition: attachment; filename=\"note.txt\"", + "Content-Transfer-Encoding: base64", + "", + encoded, + "--" + boundary + "--", + }, "\r\n") + + p := NewEmailParser() + p.ConfigureFromSetup(map[string]any{ + "output_format": "text", + "fields": []string{"from", "body", "attachments"}, + }) + + result := p.ParseWithResult(ctx, "test.eml", []byte(raw)) + if result.Err != nil { + t.Fatalf("unexpected error: %v", result.Err) + } + if !strings.Contains(result.Text, "quick brown fox") { + t.Errorf("attachment text missing from text output: %q", result.Text) + } + if !strings.Contains(result.Text, "Email body text") { + t.Errorf("email body missing from text output: %q", result.Text) + } +} + +// TestEmailParser_AttachmentEmptyPayloadSkipped verifies error isolation: +// an attachment with an empty payload (the legacy .msg nested-attachment +// case, where gomsg exposes no raw bytes) is skipped without breaking the +// email, and a valid sibling attachment is still re-chunked. +func TestEmailParser_AttachmentEmptyPayloadSkipped(t *testing.T) { + ctx := t.Context() + validContent := "VALID attachment payload" + validEncoded := base64.StdEncoding.EncodeToString([]byte(validContent)) + boundary := "mixedbound" + raw := strings.Join([]string{ + "From: sender@test.com", + "To: receiver@test.com", + "Subject: Empty Payload", + "Content-Type: multipart/mixed; boundary=" + boundary, + "", + "--" + boundary, + "Content-Type: text/plain; charset=utf-8", + "", + "Hello body.", + "--" + boundary, + "Content-Type: application/octet-stream; name=\"empty.bin\"", + "Content-Disposition: attachment; filename=\"empty.bin\"", + "Content-Transfer-Encoding: base64", + "", + "", + "--" + boundary, + "Content-Type: text/plain; charset=utf-8", + "Content-Disposition: attachment; filename=\"valid.txt\"", + "Content-Transfer-Encoding: base64", + "", + validEncoded, + "--" + boundary + "--", + }, "\r\n") + + p := NewEmailParser() + p.ConfigureFromSetup(map[string]any{ + "output_format": "json", + "fields": []string{"from", "body", "attachments"}, + }) + + result := p.ParseWithResult(ctx, "test.eml", []byte(raw)) + if result.Err != nil { + t.Fatalf("email must not fail on empty-payload attachment: %v", result.Err) + } + + found := false + for _, it := range result.JSON { + if txt, ok := it["text"].(string); ok && strings.Contains(txt, "VALID attachment payload") { + found = true + } + } + if !found { + t.Errorf("valid attachment text not found despite empty-payload sibling: %#v", result.JSON) + } +} + +// TestEmailParser_AttachmentUnknownExtSkipped verifies that an attachment +// with an unrecognized extension is skipped (no parser available) while a +// valid sibling attachment is still re-chunked. Mirrors the legacy email.py +// behaviour, which only re-chunks known file types. +func TestEmailParser_AttachmentUnknownExtSkipped(t *testing.T) { + ctx := t.Context() + validContent := "KNOWN attachment payload" + validEncoded := base64.StdEncoding.EncodeToString([]byte(validContent)) + boundary := "mixedbound" + raw := strings.Join([]string{ + "From: sender@test.com", + "To: receiver@test.com", + "Subject: Unknown Ext", + "Content-Type: multipart/mixed; boundary=" + boundary, + "", + "--" + boundary, + "Content-Type: text/plain; charset=utf-8", + "", + "Hello body.", + "--" + boundary, + "Content-Type: application/octet-stream; name=\"data.xyz\"", + "Content-Disposition: attachment; filename=\"data.xyz\"", + "Content-Transfer-Encoding: base64", + "", + base64.StdEncoding.EncodeToString([]byte("opaque bytes")), + "--" + boundary, + "Content-Type: text/plain; charset=utf-8", + "Content-Disposition: attachment; filename=\"known.txt\"", + "Content-Transfer-Encoding: base64", + "", + validEncoded, + "--" + boundary + "--", + }, "\r\n") + + p := NewEmailParser() + p.ConfigureFromSetup(map[string]any{ + "output_format": "json", + "fields": []string{"from", "body", "attachments"}, + }) + + result := p.ParseWithResult(ctx, "test.eml", []byte(raw)) + if result.Err != nil { + t.Fatalf("email must not fail on unknown-extension attachment: %v", result.Err) + } + + found := false + for _, it := range result.JSON { + if txt, ok := it["text"].(string); ok && strings.Contains(txt, "KNOWN attachment payload") { + found = true + } + } + if !found { + t.Errorf("known attachment text not found despite unknown-ext sibling: %#v", result.JSON) + } +} + func TestEmailParser_AttachmentsWithoutBody(t *testing.T) { ctx := t.Context() attachmentContent := "SECRET attachment payload" @@ -449,10 +788,16 @@ func TestEmailParser_AttachmentsWithoutBody(t *testing.T) { t.Error("text_html should be absent when body not in fields") } - // attachments must be extracted even without body. - atts, ok := item["attachments"].([]map[string]any) + // attachments are dropped from the final ParseResult (consumed by + // rechunk, then deleted); verify the high-level result is clean... + if _, ok := item["attachments"]; ok { + t.Error("attachments must be dropped from the final ParseResult") + } + // ...and verify the .eml branch still extracts them even without body. + eml := parseEML(bytes.NewReader([]byte(raw)), []string{"from", "attachments"}) + atts, ok := eml["attachments"].([]map[string]any) if !ok { - t.Fatalf("attachments missing or wrong type: %T", item["attachments"]) + t.Fatalf("attachments missing or wrong type: %T", eml["attachments"]) } if len(atts) != 1 { t.Fatalf("expected 1 attachment without body, got %d (bug: attachments silently dropped)", len(atts)) @@ -464,3 +809,258 @@ func TestEmailParser_AttachmentsWithoutBody(t *testing.T) { t.Errorf("payload = %q, want %q", pl, attachmentContent) } } + +// TestEmailParser_NestedEMLAttachmentRechunk locks the regression where a +// nested .eml attachment was re-chunked with an UNCONFIGURED EmailParser +// (fields == nil), so parseEML emitted only metadata and the text path indexed +// "metadata:{...}" garbage. The nested email must instead be parsed with the +// top-level field configuration (including "body") so its body becomes the +// indexed text. The outer email's own metadata flattening is expected; the +// nested email must NOT contribute a second "metadata:{" segment. +func TestEmailParser_NestedEMLAttachmentRechunk(t *testing.T) { + ctx := t.Context() + + innerRaw := strings.Join([]string{ + "From: inner@x.com", + "To: outer@y.com", + "Subject: inner", + "Content-Type: text/plain; charset=utf-8", + "", + "INNER BODY SECRET", + }, "\r\n") + + boundary := "outerbound" + raw := strings.Join([]string{ + "From: outer@y.com", + "To: someone@z.com", + "Subject: outer", + "MIME-Version: 1.0", + "Content-Type: multipart/mixed; boundary=" + boundary, + "", + "--" + boundary, + "Content-Type: text/plain; charset=utf-8", + "", + "OUTER BODY VISIBLE", + "--" + boundary, + "Content-Type: message/rfc822", + "Content-Disposition: attachment; filename=\"inner.eml\"", + "", + innerRaw, + "--" + boundary + "--", + }, "\r\n") + + for _, format := range []string{"json", "text"} { + p := NewEmailParser() + p.ConfigureFromSetup(map[string]any{ + "output_format": format, + "fields": []string{"from", "to", "subject", "body", "attachments", "metadata"}, + }) + + result := p.ParseWithResult(ctx, "outer.eml", []byte(raw)) + if result.Err != nil { + t.Fatalf("[%s] unexpected error: %v", format, result.Err) + } + + // Collect all indexed text for this output mode (JSON populates + // result.JSON; text populates result.Text). + var indexed strings.Builder + if format == "json" { + for _, it := range result.JSON { + if txt, ok := it["text"].(string); ok { + indexed.WriteString(txt) + indexed.WriteString("\n") + } + } + } else { + indexed.WriteString(result.Text) + } + + // The nested email body must be retrievable. + if !strings.Contains(indexed.String(), "INNER BODY SECRET") { + t.Errorf("[%s] nested body not indexed: %q", format, indexed.String()) + } + if !strings.Contains(indexed.String(), "OUTER BODY VISIBLE") { + t.Errorf("[%s] outer body missing: %q", format, indexed.String()) + } + + if format == "json" { + // The nested email must be a distinct, clean item, carrying no + // metadata key (the old bug leaked a "metadata:{...}" string as + // its text). + found := false + for _, it := range result.JSON { + if txt, ok := it["text"].(string); ok && strings.Contains(txt, "INNER BODY SECRET") { + found = true + if _, hasMeta := it["metadata"]; hasMeta { + t.Errorf("[json] nested item must not carry metadata key: %#v", it) + } + } + } + if !found { + t.Errorf("[json] nested body not found as separate item: %#v", result.JSON) + } + } else { + // Text mode: the outer email's own metadata is flattened once; + // the nested email must NOT add a second "metadata:{" segment. + if c := strings.Count(result.Text, "metadata:{"); c != 1 { + t.Errorf("[text] expected exactly 1 metadata:{ segment (outer only), got %d in: %q", + c, result.Text) + } + } + } +} + +// TestEmailParser_RechunkAttachmentTextPath exercises rechunkEmailAttachments +// directly with the {filename, payload} attachment shape that both .eml and +// .msg parsing feed into it. It verifies text attachments are indexed while +// binary (VISUAL) attachments are skipped, and that binary payloads never leak +// into the indexed text. +func TestEmailParser_RechunkAttachmentTextPath(t *testing.T) { + ctx := t.Context() + content := map[string]any{ + "attachments": []map[string]any{ + {"filename": "note.txt", "payload": "hello from attachment"}, + {"filename": "pic.jpg", "payload": ""}, + }, + } + + p := NewEmailParser() + extra, text := p.rechunkEmailAttachments(ctx, content, 0) + + if len(extra) != 1 { + t.Fatalf("expected 1 indexed attachment, got %d: %#v", len(extra), extra) + } + if v, _ := extra[0]["text"].(string); v != "hello from attachment" { + t.Errorf("text = %q, want hello from attachment", v) + } + if !strings.Contains(text, "hello from attachment") { + t.Errorf("indexed text missing attachment: %q", text) + } + if strings.Contains(text, "") { + t.Errorf("binary payload leaked into indexed text: %q", text) + } +} + +// TestFormatMsgDate verifies the .msg date rendering: a zero time (date +// missing from the .msg) maps to nil (JSON null, matching extract_msg's +// None) instead of a bogus sentinel like "0001-01-01 00:00:00+0000". +func TestFormatMsgDate(t *testing.T) { + if got := formatMsgDate(time.Time{}); got != nil { + t.Errorf("zero date should map to nil, got %#v", got) + } + got := formatMsgDate(time.Date(2018, 3, 24, 0, 6, 29, 0, time.FixedZone("CST", 8*3600))) + if got != "2018-03-24 00:06:29+0800" { + t.Errorf("formatted date = %q, want 2018-03-24 00:06:29+0800", got) + } +} + +// TestRechunkEmailAttachments_ContextCancelled verifies that an already +// cancelled context short-circuits re-chunking instead of re-parsing +// attachments (mirrors the cancellation check CodeRabbit flagged). +func TestRechunkEmailAttachments_ContextCancelled(t *testing.T) { + ctx, cancel := context.WithCancel(t.Context()) + cancel() // task already aborted + content := map[string]any{ + "attachments": []map[string]any{ + {"filename": "note.txt", "payload": "must not be re-chunked"}, + }, + } + extra, text := NewEmailParser().rechunkEmailAttachments(ctx, content, 0) + if len(extra) != 0 || text != "" { + t.Errorf("cancelled context should short-circuit re-chunk: extra=%#v text=%q", extra, text) + } +} + +// TestRecoverParse_IsolatesPanic verifies that a panic from an untrusted +// attachment parser is converted into a zero ParseResult with panicked=true +// and does NOT propagate out of recoverParse (so a single bad attachment can +// be skipped instead of failing the whole email). +func TestRecoverParse_IsolatesPanic(t *testing.T) { + res, panicked := recoverParse(func() ParseResult { + panic("boom") + }) + if !panicked { + t.Error("expected panicked=true") + } + if res.Err != nil || res.JSON != nil || res.Text != "" { + t.Errorf("panic should yield a zero ParseResult, got %#v", res) + } +} + +// TestRecoverParse_PassesThrough verifies a normal parser result is returned +// unchanged with panicked=false. +func TestRecoverParse_PassesThrough(t *testing.T) { + want := ParseResult{OutputFormat: "text", Text: "hello"} + res, panicked := recoverParse(func() ParseResult { + return want + }) + if panicked { + t.Error("expected panicked=false for a normal result") + } + if res.Text != "hello" { + t.Errorf("result not passed through: %#v", res) + } +} + +// TestEmailParser_RechunkPrefersRaw verifies that rechunkEmailAttachments +// re-parses the byte-exact "raw" bytes when present, rather than the +// charset-decoded "payload". The fallback (no "raw") still uses "payload". +func TestEmailParser_RechunkPrefersRaw(t *testing.T) { + ctx := t.Context() + p := NewEmailParser() + + withRaw, textRaw := p.rechunkEmailAttachments(ctx, map[string]any{ + "attachments": []map[string]any{ + {"filename": "note.txt", "payload": "WORLD", "raw": "HELLO"}, + }, + }, 0) + if len(withRaw) != 1 || withRaw[0]["text"] != "HELLO" { + t.Fatalf("raw not preferred: extra=%#v text=%q", withRaw, textRaw) + } + if strings.Contains(textRaw, "WORLD") { + t.Errorf("re-chunk used payload instead of raw: %q", textRaw) + } + + fallback, textFallback := p.rechunkEmailAttachments(ctx, map[string]any{ + "attachments": []map[string]any{ + {"filename": "note.txt", "payload": "WORLD"}, + }, + }, 0) + if len(fallback) != 1 || fallback[0]["text"] != "WORLD" { + t.Fatalf("payload fallback broken: extra=%#v text=%q", fallback, textFallback) + } +} + +// TestReadMailBody_AttachmentPreservesRaw verifies that .eml attachment +// collection stores the byte-exact decoded-CTE bytes under "raw" alongside the +// charset-decoded "payload". Declaring charset=gbk makes decodeMailPayload +// decode GBK bytes to a UTF-8 string, so the two differ — re-chunk relies on +// "raw" to avoid silently corrupting the attachment. +func TestReadMailBody_AttachmentPreservesRaw(t *testing.T) { + // GBK-encoded "中文" (中=0xD6D0, 文=0xCEC4). + gbk := []byte{0xD6, 0xD0, 0xCE, 0xC4} + + var buf bytes.Buffer + mw := multipart.NewWriter(&buf) + bp, _ := mw.CreatePart(textproto.MIMEHeader{"Content-Type": {"text/plain"}}) + bp.Write([]byte("body")) + ap, _ := mw.CreatePart(textproto.MIMEHeader{ + "Content-Type": {"application/octet-stream; charset=gbk"}, + "Content-Disposition": {`attachment; filename="x.bin"`}, + "Content-Transfer-Encoding": {"base64"}, + }) + ap.Write([]byte(base64.StdEncoding.EncodeToString(gbk))) + mw.Close() + + _, _, attachments := readMailBody(strings.NewReader(buf.String()), "multipart/mixed; boundary="+mw.Boundary(), true) + if len(attachments) != 1 { + t.Fatalf("expected 1 attachment, got %d: %#v", len(attachments), attachments) + } + att := attachments[0] + if raw, _ := att["raw"].(string); raw != string(gbk) { + t.Errorf("raw = %q, want byte-exact %q", raw, string(gbk)) + } + if payload, _ := att["payload"].(string); payload != "中文" { + t.Errorf("payload = %q, want 中文 (charset-decoded)", payload) + } +} diff --git a/internal/parser/parser/testdata/sample.msg b/internal/parser/parser/testdata/sample.msg new file mode 100644 index 0000000000..00e8d0ce28 Binary files /dev/null and b/internal/parser/parser/testdata/sample.msg differ diff --git a/test/test_check_files.py b/test/test_check_files.py new file mode 100644 index 0000000000..ca5acaa712 --- /dev/null +++ b/test/test_check_files.py @@ -0,0 +1,92 @@ +# Copyright 2026 The InfiniFlow Authors. All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Tests for tools/hooks/check_files.py. + +These lock the strict UTF-8 decode behaviour introduced when the hook stopped +using errors="ignore": a file whose bytes are not valid UTF-8 (and which does +not contain a NUL byte, so the binary guard misses it) must be skipped rather +than silently rewritten with dropped bytes when running in --fix mode. +""" + +import sys +from pathlib import Path + +# pytest's pythonpath includes "." but be defensive about import location. +_ROOT = Path(__file__).resolve().parents[1] +if str(_ROOT) not in sys.path: + sys.path.insert(0, str(_ROOT)) + +from tools.hooks.check_files import ( + check_merge_conflicts, + check_trailing_whitespace, +) + + +def test_trailing_whitespace_fix_skips_invalid_utf8_without_corruption(tmp_path): + # Invalid UTF-8 (no NUL byte) carrying trailing whitespace. + f = tmp_path / "blob.bin" + original = b"data \xff\xfe trailing \n" + f.write_bytes(original) + + rc = check_trailing_whitespace([f], fix=True) + + # Skipped (no error reported) and, crucially, the bytes are untouched: + # the old errors="ignore" path would have dropped the invalid bytes and + # rewritten the file, corrupting it. + assert rc == 0 + assert f.read_bytes() == original + + +def test_trailing_whitespace_fix_strips_valid_utf8(tmp_path): + f = tmp_path / "ok.txt" + f.write_bytes(b"hello \n") + + rc = check_trailing_whitespace([f], fix=True) + + assert rc == 0 + assert f.read_bytes() == b"hello\n" + + +def test_trailing_whitespace_fix_is_noop_on_clean_file(tmp_path): + f = tmp_path / "ok.txt" + f.write_bytes(b"clean\n") + + check_trailing_whitespace([f], fix=True) + + assert f.read_bytes() == b"clean\n" + + +def test_merge_conflicts_skips_binary_file_with_markers(tmp_path): + # A binary file whose bytes happen to contain ASCII conflict markers. + f = tmp_path / "blob.bin" + f.write_bytes(b"\x00<<<<<<< HEAD\n>>>>>>> branch\n") + + rc = check_merge_conflicts([f]) + + # Skipped (binary guard), so no false-positive conflict report. + assert rc == 0 + + +def test_merge_conflicts_detects_real_conflict(tmp_path): + f = tmp_path / "conflicted.txt" + f.write_text( + "<<<<<<< HEAD\nlocal\n=======\nincoming\n>>>>>>> branch\n", + encoding="utf-8", + ) + + rc = check_merge_conflicts([f]) + + # Real conflict markers are still detected. + assert rc == 1 diff --git a/tools/hooks/check_files.py b/tools/hooks/check_files.py old mode 100644 new mode 100755 index ac85253c35..7427c00483 --- a/tools/hooks/check_files.py +++ b/tools/hooks/check_files.py @@ -12,7 +12,6 @@ from pathlib import Path import yaml - MERGE_PATTERNS = ("<<<<<<< ", "=======\n", ">>>>>>> ") # Printable ASCII (0x20-0x7E) plus newline — matches the regex used by the @@ -24,6 +23,15 @@ def _read_bytes(path: Path) -> bytes: return path.read_bytes() +# Binary fixtures (OLE2/.msg, PDF, images, …) contain NUL bytes. The +# text-fixing hooks below must never rewrite them: line-ending or +# trailing-newline normalization corrupts binary files byte-for-byte +# (e.g. a .msg fixture was previously mangled by the mixed-line-ending +# and end-of-file fixers). Skip any file that contains a NUL byte. +def _is_binary(data: bytes) -> bool: + return b"\x00" in data + + def _git_paths(*args: str) -> list[Path]: proc = subprocess.run( ["git", *args, "-z"], @@ -52,7 +60,7 @@ def check_json(paths: list[Path], fix: bool = False) -> int: continue try: json.loads(path.read_text(encoding="utf-8")) - except Exception as exc: + except (json.JSONDecodeError, OSError, UnicodeDecodeError) as exc: errors.append(f"invalid json: {path}: {exc}") return _report(errors) @@ -64,7 +72,7 @@ def check_yaml(paths: list[Path], fix: bool = False) -> int: continue try: yaml.safe_load(path.read_text(encoding="utf-8")) - except Exception as exc: + except (yaml.YAMLError, OSError, UnicodeDecodeError) as exc: errors.append(f"invalid yaml: {path}: {exc}") return _report(errors) @@ -75,6 +83,8 @@ def check_eof(paths: list[Path], fix: bool = False) -> int: if not path.is_file(): continue data = _read_bytes(path) + if _is_binary(data): + continue if data and not data.endswith(b"\n"): if fix: with path.open("ab") as f: @@ -94,8 +104,17 @@ def check_trailing_whitespace(paths: list[Path], fix: bool = False) -> int: if not path.is_file(): continue try: - text = path.read_text(encoding="utf-8", errors="ignore") - except Exception: + data = _read_bytes(path) + except OSError: + continue + if _is_binary(data): + continue + try: + text = data.decode("utf-8") + except UnicodeDecodeError: + # Not valid UTF-8 (and not NUL-binary): skip rather than silently + # dropping bytes with errors="ignore", which would corrupt the + # file when run with fix=True. continue if not text: continue @@ -120,6 +139,8 @@ def check_mixed_line_endings(paths: list[Path], fix: bool = False) -> int: if not path.is_file(): continue data = _read_bytes(path) + if _is_binary(data): + continue has_crlf = b"\r\n" in data has_lf = b"\n" in data.replace(b"\r\n", b"") if has_crlf and has_lf: @@ -136,7 +157,18 @@ def check_merge_conflicts(paths: list[Path], fix: bool = False) -> int: for path in paths: if not path.is_file(): continue - text = path.read_text(encoding="utf-8", errors="ignore") + try: + data = _read_bytes(path) + except OSError: + continue + if _is_binary(data): + continue + try: + text = data.decode("utf-8") + except UnicodeDecodeError: + # Not valid UTF-8: skip rather than risking a false positive from + # bytes mangled by errors="ignore". + continue if all(pattern in text for pattern in MERGE_PATTERNS): errors.append(f"merge conflict markers: {path}") return _report(errors)