diff --git a/go.mod b/go.mod index 8d4c45cff6..2fdf58b5ca 100644 --- a/go.mod +++ b/go.mod @@ -62,6 +62,7 @@ require ( github.com/pkg/errors v0.9.1 github.com/pkoukk/tiktoken-go v0.1.8 github.com/redis/go-redis/v9 v9.18.0 + github.com/richardlehane/mscfb v1.0.8 github.com/saintfish/chardet v0.0.0-20230101081208-5e3ef4b5456d github.com/shirou/gopsutil/v3 v3.24.5 github.com/signintech/gopdf v0.36.1 @@ -208,7 +209,6 @@ require ( github.com/quic-go/qpack v0.6.0 // indirect github.com/quic-go/quic-go v0.59.1 // indirect github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect - github.com/richardlehane/mscfb v1.0.8 // indirect github.com/richardlehane/msoleps v1.0.6 // indirect github.com/rs/xid v1.6.0 // indirect github.com/sagikazarmark/locafero v0.4.0 // indirect diff --git a/internal/cli/user_command.go b/internal/cli/user_command.go index 6f27be912e..c3b1e3097a 100644 --- a/internal/cli/user_command.go +++ b/internal/cli/user_command.go @@ -3034,15 +3034,17 @@ func (c *CLI) APIParseLocalFileCommand(commandCount int, cmd *Command) (Response } fileType := utility.GetFileType(filename) - fileParser, err := parser.GetTextOnlyParser(fileType) - if err != nil { - return nil, err - } - fileContent, err := os.ReadFile(filename) if err != nil { return nil, fmt.Errorf("failed to read dsl file: %w", err) } + fileParser, actual, err := parser.GetTextOnlyParser(fileType, fileContent) + if err != nil { + return nil, err + } + if actual != fileType { + fmt.Fprintf(os.Stderr, "WARNING: file %q: requested format %s; detected %s; continuing with the %s parser\n", filepath.Base(filename), fileType, actual, actual) + } ctx := context.Background() parseResult := fileParser.ParseWithResult(ctx, filename, fileContent) diff --git a/internal/cli/user_command_office_test.go b/internal/cli/user_command_office_test.go new file mode 100644 index 0000000000..0f15732cf9 --- /dev/null +++ b/internal/cli/user_command_office_test.go @@ -0,0 +1,49 @@ +//go:build cgo + +package cli + +import ( + "io" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestParseLocalFileCorrectsOfficeFormat(t *testing.T) { + data, err := os.ReadFile("../parser/parser/testdata/docx_table_image.docx") + if err != nil { + t.Fatal(err) + } + filename := filepath.Join(t.TempDir(), "renamed.doc") + if err := os.WriteFile(filename, data, 0600); err != nil { + t.Fatal(err) + } + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + defer r.Close() + defer w.Close() + previous := os.Stderr + os.Stderr = w + t.Cleanup(func() { os.Stderr = previous }) + c := &CLI{Config: &CommandLineConfig{CLIMode: APIMode}} + response, err := c.APIParseLocalFileCommand(1, &Command{Params: map[string]interface{}{"filename": filename}}) + if err != nil { + t.Fatal(err) + } + if result, ok := response.(*SimpleResponse); !ok || result.Code != 0 { + t.Fatalf("unexpected response: %#v", response) + } + if err := w.Close(); err != nil { + t.Fatal(err) + } + warning, err := io.ReadAll(r) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(warning), "WARNING:") || !strings.Contains(string(warning), "docx") { + t.Fatalf("missing format correction warning: %q", warning) + } +} diff --git a/internal/ingestion/component/chunker/image_upload_test.go b/internal/ingestion/component/chunker/image_upload_test.go index 794360e076..e6c81e89d7 100644 --- a/internal/ingestion/component/chunker/image_upload_test.go +++ b/internal/ingestion/component/chunker/image_upload_test.go @@ -120,7 +120,7 @@ func TestImageUploadDecorator_MarkdownMediaRefMaterializes(t *testing.T) { } cache := &chunkerMediaCache{} ctx := medialib.WithCache(t.Context(), cache) - p, err := parserlib.GetParser(utility.FileTypeMarkdown) + p, _, err := parserlib.GetParser(utility.FileTypeMarkdown, nil) if err != nil { t.Fatalf("GetParser(FileTypeMarkdown): %v", err) } diff --git a/internal/ingestion/component/image_ocr_test.go b/internal/ingestion/component/image_ocr_test.go index d46b599a24..5d051d3ecf 100644 --- a/internal/ingestion/component/image_ocr_test.go +++ b/internal/ingestion/component/image_ocr_test.go @@ -174,7 +174,7 @@ func TestImageOCRSelectedTenantModel(t *testing.T) { data := picturePNG(t) setups := imageSwitchSetups(t, true) setups["image"]["ocr_model_id"] = "model-b" - res, handled, err := maybeDispatchImage(t.Context(), db, utility.FileTypeVISUAL, + res, handled, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), db, utility.FileTypeVISUAL, "invoice.png", data, map[string]any{"tenant_id": "tenant-1"}, setups, false, visionSettings{}) if err != nil || !handled { t.Fatalf("handled=%v error=%v", handled, err) @@ -238,7 +238,7 @@ func TestImageOCRReleasesMediaAdmissionBeforeRequest(t *testing.T) { } setups := imageSwitchSetups(t, true) setups["image"]["ocr_model_id"] = "model-b" - result, _, err := maybeDispatchImage(t.Context(), db, utility.FileTypeVISUAL, + result, _, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), db, utility.FileTypeVISUAL, "invoice.png", picturePNG(t), map[string]any{"tenant_id": "tenant-1"}, setups, false, visionSettings{}) if err != nil || len(result.JSON) != 1 { t.Fatalf("err=%v result=%+v", err, result) @@ -275,7 +275,7 @@ func TestImageOCRFailureRetainsAttachment(t *testing.T) { db, requests := imageOCRFixture(t, provider, true) setups := imageSwitchSetups(t, true) setups["image"]["ocr_model_id"] = "model-b" - res, _, err := maybeDispatchImage(t.Context(), db, utility.FileTypeVISUAL, + res, _, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), db, utility.FileTypeVISUAL, "invoice.png", picturePNG(t), map[string]any{"tenant_id": "tenant-1"}, setups, false, visionSettings{}) if err != nil { t.Fatal(err) @@ -321,7 +321,7 @@ func TestImageOCRInvalidSelectionNeverFallsBack(t *testing.T) { } setups := imageSwitchSetups(t, true) setups["image"]["ocr_model_id"] = tc.ref - res, _, err := maybeDispatchImage(t.Context(), db, utility.FileTypeVISUAL, + res, _, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), db, utility.FileTypeVISUAL, "invoice.png", picturePNG(t), map[string]any{"tenant_id": tc.tenant}, setups, false, visionSettings{}) if err != nil { t.Fatal(err) @@ -351,7 +351,7 @@ func TestImageOCRSelectionRespectsSwitchAndSVG(t *testing.T) { t.Run(tc.name, func(t *testing.T) { setups := imageSwitchSetups(t, tc.ocr) setups["image"]["ocr_model_id"] = "model-b" - res, _, err := maybeDispatchImage(t.Context(), db, utility.FileTypeVISUAL, + res, _, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), db, utility.FileTypeVISUAL, tc.filename, tc.data, map[string]any{"tenant_id": "tenant-1"}, setups, false, visionSettings{}) if err != nil || len(res.JSON) != 1 { t.Fatalf("err=%v result=%+v", err, res) diff --git a/internal/ingestion/component/media_dispatch.go b/internal/ingestion/component/media_dispatch.go index ad8073e320..72706e3d12 100644 --- a/internal/ingestion/component/media_dispatch.go +++ b/internal/ingestion/component/media_dispatch.go @@ -81,6 +81,7 @@ func maybeDispatchVideo( // preserves the original image and optionally appends global vision enhancement. func maybeDispatchImage( ctx context.Context, + p parser.ParseResultProducer, db *gorm.DB, fileType utility.FileType, filename string, @@ -140,7 +141,7 @@ func maybeDispatchImage( } } release() - parsed := dispatchParse(ctx, fileType, filename, binary, setups, progress.Window{}) + parsed := dispatchParse(ctx, p, fileType, filename, binary, setups, progress.Window{}) if parsed.Err != nil { return parsed, true, parsed.Err } diff --git a/internal/ingestion/component/media_dispatch_integration_test.go b/internal/ingestion/component/media_dispatch_integration_test.go index 69e808f3f7..106fc53038 100644 --- a/internal/ingestion/component/media_dispatch_integration_test.go +++ b/internal/ingestion/component/media_dispatch_integration_test.go @@ -61,7 +61,7 @@ func assertNativeOCRTextAndImage(t *testing.T, result parser.ParseResult) { // reading of canvases saved before the ocr_enabled switch existed. func TestMaybeDispatchImageNativeOCRWithoutVision(t *testing.T) { registerNativeOCRModels(t) - result, handled, err := maybeDispatchImage(t.Context(), nil, utility.FileTypeVISUAL, + result, handled, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), nil, utility.FileTypeVISUAL, "picture_ocr.png", pictureOCRFixture, nil, defaultSetups(), false, visionSettings{}) if err != nil { t.Fatal(err) @@ -77,7 +77,7 @@ func TestMaybeDispatchImageNativeOCRWithoutVision(t *testing.T) { // would only ever be exercised against a fake analyzer. func TestMaybeDispatchImageNativeOCRWithSwitch(t *testing.T) { registerNativeOCRModels(t) - result, handled, err := maybeDispatchImage(t.Context(), nil, utility.FileTypeVISUAL, + result, handled, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), nil, utility.FileTypeVISUAL, "picture_ocr.png", pictureOCRFixture, nil, imageSetupsWithSwitch(true), false, visionSettings{}) if err != nil { t.Fatal(err) @@ -94,7 +94,7 @@ func TestMaybeDispatchImageNativeOCRWithSwitch(t *testing.T) { // dispatch says so instead of failing silently. func TestMaybeDispatchImageNativeSwitchOffRunsNoOCR(t *testing.T) { registerNativeOCRModels(t) - result, handled, err := maybeDispatchImage(t.Context(), nil, utility.FileTypeVISUAL, + result, handled, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), nil, utility.FileTypeVISUAL, "picture_ocr.png", pictureOCRFixture, nil, imageSetupsWithSwitch(false), false, visionSettings{}) if err != nil || result.Err != nil { t.Fatalf("degraded config must not error: err = %v, result.Err = %v", err, result.Err) diff --git a/internal/ingestion/component/media_dispatch_test.go b/internal/ingestion/component/media_dispatch_test.go index 56befd96d5..96a668795a 100644 --- a/internal/ingestion/component/media_dispatch_test.go +++ b/internal/ingestion/component/media_dispatch_test.go @@ -176,8 +176,7 @@ func TestMaybeDispatchImage_ParseMethodModelDoesNotOverrideGlobalEnhancement(t * setups := defaultSetups() setups["image"]["parse_method"] = "custom-vlm@provider" - res, dispatched, err := maybeDispatchImage( - t.Context(), + res, dispatched, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), dao.DB, utility.FileTypeVISUAL, "photo.png", @@ -235,7 +234,7 @@ func TestMaybeDispatchImage_OCRWithoutEnhancementSkipsVision(t *testing.T) { setups := defaultSetups() setups["image"]["parse_method"] = "ocr" - res, _, err := maybeDispatchImage(t.Context(), dao.DB, utility.FileTypeVISUAL, "photo.png", picturePNG(t), map[string]any{"tenant_id": "t1"}, setups, false, visionSettings{}) + res, _, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), dao.DB, utility.FileTypeVISUAL, "photo.png", picturePNG(t), map[string]any{"tenant_id": "t1"}, setups, false, visionSettings{}) if err != nil { t.Fatalf("maybeDispatchImage: %v", err) } @@ -325,8 +324,7 @@ func TestMaybeDispatchImage_UsesSystemPrompt(t *testing.T) { setups["image"]["system_prompt"] = "自定义视觉提示" ctx := t.Context() - res, dispatched, err := maybeDispatchImage( - ctx, + res, dispatched, err := maybeDispatchImage(ctx, parser.NewPictureParser(), dao.DB, utility.FileTypeVISUAL, "test.png", @@ -377,8 +375,7 @@ func TestMaybeDispatchImage_DefaultPromptUsesDatasetLanguage(t *testing.T) { setups["image"]["lang"] = "Chinese" setups["image"]["system_prompt"] = "" - _, _, err := maybeDispatchImage( - t.Context(), + _, _, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), dao.DB, utility.FileTypeVISUAL, "test.png", @@ -448,8 +445,7 @@ func TestMaybeDispatchImage_GlobalVisionPromptDrivesImage(t *testing.T) { setups := defaultSetups() setups["image"]["lang"] = tc.familyLang setups["image"]["system_prompt"] = tc.familyPrompt - _, _, err := maybeDispatchImage( - t.Context(), dao.DB, utility.FileTypeVISUAL, "test.png", picturePNG(t), + _, _, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), dao.DB, utility.FileTypeVISUAL, "test.png", picturePNG(t), tc.inputs, setups, true, visionSettings{systemPrompt: tc.globalPrompt}, ) @@ -484,8 +480,7 @@ func TestMaybeDispatchImage_ReturnsJSONWithImage(t *testing.T) { setups := defaultSetups() ctx := t.Context() - res, dispatched, err := maybeDispatchImage( - ctx, + res, dispatched, err := maybeDispatchImage(ctx, parser.NewPictureParser(), dao.DB, utility.FileTypeVISUAL, "test.png", @@ -533,8 +528,7 @@ func TestMaybeDispatchImage_HardcodesJSONOutput(t *testing.T) { setups := defaultSetups() ctx := t.Context() - res, _, err := maybeDispatchImage( - ctx, + res, _, err := maybeDispatchImage(ctx, parser.NewPictureParser(), dao.DB, utility.FileTypeVISUAL, "test.png", @@ -670,8 +664,7 @@ func TestMaybeDispatchImage_UsesConfiguredVLMModel(t *testing.T) { setups := defaultSetups() setups["image"]["parse_method"] = "custom-vlm@provider" - _, dispatched, err := maybeDispatchImage( - t.Context(), + _, dispatched, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), dao.DB, utility.FileTypeVISUAL, "test.png", @@ -805,7 +798,7 @@ func TestMaybeDispatchImageWithoutOCRTextKeepsImage(t *testing.T) { for _, method := range []string{"ocr", ""} { setups := defaultSetups() setups["image"]["parse_method"] = method - result, handled, err := maybeDispatchImage(t.Context(), nil, utility.FileTypeVISUAL, "photo.png", picturePNG(t), nil, setups, false, visionSettings{}) + result, handled, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), nil, utility.FileTypeVISUAL, "photo.png", picturePNG(t), nil, setups, false, visionSettings{}) if err != nil { t.Fatal(err) } @@ -829,7 +822,7 @@ func TestMaybeDispatchImageUnhealthyOCRKeepsImage(t *testing.T) { deepdoctype.NativeDocAnalyzerFactory = func() (deepdoctype.DocAnalyzer, bool) { return &unhealthyPictureOCRAnalyzer{}, true } - result, handled, err := maybeDispatchImage(t.Context(), nil, utility.FileTypeVISUAL, "photo.png", picturePNG(t), nil, defaultSetups(), false, visionSettings{}) + result, handled, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), nil, utility.FileTypeVISUAL, "photo.png", picturePNG(t), nil, defaultSetups(), false, visionSettings{}) if err != nil || result.Err != nil { t.Fatalf("unhealthy OCR should degrade without an error: err = %v, result.Err = %v", err, result.Err) } @@ -845,7 +838,7 @@ func TestMaybeDispatchImageRejectsUndecodableBytes(t *testing.T) { for _, method := range []string{"ocr", "custom-vlm"} { setups := defaultSetups() setups["image"]["parse_method"] = method - _, _, err := maybeDispatchImage(t.Context(), nil, utility.FileTypeVISUAL, "photo.png", []byte("bad image"), nil, setups, false, visionSettings{}) + _, _, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), nil, utility.FileTypeVISUAL, "photo.png", []byte("bad image"), nil, setups, false, visionSettings{}) if err == nil || !strings.Contains(err.Error(), "decode") { t.Fatalf("method %q: error = %v, want decode error", method, err) } @@ -865,7 +858,7 @@ func TestMaybeDispatchImageReleasesAdmissionAfterPanic(t *testing.T) { t.Error("expected OCR analyzer panic") } }() - _, _, _ = maybeDispatchImage(t.Context(), nil, utility.FileTypeVISUAL, "photo.png", data, nil, defaultSetups(), false, visionSettings{}) + _, _, _ = maybeDispatchImage(t.Context(), parser.NewPictureParser(), nil, utility.FileTypeVISUAL, "photo.png", data, nil, defaultSetups(), false, visionSettings{}) }() ctx, cancel := context.WithTimeout(t.Context(), time.Second) defer cancel() @@ -892,7 +885,7 @@ func TestMaybeDispatchImageDecodesRasterOnlyForOCR(t *testing.T) { t.Run(method, func(t *testing.T) { setups := defaultSetups() setups["image"]["parse_method"] = method - result, handled, err := maybeDispatchImage(t.Context(), nil, utility.FileTypeVISUAL, + result, handled, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), nil, utility.FileTypeVISUAL, "photo.png", []byte(magic), map[string]any{"tenant_id": "t1"}, setups, true, visionSettings{}) if method == "ocr" { if err == nil || !strings.Contains(err.Error(), "raster decoding requested") { @@ -935,7 +928,7 @@ func TestMaybeDispatchImageOCRSwitch(t *testing.T) { } t.Run("ocr-on", func(t *testing.T) { - res, handled, err := maybeDispatchImage(t.Context(), nil, utility.FileTypeVISUAL, "photo.png", picturePNG(t), + res, handled, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), nil, utility.FileTypeVISUAL, "photo.png", picturePNG(t), map[string]any{"tenant_id": "t1"}, ocrSetups(true), false, visionSettings{}) if err != nil || !handled { t.Fatalf("err = %v, handled = %v", err, handled) @@ -945,7 +938,7 @@ func TestMaybeDispatchImageOCRSwitch(t *testing.T) { } }) t.Run("ocr-off-no-enhancement-keeps-empty", func(t *testing.T) { - res, handled, err := maybeDispatchImage(t.Context(), nil, utility.FileTypeVISUAL, "photo.png", picturePNG(t), + res, handled, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), nil, utility.FileTypeVISUAL, "photo.png", picturePNG(t), map[string]any{"tenant_id": "t1"}, ocrSetups(false), false, visionSettings{}) if err != nil || !handled { t.Fatalf("err = %v, handled = %v", err, handled) @@ -961,7 +954,7 @@ func TestMaybeDispatchImageOCRSwitch(t *testing.T) { } }) t.Run("ocr-off-vision-only", func(t *testing.T) { - res, handled, err := maybeDispatchImage(t.Context(), nil, utility.FileTypeVISUAL, "photo.png", picturePNG(t), + res, handled, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), nil, utility.FileTypeVISUAL, "photo.png", picturePNG(t), map[string]any{"tenant_id": "t1"}, ocrSetups(false), true, visionSettings{}) if err != nil || !handled { t.Fatalf("err = %v, handled = %v", err, handled) @@ -973,7 +966,7 @@ func TestMaybeDispatchImageOCRSwitch(t *testing.T) { t.Run("switch-beats-stale-parse-method", func(t *testing.T) { setups := defaultSetups() // image still carries parse_method:"ocr" setups["image"]["ocr_enabled"] = false - res, _, err := maybeDispatchImage(t.Context(), nil, utility.FileTypeVISUAL, "photo.png", picturePNG(t), + res, _, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), nil, utility.FileTypeVISUAL, "photo.png", picturePNG(t), map[string]any{"tenant_id": "t1"}, setups, false, visionSettings{}) if err != nil { t.Fatal(err) @@ -1003,7 +996,7 @@ func TestMaybeDispatchImageBlankVisionDescriptionWarns(t *testing.T) { return blankAnswerVisionDriver{}, "vision", &modelModule.APIConfig{}, 0, nil } - res, handled, err := maybeDispatchImage(t.Context(), nil, utility.FileTypeVISUAL, "photo.png", picturePNG(t), + res, handled, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), nil, utility.FileTypeVISUAL, "photo.png", picturePNG(t), map[string]any{"tenant_id": "t1"}, imageSwitchSetups(t, false), true, visionSettings{}) if err != nil || !handled { t.Fatalf("err = %v, handled = %v", err, handled) diff --git a/internal/ingestion/component/parser.go b/internal/ingestion/component/parser.go index 33791d9ec1..71cd9ca5b1 100644 --- a/internal/ingestion/component/parser.go +++ b/internal/ingestion/component/parser.go @@ -495,7 +495,13 @@ func (c *ParserComponent) Invoke(ctx context.Context, db *gorm.DB, inputs map[st // "docx", ...). Used by parser.GetParser, whose switch // arms are keyed off the utility constants. // - fileTypeExt := fileTypeFromInputs(inputs) + requestedType := fileTypeFromInputs(inputs) + fileParser, fileTypeExt, selectionErr := parser.GetParser(requestedType, binary) + if selectionErr == nil && fileTypeExt != requestedType { + warning := fmt.Sprintf("file %q: requested format %s does not match detected format %s; continuing with the %s parser", filename, requestedType, fileTypeExt, fileTypeExt) + common.Warn(warning) + reportParserWarnings(ctx, []string{warning}) + } parsePagesOn := pdfPageProgressSupported(fileTypeExt, setups) enhanceOn := c.enableVisionEnhancement split := progress.NewSplit(parsePagesOn, enhanceOn) @@ -519,7 +525,7 @@ func (c *ParserComponent) Invoke(ctx context.Context, db *gorm.DB, inputs map[st var handledImage bool if !handledVision && !handledMedia { // Image dispatch: OCR with independently controlled VLM enhancement. - dispatched, handledImage, visionErr = maybeDispatchImage(ctx, db, fileTypeExt, filename, binary, inputs, setups, c.enableVisionEnhancement, c.vision) + dispatched, handledImage, visionErr = maybeDispatchImage(ctx, fileParser, db, fileTypeExt, filename, binary, inputs, setups, c.enableVisionEnhancement, c.vision) if visionErr != nil { return nil, visionErr } @@ -534,7 +540,10 @@ func (c *ParserComponent) Invoke(ctx context.Context, db *gorm.DB, inputs map[st } } if !handledVision && !handledMedia && !handledImage && !handledAudio { - dispatched = dispatchParse(ctx, fileTypeExt, filename, binary, setups, pageWin) + if selectionErr != nil && fileTypeExt != utility.FileTypeOTHER { + return nil, fmt.Errorf("parser: resolve %q: %w", fileTypeExt, selectionErr) + } + dispatched = dispatchParse(ctx, fileParser, fileTypeExt, filename, binary, setups, pageWin) if dispatched.Err == nil && ctx.Err() == nil { pageWin.Report(ctx, 1.0) } diff --git a/internal/ingestion/component/parser_dispatch.go b/internal/ingestion/component/parser_dispatch.go index 23fffb6039..b17b30dda2 100644 --- a/internal/ingestion/component/parser_dispatch.go +++ b/internal/ingestion/component/parser_dispatch.go @@ -58,8 +58,8 @@ func configureParserFromSetups(p any, fileType utility.FileType, setups map[stri cfg.ConfigureFromSetup(map[string]any(setup)) } -// dispatchParse resolves the parser for the given fileType and invokes -// its structured ParseWithResult contract. +// dispatchParse configures the selected parser and invokes its structured +// ParseWithResult contract using the effective file type. // // The function NEVER returns a partial result. On error the result // is the zero value (Err != nil, no JSON and no File). Callers detect the @@ -74,7 +74,7 @@ func configureParserFromSetups(p any, fileType utility.FileType, setups map[stri // re-reading setups. lib_type is no longer threaded through: the // Python dispatcher picks a single backend per family and the Go // constructors mirror that. -func dispatchParse(ctx context.Context, fileType utility.FileType, filename string, data []byte, setups map[string]schema.ParserSetup, window progress.Window) parser.ParseResult { +func dispatchParse(ctx context.Context, p parser.ParseResultProducer, fileType utility.FileType, filename string, data []byte, setups map[string]schema.ParserSetup, window progress.Window) parser.ParseResult { if fileType == utility.FileTypeOTHER { // Unknown / unset family. The component treats the bytes // as text pages; splitIntoPages handles it. We return no @@ -89,10 +89,6 @@ func dispatchParse(ctx context.Context, fileType utility.FileType, filename stri } } - p, err := parser.GetParser(fileType) - if err != nil { - return parser.ParseResult{Err: fmt.Errorf("parser: resolve %q: %w", fileType, err)} - } configureParserFromSetups(p, fileType, setups) attachPDFPageProgress(ctx, p, window) diff --git a/internal/ingestion/component/parser_try_parse_test.go b/internal/ingestion/component/parser_try_parse_test.go new file mode 100644 index 0000000000..b43773e2de --- /dev/null +++ b/internal/ingestion/component/parser_try_parse_test.go @@ -0,0 +1,156 @@ +package component + +import ( + "archive/zip" + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/xuri/excelize/v2" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" + "ragflow/internal/agent/runtime" + "ragflow/internal/common" +) + +func TestParserCorrectionUsesTargetSetupForRemoteParsing(t *testing.T) { + withSSRFBypass(t) + f := excelize.NewFile() + defer f.Close() + if err := f.SetCellValue("Sheet1", "A1", "Workbook"); err != nil { + t.Fatal(err) + } + data, err := f.WriteToBuffer() + if err != nil { + t.Fatal(err) + } + payload := tcadpZipFixtureForComponent(t) + var receivedType string + var server *httptest.Server + server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/reconstruct_document": + var request struct { + FileType string `json:"file_type"` + } + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + t.Error(err) + } + receivedType = request.FileType + _, _ = w.Write([]byte(`{"DocumentRecognizeResultUrl":"` + server.URL + `/download.zip"}`)) + case "/download.zip": + _, _ = w.Write(payload) + default: + http.NotFound(w, r) + } + })) + defer server.Close() + setups := defaultSetups() + setups["spreadsheet"]["parse_method"] = "tcadp" + setups["spreadsheet"]["tcadp_apiserver"] = server.URL + c := &ParserComponent{setups: setups} + out, err := c.Invoke(t.Context(), nil, map[string]any{"name": "renamed.doc", "binary": data.Bytes()}) + if err != nil { + t.Fatal(err) + } + if receivedType != "XLSX" { + t.Fatalf("remote file_type=%q, want XLSX", receivedType) + } + if out["file_type"] != "xlsx" { + t.Fatalf("output file_type=%v", out["file_type"]) + } + requireJSONText(t, out, "Hello TCADP") +} + +func TestParserComponentCorrectsDOCXUploadedAsDOC(t *testing.T) { + var buf bytes.Buffer + w := zip.NewWriter(&buf) + for name, content := range map[string]string{ + "[Content_Types].xml": ``, + "_rels/.rels": ``, + "word/document.xml": `Recognized document`, + } { + f, err := w.Create(name) + if err != nil { + t.Fatal(err) + } + if _, err := f.Write([]byte(content)); err != nil { + t.Fatal(err) + } + } + if err := w.Close(); err != nil { + t.Fatal(err) + } + c, err := NewParserComponent(map[string]any{}) + if err != nil { + t.Fatal(err) + } + core, logs := observer.New(zap.WarnLevel) + originalLogger := common.Logger + common.Logger = zap.New(core) + t.Cleanup(func() { common.Logger = originalLogger }) + for _, suffix := range []string{"doc", "xls", "ppt", "wps", "docx"} { + t.Run(suffix, func(t *testing.T) { + var messages []string + ctx := runtime.WithProgressMessageCallback(t.Context(), func(_, message string) { messages = append(messages, message) }) + logs.TakeAll() + name := "renamed." + suffix + out, err := c.Invoke(ctx, nil, map[string]any{"name": name, "binary": buf.Bytes()}) + if err != nil { + t.Fatal(err) + } + if out["file_type"] != "docx" || out["name"] != name { + t.Fatalf("incorrect output metadata: type=%v name=%v", out["file_type"], out["name"]) + } + requireJSONText(t, out, "Recognized document") + if suffix == "docx" { + if len(messages) != 0 || logs.Len() != 0 { + t.Fatal("matching format should not warn") + } + return + } + if len(messages) != 1 || !strings.Contains(messages[0], "WARNING:") || !strings.Contains(messages[0], "docx") { + t.Fatalf("expected correction warning, got %v", messages) + } + if logs.Len() != 1 { + t.Fatalf("expected one server warning, got %d", logs.Len()) + } + }) + } +} + +func TestParserComponentKeepsUnrecognizedFormat(t *testing.T) { + c, err := NewParserComponent(map[string]any{}) + if err != nil { + t.Fatal(err) + } + var messages []string + ctx := runtime.WithProgressMessageCallback(t.Context(), func(_, message string) { messages = append(messages, message) }) + _, err = c.Invoke(ctx, nil, map[string]any{"name": "broken.doc", "binary": []byte("unrecognized content")}) + if err == nil || !strings.Contains(err.Error(), `"doc"`) { + t.Fatalf("expected original DOC parser error, got %v", err) + } + if len(messages) != 0 { + t.Fatalf("unknown content should not trigger a correction warning: %v", messages) + } +} + +func TestParserComponentSkipsUnavailableTypeCheck(t *testing.T) { + c, err := NewParserComponent(map[string]any{}) + if err != nil { + t.Fatal(err) + } + var messages []string + ctx := runtime.WithProgressMessageCallback(t.Context(), func(_, message string) { messages = append(messages, message) }) + out, err := c.Invoke(ctx, nil, map[string]any{"name": "notes.txt", "binary": []byte("Unchanged text")}) + if err != nil { + t.Fatal(err) + } + if out["file_type"] != "txt" || len(messages) != 0 { + t.Fatalf("unexpected type correction: type=%v messages=%v", out["file_type"], messages) + } + requireJSONText(t, out, "Unchanged text") +} diff --git a/internal/ingestion/component/svg_text_test.go b/internal/ingestion/component/svg_text_test.go index 06f36e8fba..ab88af0d29 100644 --- a/internal/ingestion/component/svg_text_test.go +++ b/internal/ingestion/component/svg_text_test.go @@ -189,7 +189,7 @@ func TestMaybeDispatchImageSVGWarnings(t *testing.T) { } t.Cleanup(func() { resolveTenantModelByType = original }) textless := []byte(``) - result, handled, err := maybeDispatchImage(t.Context(), nil, utility.FileTypeVISUAL, "shape.SVG", textless, map[string]any{"tenant_id": "t1"}, defaultSetups(), true, visionSettings{}) + result, handled, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), nil, utility.FileTypeVISUAL, "shape.SVG", textless, map[string]any{"tenant_id": "t1"}, defaultSetups(), true, visionSettings{}) if err != nil { t.Fatal(err) } @@ -211,11 +211,11 @@ func TestMaybeDispatchImageRejectsInvalidSVG(t *testing.T) { for _, ocr := range []bool{true, false} { setups := defaultSetups() setups["image"]["ocr_enabled"] = ocr - _, handled, err := maybeDispatchImage(t.Context(), nil, utility.FileTypeVISUAL, "broken.svg", []byte(`a`), nil, setups, false, visionSettings{}) + _, handled, err := maybeDispatchImage(t.Context(), parser.NewPictureParser(), nil, utility.FileTypeVISUAL, "broken.svg", []byte(`a`), nil, setups, false, visionSettings{}) if !handled || err == nil || !strings.Contains(err.Error(), "parser: decode SVG") { t.Fatalf("ocr %v: handled = %v, error = %v, want SVG decode error", ocr, handled, err) } - _, _, err = maybeDispatchImage(t.Context(), nil, utility.FileTypeVISUAL, "empty.svg", nil, nil, setups, false, visionSettings{}) + _, _, err = maybeDispatchImage(t.Context(), parser.NewPictureParser(), nil, utility.FileTypeVISUAL, "empty.svg", nil, nil, setups, false, visionSettings{}) if err == nil || !strings.Contains(err.Error(), "size limits") { t.Fatalf("ocr %v: error = %v, want size limit error", ocr, err) } diff --git a/internal/parser/parser/docx_extract_test.go b/internal/parser/parser/docx_extract_test.go new file mode 100644 index 0000000000..bb9434404e --- /dev/null +++ b/internal/parser/parser/docx_extract_test.go @@ -0,0 +1,51 @@ +package parser + +import ( + "fmt" + "testing" +) + +// TestDOCXParser_JSONEmptyIRReturnsError locks Finding B: when the native +// backend opens the document but returns an empty structured IR, the JSON +// output path must fail loud (it is built entirely from the IR, which the +// Markdown path can fall back from plain text). This guards against silently +// emitting empty JSON sections. +func TestDOCXParser_JSONEmptyIRReturnsError(t *testing.T) { + ctx := t.Context() + p := NewDOCXParser() + + orig := docxExtract + defer func() { docxExtract = orig }() + docxExtract = func(data []byte, format string) (string, string, string, error, error) { + // Open succeeds, but the IR view is empty. + return "", "", "", nil, nil + } + + res := p.ParseWithResult(ctx, "empty.docx", []byte("PK\x03\x04")) + if res.Err == nil { + t.Fatal("expected error for JSON output with empty IR, got nil") + } +} + +// TestDOCXParser_AllViewsEmptyReturnsError locks the engine-level contract +// at the parser boundary: when the native backend opens the document but +// every view (IR / Markdown / PlainText) is empty, OpenAndExtract reports a +// joined error, and the parser must fail loud rather than silently emit +// empty JSON. (office_oxide always returns a structured IR for an openable +// doc, so this all-empty error is simulated via the seam rather than a real +// fixture.) +func TestDOCXParser_AllViewsEmptyReturnsError(t *testing.T) { + ctx := t.Context() + p := NewDOCXParser() + + orig := docxExtract + defer func() { docxExtract = orig }() + docxExtract = func(data []byte, format string) (string, string, string, error, error) { + return "", "", "", nil, fmt.Errorf("office_oxide docx: all text views failed") + } + + res := p.ParseWithResult(ctx, "empty.docx", []byte("PK\x03\x04")) + if res.Err == nil { + t.Fatal("expected error for a document with no extractable text in any view, got nil") + } +} diff --git a/internal/parser/parser/docx_parser.go b/internal/parser/parser/docx_parser.go index 3d5a9e59c8..780f1d7371 100644 --- a/internal/parser/parser/docx_parser.go +++ b/internal/parser/parser/docx_parser.go @@ -24,14 +24,11 @@ import ( ) // docxExtract is a test seam for the native office_oxide extraction. It -// defaults to the deepdoc/docx backend (which is the only place that depends -// on office_oxide) and is overridden in tests to capture the effective -// container format passed to the engine. +// defaults to the deepdoc/office backend, which owns the native dependency. var docxExtract = deepdocoffice.OpenAndExtract -// DOCXParser is the DOCX/DOC parser. It is a CGO-free facade: magic-byte -// sniffing and post-processing live here, while the native extraction lives -// in internal/deepdoc/parser/office. The IR data model and postprocessing live +// DOCXParser parses DOCX documents. Native extraction lives in +// internal/deepdoc/parser/office. The IR data model and postprocessing live // in cgo-free files (docx_ir.go, docx_postprocess.go, office_table_render.go) // so they compile and test without native libraries. type DOCXParser struct { @@ -62,28 +59,8 @@ func (p *DOCXParser) ConfigureFromSetup(setup map[string]any) { // ParseWithResult produces structured JSON items from a docx document. // Embedded images are stored in the run media cache and emitted as media_ref; // text-only parsers skip them instead. -// -// File["format"] in the returned ParseResult reflects the effective -// container format after magic-byte sniffing (e.g. "doc" for an OLE -// payload uploaded as .docx), not merely the file extension. See -// office_detect.go:officeContainer for the detection contract. func (p *DOCXParser) ParseWithResult(ctx context.Context, filename string, data []byte) ParseResult { - // office_oxide's OpenFromBytes takes the container format as given - // and does no magic-byte detection, so a legacy .doc uploaded with a - // .docx extension would be parsed as ZIP/OOXML and fail. Sniff the - // real container and pass the matching format so mislabeled files - // still parse (mirrors the magic-byte correction office_oxide::Open - // performs on file paths). - format := "docx" - if officeContainer(data) == "ole" { - format = "doc" - } - // Note: when format == "doc" (OLE fallback), header/footer and TOC - // post-processing below degrades gracefully: extractDOCXHeaderFooterTexts - // opens the payload as ZIP and fails closed, so RemoveHeaderFooter becomes - // a no-op for legacy OLE inputs. This is acceptable — legacy DOC has no - // OOXML header/footer parts to strip. - irJSON, _, _, _, err := docxExtract(data, format) + irJSON, _, _, _, err := docxExtract(data, "docx") if err != nil { if errors.Is(err, deepdocoffice.ErrOfficeCGORequired) { return ParseResult{Err: fmt.Errorf("%w: %s", ErrOfficeCGORequired, filename)} @@ -93,7 +70,7 @@ func (p *DOCXParser) ParseWithResult(ctx context.Context, filename string, data fileMeta := map[string]any{ "name": filename, - "format": format, + "format": "docx", } var sink *officeMediaSink diff --git a/internal/parser/parser/email_parser.go b/internal/parser/parser/email_parser.go index 8a6bd91e3e..b4ad57fee7 100644 --- a/internal/parser/parser/email_parser.go +++ b/internal/parser/parser/email_parser.go @@ -606,7 +606,7 @@ func recoverParse(fn func() ParseResult) (res ParseResult, panicked bool) { // document. // // KNOWN LIMITATION: re-chunk uses the text-only, default-config parser -// returned by GetTextOnlyParser(ft) (and, for a nested email, only the +// returned by GetTextOnlyParser (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 @@ -688,10 +688,13 @@ func (p *EmailParser) rechunkEmailAttachments(ctx context.Context, content map[s return ep.parseEmail(ctx, fn, []byte(payload), depth+1) }) } else { - np, err := GetTextOnlyParser(ft) + np, actual, err := GetTextOnlyParser(ft, []byte(payload)) if err != nil { continue } + if actual != ft { + common.Warn(fmt.Sprintf("email attachment %q: requested format %s; detected %s; continuing with the %s parser", fn, ft, actual, actual)) + } res, panicked = recoverParse(func() ParseResult { return np.ParseWithResult(ctx, fn, []byte(payload)) }) diff --git a/internal/parser/parser/html_remote_media_test.go b/internal/parser/parser/html_remote_media_test.go index 7c01301907..c98a412a79 100644 --- a/internal/parser/parser/html_remote_media_test.go +++ b/internal/parser/parser/html_remote_media_test.go @@ -299,7 +299,7 @@ func TestHTMLTextParserSkipsRemoteImages(t *testing.T) { var hits atomic.Int32 server := imageTestServer(t, "remote-bytes", &hits) - p, err := GetTextOnlyParser(utility.FileTypeHTML) + p, _, err := GetTextOnlyParser(utility.FileTypeHTML, nil) if err != nil { t.Fatalf("GetTextOnlyParser: %v", err) } diff --git a/internal/parser/parser/markdown_media_test.go b/internal/parser/parser/markdown_media_test.go index 92dba00cc7..d3028d9590 100644 --- a/internal/parser/parser/markdown_media_test.go +++ b/internal/parser/parser/markdown_media_test.go @@ -36,7 +36,7 @@ const markdownMediaDataURI = "data:image/png;base64,aGVsbG8=" func markdownIngestionParser(t *testing.T) ParseResultProducer { t.Helper() - p, err := GetParser(utility.FileTypeMarkdown) + p, _, err := GetParser(utility.FileTypeMarkdown, nil) if err != nil { t.Fatalf("GetParser(FileTypeMarkdown): %v", err) } @@ -592,7 +592,7 @@ func TestMarkdownTextParserSkipsImages(t *testing.T) { var hits atomic.Int32 server := imageTestServer(t, "\x89PNG\r\n\x1a\nremote", &hits) - p, err := GetTextOnlyParser(utility.FileTypeMarkdown) + p, _, err := GetTextOnlyParser(utility.FileTypeMarkdown, nil) if err != nil { t.Fatalf("GetTextOnlyParser(FileTypeMarkdown): %v", err) } diff --git a/internal/parser/parser/office_detect.go b/internal/parser/parser/office_detect.go deleted file mode 100644 index d310f10d7f..0000000000 --- a/internal/parser/parser/office_detect.go +++ /dev/null @@ -1,29 +0,0 @@ -package parser - -// officeContainer reports the on-disk container format of an Office -// document by sniffing its leading magic bytes. It returns "ooxml" for -// the ZIP-based OOXML family (docx/xlsx/pptx), "ole" for the legacy -// binary OLE family (doc/xls/ppt), or "" when the container cannot be -// determined from the first bytes. The return values are internal -// markers — callers map them to the concrete office_oxide format strings -// ("docx"→"doc", "pptx"→"ppt"); this avoids coupling the detector to a -// single family and keeps it reusable for xlsx/xls if needed later. -// -// The office_oxide backend's OpenFromBytes requires the caller to name -// the container format and does no magic-byte detection of its own, so -// a legacy file renamed to an OOXML extension (e.g. a .doc uploaded as -// .docx) is otherwise parsed as ZIP and fails with OFFICE_ERR_PARSE. -// Sniffing here lets the cgo parsers pass the real container format. -func officeContainer(data []byte) string { - if len(data) >= 4 && data[0] == 0x50 && data[1] == 0x4B && data[2] == 0x03 && data[3] == 0x04 { - return "ooxml" - } - // OLE Compound Document signature: D0 CF 11 E0 A1 B1 1A E1. Require - // the full eight bytes to avoid misclassifying truncated input - // (a four-byte prefix could also be garbage). - if len(data) >= 8 && data[0] == 0xD0 && data[1] == 0xCF && data[2] == 0x11 && data[3] == 0xE0 && - data[4] == 0xA1 && data[5] == 0xB1 && data[6] == 0x1A && data[7] == 0xE1 { - return "ole" - } - return "" -} diff --git a/internal/parser/parser/office_detect_test.go b/internal/parser/parser/office_detect_test.go deleted file mode 100644 index 52db5a87fb..0000000000 --- a/internal/parser/parser/office_detect_test.go +++ /dev/null @@ -1,228 +0,0 @@ -package parser - -import ( - "fmt" - "testing" -) - -// TestOfficeContainer verifies the magic-byte sniffing that lets the -// office_oxide parsers recover from a mislabeled extension (e.g. a -// legacy .doc uploaded as .docx). office_oxide::OpenFromBytes takes the -// container format on trust and does no detection of its own. -func TestOfficeContainer(t *testing.T) { - tests := []struct { - name string - data []byte - want string - }{ - {"ooxml", []byte{0x50, 0x4B, 0x03, 0x04, 0x00, 0x00}, "ooxml"}, - {"ole", []byte{0xD0, 0xCF, 0x11, 0xE0, 0xA1, 0xB1, 0x1A, 0xE1}, "ole"}, - {"ole_with_trailing", []byte{0xD0, 0xCF, 0x11, 0xE0, 0xA1, 0xB1, 0x1A, 0xE1, 0x00, 0xFF}, "ole"}, - {"ole_short_header", []byte{0xD0, 0xCF, 0x11, 0xE0}, ""}, - {"ole_truncated_7", []byte{0xD0, 0xCF, 0x11, 0xE0, 0xA1, 0xB1, 0x1A}, ""}, - {"empty", []byte{}, ""}, - {"too_short", []byte{0x50, 0x4B}, ""}, - {"ooxml_truncated_3", []byte{0x50, 0x4B, 0x03}, ""}, - {"garbage", []byte{0x00, 0x11, 0x22, 0x33}, ""}, - {"ole_prefix_mismatch", []byte{0xD0, 0xCF, 0x11, 0xE0, 0xA1, 0xB1, 0x1A, 0x00}, ""}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := officeContainer(tt.data); got != tt.want { - t.Errorf("officeContainer(% x) = %q, want %q", tt.data, got, tt.want) - } - }) - } -} - -// TestDOCXParser_FallsBackToDOCForOLEHeader ensures a legacy OLE .doc -// payload routed to the DOCXParser (the .doc-as-.docx case) is opened -// with the "doc" container format rather than "docx". A minimal OLE -// header is not a valid document, so office_oxide still errors — but -// the error must come from the DOC path, proving the fallback fired. -// The test uses a seam to capture the effective format without needing -// a full valid OLE document fixture. -func TestDOCXParser_FallsBackToDOCForOLEHeader(t *testing.T) { - ctx := t.Context() - p := NewDOCXParser() - - orig := docxExtract - defer func() { docxExtract = orig }() - - var gotFormat string - docxExtract = func(data []byte, format string) (string, string, string, error, error) { - gotFormat = format - return "", "", "", nil, fmt.Errorf("stub doc open: %s", format) - } - - data := []byte{0xD0, 0xCF, 0x11, 0xE0, 0xA1, 0xB1, 0x1A, 0xE1} - res := p.ParseWithResult(ctx, "renamed.docx", data) - if res.Err == nil { - t.Fatal("expected parse error for a non-document OLE header, got nil") - } - if gotFormat != "doc" { - t.Fatalf("docxExtract format = %q, want %q (OLE fallback not triggered)", gotFormat, "doc") - } -} - -// TestDOCXParser_JSONEmptyIRReturnsError locks Finding B: when the native -// backend opens the document but returns an empty structured IR, the JSON -// output path must fail loud (it is built entirely from the IR, which the -// Markdown path can fall back from plain text). This guards against silently -// emitting empty JSON sections. -func TestDOCXParser_JSONEmptyIRReturnsError(t *testing.T) { - ctx := t.Context() - p := NewDOCXParser() - - orig := docxExtract - defer func() { docxExtract = orig }() - docxExtract = func(data []byte, format string) (string, string, string, error, error) { - // Open succeeds, but the IR view is empty. - return "", "", "", nil, nil - } - - res := p.ParseWithResult(ctx, "empty.docx", []byte("PK\x03\x04")) - if res.Err == nil { - t.Fatal("expected error for JSON output with empty IR, got nil") - } -} - -// TestDOCXParser_AllViewsEmptyReturnsError locks the engine-level contract -// at the parser boundary: when the native backend opens the document but -// every view (IR / Markdown / PlainText) is empty, OpenAndExtract reports a -// joined error, and the parser must fail loud rather than silently emit -// empty JSON. (office_oxide always returns a structured IR for an openable -// doc, so this all-empty error is simulated via the seam rather than a real -// fixture.) -func TestDOCXParser_AllViewsEmptyReturnsError(t *testing.T) { - ctx := t.Context() - p := NewDOCXParser() - - orig := docxExtract - defer func() { docxExtract = orig }() - docxExtract = func(data []byte, format string) (string, string, string, error, error) { - return "", "", "", nil, fmt.Errorf("office_oxide docx: all text views failed") - } - - res := p.ParseWithResult(ctx, "empty.docx", []byte("PK\x03\x04")) - if res.Err == nil { - t.Fatal("expected error for a document with no extractable text in any view, got nil") - } -} - -// TestDOCXParser_TruncatedOLENotFallback ensures a truncated OLE prefix -// (only 4 bytes) does not trigger the DOC fallback — it must stay "docx" -// so truncated garbage is not misclassified as OLE. -func TestDOCXParser_TruncatedOLENotFallback(t *testing.T) { - ctx := t.Context() - p := NewDOCXParser() - - orig := docxExtract - defer func() { docxExtract = orig }() - - var gotFormat string - docxExtract = func(data []byte, format string) (string, string, string, error, error) { - gotFormat = format - return "", "", "", nil, fmt.Errorf("stub") - } - - data := []byte{0xD0, 0xCF, 0x11, 0xE0} - res := p.ParseWithResult(ctx, "truncated.docx", data) - if res.Err == nil { - t.Fatal("expected error, got nil") - } - if gotFormat != "docx" { - t.Fatalf("format = %q, want %q for truncated OLE (should not fallback)", gotFormat, "docx") - } -} - -// TestPPTXParser_FallsBackToPPTForOLEHeader ensures a legacy OLE .ppt -// payload routed to PPTXParser is opened with "ppt" rather than "pptx". -func TestPPTXParser_FallsBackToPPTForOLEHeader(t *testing.T) { - ctx := t.Context() - p := NewPPTXParser() - - orig := pptxExtract - defer func() { pptxExtract = orig }() - - var gotFormat string - pptxExtract = func(data []byte, format string) (string, string, string, error, error) { - gotFormat = format - return "", "", "", nil, fmt.Errorf("stub ppt open: %s", format) - } - - data := []byte{0xD0, 0xCF, 0x11, 0xE0, 0xA1, 0xB1, 0x1A, 0xE1} - res := p.ParseWithResult(ctx, "renamed.pptx", data) - if res.Err == nil { - t.Fatal("expected parse error for OLE header, got nil") - } - if gotFormat != "ppt" { - t.Fatalf("pptxExtract format = %q, want %q", gotFormat, "ppt") - } - if p.format != "pptx" { - t.Fatalf("PPTXParser.format mutated to %q, want %q (must use local effFormat)", p.format, "pptx") - } -} - -// TestPPTParser_FallsBackToPPTXForOOXMLHeader ensures an OOXML .pptx -// payload uploaded as .ppt ( PPTParser with p.format == "ppt") is -// correctly routed to "pptx" via the bidirectional container sniffing. -func TestPPTParser_FallsBackToPPTXForOOXMLHeader(t *testing.T) { - ctx := t.Context() - p := NewPPTParser() // underlying PPTXParser{format:"ppt"} - - orig := pptxExtract - defer func() { pptxExtract = orig }() - - var gotFormat string - pptxExtract = func(data []byte, format string) (string, string, string, error, error) { - gotFormat = format - return "", "", "", nil, fmt.Errorf("stub ppt open: %s", format) - } - - // Minimal OOXML ZIP local file header. - data := []byte{0x50, 0x4B, 0x03, 0x04, 0x00, 0x00} - res := p.ParseWithResult(ctx, "renamed.ppt", data) - if res.Err == nil { - t.Fatal("expected parse error for OOXML header stub, got nil") - } - if gotFormat != "pptx" { - t.Fatalf("pptxExtract format = %q, want %q (OOXML fallback ppt->pptx)", gotFormat, "pptx") - } - // Underlying PPTXParser must not be mutated permanently. - if p.pptx.format != "ppt" { - t.Fatalf("PPTParser.pptx.format mutated to %q, want %q (must use local effFormat)", p.pptx.format, "ppt") - } -} - -// TestPPTXParser_NoStatePollution verifies that after an OLE file mutates -// the effective format for that call, a subsequent OOXML file still uses -// "pptx" — i.e., the parser instance is not permanently polluted. -func TestPPTXParser_NoStatePollution(t *testing.T) { - ctx := t.Context() - p := NewPPTXParser() - - orig := pptxExtract - defer func() { pptxExtract = orig }() - - calls := []string{} - pptxExtract = func(data []byte, format string) (string, string, string, error, error) { - calls = append(calls, format) - return "", "", "", nil, fmt.Errorf("stub") - } - - oleData := []byte{0xD0, 0xCF, 0x11, 0xE0, 0xA1, 0xB1, 0x1A, 0xE1} - _ = p.ParseWithResult(ctx, "a.pptx", oleData) - ooxmlData := []byte{0x50, 0x4B, 0x03, 0x04, 0x00, 0x00} - _ = p.ParseWithResult(ctx, "b.pptx", ooxmlData) - - if len(calls) != 2 { - t.Fatalf("calls = %v, want 2", calls) - } - if calls[0] != "ppt" { - t.Errorf("first call format = %q, want %q", calls[0], "ppt") - } - if calls[1] != "pptx" { - t.Errorf("second call format = %q, want %q (state polluted)", calls[1], "pptx") - } -} diff --git a/internal/parser/parser/office_media_test.go b/internal/parser/parser/office_media_test.go index 5a736e6eb1..1ebb9d6f47 100644 --- a/internal/parser/parser/office_media_test.go +++ b/internal/parser/parser/office_media_test.go @@ -373,7 +373,7 @@ func TestDOCXTextParserSkipsImagesWithoutCache(t *testing.T) { return irJSON, "", "", nil, nil } - p, err := GetTextOnlyParser(utility.FileTypeDOCX) + p, _, err := GetTextOnlyParser(utility.FileTypeDOCX, nil) if err != nil { t.Fatal(err) } @@ -418,7 +418,7 @@ func TestPPTXTextParserSkipsImagesWithoutCache(t *testing.T) { return irJSON, "", "", nil, nil } - p, err := GetTextOnlyParser(utility.FileTypePPTX) + p, _, err := GetTextOnlyParser(utility.FileTypePPTX, nil) if err != nil { t.Fatal(err) } diff --git a/internal/parser/parser/office_routes_cgo_test.go b/internal/parser/parser/office_routes_cgo_test.go index 713aafb530..7652a26f2c 100644 --- a/internal/parser/parser/office_routes_cgo_test.go +++ b/internal/parser/parser/office_routes_cgo_test.go @@ -22,7 +22,7 @@ func TestGetParser_RoutesOfficeFamilies(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - p, err := GetParser(tc.fileType) + p, _, err := GetParser(tc.fileType, nil) if err != nil { t.Fatalf("GetParser(%q): %v", tc.fileType, err) } diff --git a/internal/parser/parser/office_text_only_test.go b/internal/parser/parser/office_text_only_test.go index 34348d3c08..85b4f5fa99 100644 --- a/internal/parser/parser/office_text_only_test.go +++ b/internal/parser/parser/office_text_only_test.go @@ -47,13 +47,13 @@ func TestGetTextOnlyParserRoutesOfficeFamilies(t *testing.T) { return ok && parser.textOnly }}, {utility.FileTypeXLS, func(p ParseResultProducer) bool { - parser, ok := p.(*XLSParser) - return ok && parser.textOnly + _, ok := p.(*XLSParser) + return ok }}, } for _, tc := range tests { t.Run(string(tc.fileType), func(t *testing.T) { - p, err := GetTextOnlyParser(tc.fileType) + p, _, err := GetTextOnlyParser(tc.fileType, nil) if err != nil { t.Fatalf("GetTextOnlyParser: %v", err) } diff --git a/internal/parser/parser/office_try_parse.go b/internal/parser/parser/office_try_parse.go new file mode 100644 index 0000000000..66ba92e9ac --- /dev/null +++ b/internal/parser/parser/office_try_parse.go @@ -0,0 +1,85 @@ +package parser + +import ( + "archive/zip" + "bytes" + + "github.com/richardlehane/mscfb" + "ragflow/internal/utility" +) + +func (*DOCParser) TryParse(data []byte) utility.FileType { return tryParseOffice(data) } +func (*DOCXParser) TryParse(data []byte) utility.FileType { return tryParseOffice(data) } +func (*XLSParser) TryParse(data []byte) utility.FileType { return tryParseOffice(data) } +func (*XLSXParser) TryParse(data []byte) utility.FileType { return tryParseOffice(data) } +func (*PPTParser) TryParse(data []byte) utility.FileType { return tryParseOffice(data) } +func (*PPTXParser) TryParse(data []byte) utility.FileType { return tryParseOffice(data) } + +// tryParseOffice reads container directories without extracting document bodies. +// A container signature alone is insufficient to identify an Office format. +func tryParseOffice(data []byte) utility.FileType { + actual := utility.FileTypeOTHER + accept := func(candidate utility.FileType) bool { + if actual != utility.FileTypeOTHER && actual != candidate { + return false + } + actual = candidate + return true + } + switch { + case bytes.HasPrefix(data, []byte{'P', 'K', 3, 4}): + r, err := zip.NewReader(bytes.NewReader(data), int64(len(data))) + if err != nil { + return utility.FileTypeOTHER + } + hasContentTypes := false + for _, f := range r.File { + switch f.Name { + case "[Content_Types].xml": + hasContentTypes = true + case "word/document.xml": + if !accept(utility.FileTypeDOCX) { + return utility.FileTypeOTHER + } + case "xl/workbook.xml": + if !accept(utility.FileTypeXLSX) { + return utility.FileTypeOTHER + } + case "ppt/presentation.xml": + if !accept(utility.FileTypePPTX) { + return utility.FileTypeOTHER + } + } + } + if !hasContentTypes { + return utility.FileTypeOTHER + } + case bytes.HasPrefix(data, CompoundDocumentSignature): + r, err := mscfb.New(bytes.NewReader(data)) + if err != nil { + return utility.FileTypeOTHER + } + for _, f := range r.File { + if len(f.Path) != 0 || f.FileInfo().IsDir() { + continue + } + switch f.Name { + case "EncryptedPackage", "EncryptionInfo": + return utility.FileTypeOTHER + case "WordDocument": + if !accept(utility.FileTypeDOC) { + return utility.FileTypeOTHER + } + case "Workbook", "Book": + if !accept(utility.FileTypeXLS) { + return utility.FileTypeOTHER + } + case "PowerPoint Document": + if !accept(utility.FileTypePPT) { + return utility.FileTypeOTHER + } + } + } + } + return actual +} diff --git a/internal/parser/parser/office_try_parse_test.go b/internal/parser/parser/office_try_parse_test.go new file mode 100644 index 0000000000..437ebc0402 --- /dev/null +++ b/internal/parser/parser/office_try_parse_test.go @@ -0,0 +1,76 @@ +package parser + +import ( + "archive/zip" + "bytes" + "os" + "testing" + + "ragflow/internal/utility" +) + +func TestOfficeTryParse(t *testing.T) { + docx := officeProbeZIP(t, "[Content_Types].xml", "word/document.xml") + xlsx := officeProbeZIP(t, "[Content_Types].xml", "xl/workbook.xml") + pptx := officeProbeZIP(t, "[Content_Types].xml", "ppt/presentation.xml") + ordinaryZIP := officeProbeZIP(t, "notes.txt") + missingContentTypes := officeProbeZIP(t, "word/document.xml") + ambiguousZIP := officeProbeZIP(t, "[Content_Types].xml", "word/document.xml", "xl/workbook.xml") + doc, err := os.ReadFile("testdata/table.doc") + if err != nil { + t.Fatal(err) + } + for _, tc := range []struct { + name string + data []byte + want utility.FileType + }{ + {"docx", docx, utility.FileTypeDOCX}, + {"xlsx", xlsx, utility.FileTypeXLSX}, + {"pptx", pptx, utility.FileTypePPTX}, + {"doc", doc, utility.FileTypeDOC}, + {"ordinary_zip", ordinaryZIP, utility.FileTypeOTHER}, + {"missing_content_types", missingContentTypes, utility.FileTypeOTHER}, + {"ambiguous_zip", ambiguousZIP, utility.FileTypeOTHER}, + {"truncated_zip", []byte("PK\x03\x04"), utility.FileTypeOTHER}, + {"truncated_ole", CompoundDocumentSignature, utility.FileTypeOTHER}, + {"unknown", []byte("plain text"), utility.FileTypeOTHER}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := tryParseOffice(tc.data); got != tc.want { + t.Fatalf("got %q, want %q", got, tc.want) + } + }) + } +} + +func officeProbeZIP(t *testing.T, names ...string) []byte { + t.Helper() + var buf bytes.Buffer + w := zip.NewWriter(&buf) + for _, name := range names { + f, err := w.Create(name) + if err != nil { + t.Fatal(err) + } + if _, err := f.Write([]byte("")); err != nil { + t.Fatal(err) + } + } + if err := w.Close(); err != nil { + t.Fatal(err) + } + return buf.Bytes() +} + +func TestOfficeParsersSupportTryParse(t *testing.T) { + for _, ft := range []utility.FileType{utility.FileTypeDOC, utility.FileTypeDOCX, utility.FileTypeXLS, utility.FileTypeXLSX, utility.FileTypePPT, utility.FileTypePPTX} { + p, err := newParser(ft, false) + if err != nil { + t.Fatal(err) + } + if _, ok := p.(TryParser); !ok { + t.Fatalf("%s parser does not support TryParse", ft) + } + } +} diff --git a/internal/parser/parser/parse_result.go b/internal/parser/parser/parse_result.go index e311ae9823..61ee7a8d56 100644 --- a/internal/parser/parser/parse_result.go +++ b/internal/parser/parser/parse_result.go @@ -49,14 +49,7 @@ type ParseResult struct { // Nil when the backend does not produce metadata. It is not an augmented // copy of the upstream file descriptor. // - // For the office family (docx/doc, pptx/ppt), File["format"] - // reflects the real container format detected via magic-byte - // sniffing (officeContainer), not just the file extension. - // E.g. a legacy OLE .doc uploaded as .docx yields - // File["format"] == "doc" so downstream consumers see the - // effective format. This is an intentional contract change - // from the pre-fallback behaviour where File["format"] always - // echoed the requested extension. + // Office parsers report their selected format in File["format"]. File map[string]any // JSON is the structured payload when available, as typed DocItems. diff --git a/internal/parser/parser/parse_with_result_test.go b/internal/parser/parser/parse_with_result_test.go index 8baed39852..96a64766ff 100644 --- a/internal/parser/parser/parse_with_result_test.go +++ b/internal/parser/parser/parse_with_result_test.go @@ -289,7 +289,7 @@ func TestHTMLParser_ParseWithResult_SkipsScriptAndStyle(t *testing.T) { // `utility.FileTypeTXT` resolves to a TextParser that satisfies // ParseResultProducer. func TestGetParser_RoutesTextAndCode(t *testing.T) { - p, err := GetParser(utility.FileTypeTXT) + p, _, err := GetParser(utility.FileTypeTXT, nil) if err != nil { t.Fatalf("GetParser(FileTypeTXT): %v", err) } diff --git a/internal/parser/parser/parser_selection_test.go b/internal/parser/parser/parser_selection_test.go new file mode 100644 index 0000000000..aa1f269706 --- /dev/null +++ b/internal/parser/parser/parser_selection_test.go @@ -0,0 +1,112 @@ +//go:build cgo + +package parser + +import ( + "encoding/base64" + "os" + "testing" + + "github.com/xuri/excelize/v2" + "ragflow/internal/utility" +) + +func TestParserSelectionCorrectsFormat(t *testing.T) { + type selector func(utility.FileType, []byte) (ParseResultProducer, utility.FileType, error) + doc, err := os.ReadFile("testdata/table.doc") + if err != nil { + t.Fatal(err) + } + xlsx := newTestXLSX(t, func(f *excelize.File) { mustSetCell(t, f, "Sheet1", "A1", "Selected workbook") }) + xls, err := base64.StdEncoding.DecodeString(genuineBIFF8FixtureBase64) + if err != nil { + t.Fatal(err) + } + for mode, selectParser := range map[string]selector{"ingestion": GetParser, "text": GetTextOnlyParser} { + t.Run(mode, func(t *testing.T) { + for _, tc := range []struct { + name string + requested, want utility.FileType + data []byte + }{ + {"doc_as_docx", utility.FileTypeDOCX, utility.FileTypeDOC, doc}, + {"docx_as_doc", utility.FileTypeDOC, utility.FileTypeDOCX, minimalDOCX(t, "Selected document")}, + {"xlsx_as_xls", utility.FileTypeXLS, utility.FileTypeXLSX, xlsx}, + {"xls_as_xlsx", utility.FileTypeXLSX, utility.FileTypeXLS, xls}, + {"pptx_as_ppt", utility.FileTypePPT, utility.FileTypePPTX, buildPPTX(t, "Selected presentation")}, + {"unknown", utility.FileTypeDOC, utility.FileTypeDOC, []byte("unrecognized")}, + {"unchecked_text", utility.FileTypeTXT, utility.FileTypeTXT, []byte("plain text")}, + } { + t.Run(tc.name, func(t *testing.T) { + p, actual, err := selectParser(tc.requested, tc.data) + if err != nil { + t.Fatal(err) + } + if actual != tc.want { + t.Fatalf("format=%q, want %q", actual, tc.want) + } + res := p.ParseWithResult(t.Context(), "original."+string(tc.requested), tc.data) + if tc.name == "unknown" { + if res.Err == nil { + t.Fatal("unrecognized document should still fail in the original parser") + } + return + } + if res.Err != nil { + t.Fatal(res.Err) + } + if !res.Produced() { + t.Fatal("selected parser produced no result") + } + if actual != utility.FileTypeTXT && res.File["format"] != string(actual) { + t.Fatalf("result format=%v, selected type=%s", res.File["format"], actual) + } + }) + } + }) + } +} + +func TestOfficeParsersDoNotChangeFormat(t *testing.T) { + doc, err := os.ReadFile("testdata/table.doc") + if err != nil { + t.Fatal(err) + } + if res := NewDOCXParser().ParseWithResult(t.Context(), "renamed.docx", doc); res.Err == nil { + t.Fatal("DOCX parser silently accepted DOC") + } + xlsx := newTestXLSX(t, func(f *excelize.File) { mustSetCell(t, f, "Sheet1", "A1", "workbook") }) + p, err := NewXLSParser("") + if err != nil { + t.Fatal(err) + } + if res := p.ParseWithResult(t.Context(), "renamed.xls", xlsx); res.Err == nil { + t.Fatal("XLS parser silently accepted XLSX") + } +} + +func TestCorrectedTextOnlyParserSkipsMedia(t *testing.T) { + data, err := os.ReadFile("testdata/docx_table_image.docx") + if err != nil { + t.Fatal(err) + } + p, actual, err := GetTextOnlyParser(utility.FileTypeDOC, data) + if err != nil { + t.Fatal(err) + } + if actual != utility.FileTypeDOCX { + t.Fatalf("actual=%q", actual) + } + res := p.ParseWithResult(t.Context(), "renamed.doc", data) + if res.Err != nil { + t.Fatal(res.Err) + } + if len(res.JSON) == 0 { + t.Fatal("no extracted contents") + } + for _, item := range res.JSON { + if item.ToMap()["doc_type_kwd"] == "image" { + t.Fatal("text-only selection extracted an image") + } + } +} diff --git a/internal/parser/parser/parser_type.go b/internal/parser/parser/parser_type.go index 915744fbd4..17a0316e5d 100644 --- a/internal/parser/parser/parser_type.go +++ b/internal/parser/parser/parser_type.go @@ -22,17 +22,39 @@ import ( "ragflow/internal/utility" ) -// GetTextOnlyParser returns a parser for readable-text extraction: previews, -// attachments, agent uploads and CLI parsing. Embedded media is skipped even -// when the extraction is nested inside an ingestion run (email attachments). -// These paths also work without an injected run media cache. -func GetTextOnlyParser(fileType utility.FileType) (ParseResultProducer, error) { - return newParser(fileType, true) +// TryParser optionally identifies a format before full parsing by inspecting +// recognition structures. FileTypeOTHER means unknown: keep the requested type. +type TryParser interface { + TryParse(data []byte) utility.FileType } -// GetParser returns the ingestion parser for the given file type. -func GetParser(fileType utility.FileType) (ParseResultProducer, error) { - return newParser(fileType, false) +// GetTextOnlyParser selects a parser and effective file type for readable-text +// extraction: previews, attachments, agent uploads and CLI parsing. +// Embedded media is skipped even when extraction is nested inside an ingestion +// run (email attachments). +// These paths also work without an injected run media cache. +func GetTextOnlyParser(fileType utility.FileType, data []byte) (ParseResultProducer, utility.FileType, error) { + return selectParser(fileType, data, true) +} + +// GetParser selects the ingestion parser and effective file type from the +// requested type and contents. Unrecognized contents keep the requested type. +func GetParser(fileType utility.FileType, data []byte) (ParseResultProducer, utility.FileType, error) { + return selectParser(fileType, data, false) +} + +func selectParser(fileType utility.FileType, data []byte, textOnly bool) (ParseResultProducer, utility.FileType, error) { + p, err := newParser(fileType, textOnly) + if err != nil { + return nil, fileType, err + } + if probe, ok := p.(TryParser); ok { + if actual := probe.TryParse(data); actual != "" && actual != utility.FileTypeOTHER && actual != fileType { + p, err = newParser(actual, textOnly) + return p, actual, err + } + } + return p, fileType, nil } // newParser owns the format routes for both ingestion and text extraction. @@ -54,12 +76,7 @@ func newParser(fileType utility.FileType, textOnly bool) (ParseResultProducer, e p.textOnly = textOnly return p, nil case utility.FileTypeXLS: - p, err := NewXLSParser("") - if err != nil { - return nil, err - } - p.textOnly = textOnly - return p, nil + return NewXLSParser("") case utility.FileTypeCSV: return NewCSVParser(), nil case utility.FileTypeDOCX: diff --git a/internal/parser/parser/ppt_parser_cgo_test.go b/internal/parser/parser/ppt_parser_cgo_test.go index 10dd7585f4..a3e45e80af 100644 --- a/internal/parser/parser/ppt_parser_cgo_test.go +++ b/internal/parser/parser/ppt_parser_cgo_test.go @@ -132,24 +132,6 @@ func TestPPTXParser_ParseWithResult_EmptySlide_CGO(t *testing.T) { } } -// TestPPTParser_ParseWithResult_CGO verifies that PPTParser -// delegates correctly to PPTXParser{format:"ppt"} and, via the -// bidirectional magic-byte fallback, correctly opens OOXML content -// through the PPTX path. A .pptx payload uploaded as .ppt should -// succeed and report File["format"] == "pptx" (real container). -func TestPPTParser_ParseWithResult_CGO(t *testing.T) { - ctx := t.Context() - p := NewPPTParser() - data := buildPPTX(t, "Hello") - res := p.ParseWithResult(ctx, "test.ppt", data) - if res.Err != nil { - t.Fatalf("ParseWithResult: %v", res.Err) - } - if got := res.File["format"]; got != "pptx" { - t.Errorf("File[format] = %v, want %q (OOXML fallback ppt->pptx)", got, "pptx") - } -} - // TestPPTParser_TCADPFileType verifies that when a .ppt file is routed // through PPTParser with parse_method="tcadp", the underlying // PPTXParser must pass "PPT" as the file_type to TCADP, not "PPTX". diff --git a/internal/parser/parser/pptx_parser.go b/internal/parser/parser/pptx_parser.go index 3d97430e4e..e7f4b466a9 100644 --- a/internal/parser/parser/pptx_parser.go +++ b/internal/parser/parser/pptx_parser.go @@ -91,12 +91,6 @@ func (p *PPTXParser) ParseWithResult(ctx context.Context, filename string, data method := strings.ToLower(strings.TrimSpace(p.ParseMethod)) switch method { case "tcadp": - // TCADP file_type is intentionally derived from the parser's - // configured family (p.format == "pptx" → "PPTX", "ppt" → "PPT"), - // not from magic-byte sniffing. The pipeline routes purely on - // extension → parser family, and TCADP reconstructs by family. - // The OLE fallback below therefore only applies to the local - // office_oxide path. return parseWithTCADP(ctx, filename, data, strings.ToUpper(p.format), p.TCADPAPIServer, p.TCADPAPIKey, @@ -109,27 +103,7 @@ func (p *PPTXParser) ParseWithResult(ctx context.Context, filename string, data // meaningless for PPTX; treat as default path. } - // office_oxide's OpenFromBytes takes the container format as given - // and does no magic-byte detection, so a legacy .ppt uploaded with a - // .pptx extension would be parsed as ZIP/OOXML and fail. Sniff the - // real container and pass the matching format so mislabeled files - // still parse (mirrors the magic-byte correction office_oxide::Open - // performs on file paths). - // File["format"] in the success case reflects effFormat, i.e. the - // real container ("ppt" for OLE fallback, "pptx" for OOXML fallback), - // not just the extension. - effFormat := p.format - switch officeContainer(data) { - case "ole": - if effFormat == "pptx" { - effFormat = "ppt" - } - case "ooxml": - if effFormat == "ppt" { - effFormat = "pptx" - } - } - irJSON, _, plainText, _, err := pptxExtract(data, effFormat) + irJSON, _, plainText, _, err := pptxExtract(data, p.format) if err != nil { if errors.Is(err, deepdocoffice.ErrOfficeCGORequired) { return ParseResult{Err: fmt.Errorf("%w: %s", ErrOfficeCGORequired, filename)} @@ -148,7 +122,7 @@ func (p *PPTXParser) ParseWithResult(ctx context.Context, filename string, data return ParseResult{Err: fmt.Errorf("presentation ir-json: empty result")} } return ParseResult{ - File: map[string]any{"name": filename, "format": effFormat}, + File: map[string]any{"name": filename, "format": p.format}, JSON: MapsToItems(itemsFromPlainText(plainText)), } } @@ -179,7 +153,7 @@ func (p *PPTXParser) ParseWithResult(ctx context.Context, filename string, data } return ParseResult{ - File: map[string]any{"name": filename, "format": effFormat}, + File: map[string]any{"name": filename, "format": p.format}, JSON: MapsToItems(items), Warnings: sink.warnings(), } diff --git a/internal/parser/parser/xls_parser.go b/internal/parser/parser/xls_parser.go index 46f0421457..ecbccbca09 100644 --- a/internal/parser/parser/xls_parser.go +++ b/internal/parser/parser/xls_parser.go @@ -33,7 +33,6 @@ import ( var CompoundDocumentSignature = []byte{0xd0, 0xcf, 0x11, 0xe0, 0xa1, 0xb1, 0x1a, 0xe1} type XLSParser struct { - textOnly bool ParseMethod string TCADPAPIServer string TCADPAPIKey string @@ -92,27 +91,7 @@ func (p *XLSParser) ParseWithResult(ctx context.Context, filename string, data [ } } - var ( - items []map[string]any - warnings []string - sheetsCount int - err error - ) - if bytes.HasPrefix(data, CompoundDocumentSignature) { - // BIFF8 extraction reads cell text only; the drawing layer is not - // parsed, so no image can reach the media cache from this branch. - items, sheetsCount, err = parseBIFF8Bytes(ctx, data) - } else { - var sink *officeMediaSink - if !p.textOnly { - sink = newOfficeMediaSink(ctx, officeMediaFormatXLSX) - } - items, warnings, sheetsCount, err = parseXLSXBytes(data, sink) - if err == nil && sink != nil && sink.err != nil { - return ParseResult{Err: sink.err} - } - warnings = append(warnings, sink.warnings()...) - } + items, sheetsCount, err := parseBIFF8Bytes(ctx, data) if err := ctx.Err(); err != nil { return ParseResult{Err: err} } @@ -121,9 +100,8 @@ func (p *XLSParser) ParseWithResult(ctx context.Context, filename string, data [ } return ParseResult{ - File: map[string]any{"name": filename, "format": "xls", "sheets": sheetsCount}, - JSON: MapsToItems(items), - Warnings: warnings, + File: map[string]any{"name": filename, "format": "xls", "sheets": sheetsCount}, + JSON: MapsToItems(items), } } diff --git a/internal/parser/parser/xls_parser_test.go b/internal/parser/parser/xls_parser_test.go index 31c0b065b9..cd1afc1a27 100644 --- a/internal/parser/parser/xls_parser_test.go +++ b/internal/parser/parser/xls_parser_test.go @@ -205,50 +205,6 @@ func TestXLSParser_GenuineBIFF8SpreadsheetJSONOutput(t *testing.T) { } } -func TestXLSParser_SpreadsheetJSONOutput(t *testing.T) { - data := createTestExcelBytes(t) - p, err := NewXLSParser("excelize") - if err != nil { - t.Fatalf("NewXLSParser failed: %v", err) - } - - res := p.ParseWithResult(context.Background(), "test.xls", data) - if res.Err != nil { - t.Fatalf("ParseWithResult failed: %v", res.Err) - } - - // Default output format for XLSParser is "json" - if !res.Produced() { - t.Errorf("res.produced = %t, want true", res.Produced()) - } - if len(res.JSON) == 0 { - t.Fatalf("res.JSON should have structured table items") - } - item := res.JSON[0].ToMap() - if item["doc_type_kwd"] != "table" { - t.Errorf("table item doc_type_kwd = %v, want table", item["doc_type_kwd"]) - } - text := tableItemHTML(t, item) - if !strings.Contains(text, "Header1Header2") || - !strings.Contains(text, "Val1Val2") { - t.Errorf("table markup = %q, want captioned header and data row", text) - } - if item["doc_type_kwd"] != "table" { - t.Errorf("doc_type_kwd = %v, want 'table'", item["doc_type_kwd"]) - } - - resJSON := p.ParseWithResult(context.Background(), "test.xls", data) - if resJSON.Err != nil { - t.Fatalf("ParseWithResult failed: %v", resJSON.Err) - } - if !resJSON.Produced() { - t.Errorf("resJSON.produced = %t, want true", resJSON.Produced()) - } - if len(resJSON.JSON) == 0 { - t.Errorf("resJSON.JSON should not be empty") - } -} - func TestXLSXParser_JSONOutput(t *testing.T) { data := createTestExcelBytes(t) p, err := NewXLSXParser("excelize") @@ -272,7 +228,7 @@ func TestXLSXParser_JSONOutput(t *testing.T) { } } -func TestXLSParser_ImagesDoNotRunOCR(t *testing.T) { +func TestXLSXParser_ImagesDoNotRunOCR(t *testing.T) { var imageData bytes.Buffer if err := png.Encode(&imageData, image.NewRGBA(image.Rect(0, 0, 2, 2))); err != nil { t.Fatalf("encode fixture: %v", err) @@ -298,12 +254,12 @@ func TestXLSParser_ImagesDoNotRunOCR(t *testing.T) { return &deepdocpdf.MockDocAnalyzer{Healthy: true, OCRDetectErr: errors.New("xls OCR sentinel")}, true }) - p, err := NewXLSParser("excelize") + p, err := NewXLSXParser("excelize") if err != nil { - t.Fatalf("NewXLSParser: %v", err) + t.Fatalf("NewXLSXParser: %v", err) } ctx, cache := officeMediaTestContext() - result := p.ParseWithResult(ctx, "with-image.xls", workbook.Bytes()) + result := p.ParseWithResult(ctx, "with-image.xlsx", workbook.Bytes()) if result.Err != nil { t.Fatalf("ParseWithResult: %v", result.Err) } @@ -329,7 +285,7 @@ func TestXLSParser_ImagesDoNotRunOCR(t *testing.T) { } } -func TestXLSParser_CapsEmbeddedImageCount(t *testing.T) { +func TestXLSXParser_CapsEmbeddedImageCount(t *testing.T) { const limit = 256 t.Setenv("RAGFLOW_MEDIA_MAX_IMAGES_PER_RUN", "256") f := excelize.NewFile() @@ -353,12 +309,12 @@ func TestXLSParser_CapsEmbeddedImageCount(t *testing.T) { deepdoctype.SetNativeDocAnalyzerFactory(func() (deepdoctype.DocAnalyzer, bool) { return &deepdocpdf.MockDocAnalyzer{Healthy: true}, true }) - p, err := NewXLSParser("excelize") + p, err := NewXLSXParser("excelize") if err != nil { - t.Fatalf("NewXLSParser: %v", err) + t.Fatalf("NewXLSXParser: %v", err) } ctx, _ := officeMediaTestContext() - result := p.ParseWithResult(ctx, "many-images.xls", workbook.Bytes()) + result := p.ParseWithResult(ctx, "many-images.xlsx", workbook.Bytes()) if result.Err != nil { t.Fatalf("ParseWithResult: %v", result.Err) } diff --git a/internal/parser/parser/xlsx_media_test.go b/internal/parser/parser/xlsx_media_test.go index a66578094c..604d334dab 100644 --- a/internal/parser/parser/xlsx_media_test.go +++ b/internal/parser/parser/xlsx_media_test.go @@ -56,7 +56,7 @@ func TestXLSXTextParserSkipsImagesWithoutCache(t *testing.T) { t.Fatalf("AddPictureFromBytes: %v", err) } }) - p, err := GetTextOnlyParser(utility.FileTypeXLSX) + p, _, err := GetTextOnlyParser(utility.FileTypeXLSX, nil) if err != nil { t.Fatal(err) } diff --git a/internal/service/document/document_artifact.go b/internal/service/document/document_artifact.go index 097406b7e5..5c5afbf37a 100644 --- a/internal/service/document/document_artifact.go +++ b/internal/service/document/document_artifact.go @@ -319,10 +319,13 @@ func (s *DocumentService) GetDocumentPreview(ctx context.Context, docID string) // document via the office_oxide-backed DOCParser. The parser only emits JSON // payloads, so the preview text is flattened from the JSON items. func extractDOCPreviewText(ctx context.Context, filename string, data []byte) (string, error) { - p, err := parser.GetTextOnlyParser(utility.FileTypeDOC) + p, actual, err := parser.GetTextOnlyParser(utility.FileTypeDOC, data) if err != nil { return "", err } + if actual != utility.FileTypeDOC { + common.Warn(fmt.Sprintf("file %q: requested format doc; detected %s; continuing with the %s parser", filename, actual, actual)) + } res := p.ParseWithResult(ctx, filename, data) if res.Err != nil { return "", res.Err diff --git a/internal/service/document/document_preview_format_test.go b/internal/service/document/document_preview_format_test.go new file mode 100644 index 0000000000..31029c39f6 --- /dev/null +++ b/internal/service/document/document_preview_format_test.go @@ -0,0 +1,23 @@ +//go:build cgo + +package document + +import ( + "os" + "strings" + "testing" +) + +func TestDOCPreviewCorrectsDOCXContent(t *testing.T) { + data, err := os.ReadFile("../../parser/parser/testdata/docx_table_image.docx") + if err != nil { + t.Fatal(err) + } + text, err := extractDOCPreviewText(t.Context(), "renamed.doc", data) + if err != nil { + t.Fatal(err) + } + if strings.TrimSpace(text) == "" || text == string(data) { + t.Fatal("preview did not extract corrected DOCX content") + } +} diff --git a/internal/service/file/file_content.go b/internal/service/file/file_content.go index d53da9f5c2..3574ec9c3b 100644 --- a/internal/service/file/file_content.go +++ b/internal/service/file/file_content.go @@ -5,6 +5,7 @@ import ( "encoding/base64" "encoding/json" "fmt" + "ragflow/internal/common" "ragflow/internal/dao" "ragflow/internal/entity" "ragflow/internal/parser/parser" @@ -217,10 +218,13 @@ func parseAgentUploadContent(ctx context.Context, filename string, data []byte, content := string(data) fileType := utility.GetFileType(filename) if fileType != utility.FileTypeOTHER { - fp, err := parser.GetTextOnlyParser(fileType) + fp, actual, err := parser.GetTextOnlyParser(fileType, data) if err != nil { return "", err } + if actual != fileType { + common.Warn(fmt.Sprintf("file %q: requested format %s; detected %s; continuing with the %s parser", filename, fileType, actual, actual)) + } if configurable, ok := fp.(interface{ ConfigureFromSetup(map[string]any) }); ok { configurable.ConfigureFromSetup(map[string]any{"layout_recognize": layoutRecognize}) } @@ -269,10 +273,13 @@ func parseFileContent(ctx context.Context, filename string, data []byte) string if fileType == utility.FileTypeOTHER { return string(data) } - fp, err := parser.GetTextOnlyParser(fileType) + fp, actual, err := parser.GetTextOnlyParser(fileType, data) if err != nil { return string(data) } + if actual != fileType { + common.Warn(fmt.Sprintf("file %q: requested format %s; detected %s; continuing with the %s parser", filename, fileType, actual, actual)) + } res := fp.ParseWithResult(ctx, filename, data) if res.Err != nil { return string(data) diff --git a/internal/service/file/file_content_cgo_test.go b/internal/service/file/file_content_cgo_test.go index 5687ed36cd..3bf99beb6e 100644 --- a/internal/service/file/file_content_cgo_test.go +++ b/internal/service/file/file_content_cgo_test.go @@ -82,6 +82,25 @@ func TestParseFileContent_DOCXReadsJSONItems(t *testing.T) { } } +func TestMisnamedOfficeContentExtraction(t *testing.T) { + data := minimalDOCX(t, "Corrected document content") + for _, name := range []string{"renamed.doc", "renamed.wps", "renamed.xls"} { + t.Run(name, func(t *testing.T) { + content := parseFileContent(t.Context(), name, data) + if !strings.Contains(content, "Corrected document content") || content == string(data) { + t.Fatal("file extraction did not use the corrected parser") + } + content, err := parseAgentUploadContent(t.Context(), name, data, "") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(content, "Corrected document content") { + t.Fatal("agent extraction did not use the corrected parser") + } + }) + } +} + // TestOfficeContentExtractionWithoutMediaCache pins that previews, uploads and // other non-ingestion callers use the text-only Office parsers: an // image-bearing .docx must yield readable text instead of failing because no