diff --git a/AGENTS.md b/AGENTS.md index 5ff5986e8..de871673b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -21,7 +21,9 @@ bytes ─▶ magic/open_strategy ─▶ DecodedFile ─▶ Document ─▶ Eleme 1. **Detect** — `internal/magic.cpp` sniffs the head of the file; `internal/open_strategy.cpp` picks a `FileType` + `DecoderEngine` and builds the matching `abstract::DecodedFile`. `odr::mimetype` composes the two, so a - zip is named by what is inside it. + zip is named by what is inside it. Only bytes can *claim* a file; a name adds + a candidate the bytes already allow, which is the sole way in for a + signature-less format (`file_type_by_name`, markdown). 2. **Decode** — a document file yields an `abstract::Document`. 3. **Element tree** — a `Document` exposes a root `ElementIdentifier` plus an `abstract::ElementAdapter`. Public value-semantics handles (`Element`, `Slide`, diff --git a/CHANGELOG.md b/CHANGELOG.md index e2def7c47..c5ee55768 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,11 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- A `.md` opened by path decodes as markdown rather than as plain text, and + `list_file_types` offers it. The extension only adds a candidate the bytes + already allow; `File::from_memory` has no name, so it still needs + `FileType::markdown`. Closes #760. + - **Breaking**: a document decrypted from a password-protected package is no longer savable — every `save` overload throws `UnsupportedOperation`, where it used to write the content out unprotected. Towards #64. diff --git a/src/odr/internal/markdown/AGENTS.md b/src/odr/internal/markdown/AGENTS.md index e42451162..0275f835b 100644 --- a/src/odr/internal/markdown/AGENTS.md +++ b/src/odr/internal/markdown/AGENTS.md @@ -16,15 +16,19 @@ Decoding to a `TextRoot` is the whole argument for a decoder rather than a markdown→HTML renderer next to `html/text_file.cpp`: the latter would produce html only, with no element api and nothing for JNI/embind/pybind/ObjC. -## Nothing detects it, and decoding never rejects +## The name detects it, not the content `detect_by_content` is **false**. Markdown has no signature, and a content probe for it is a probe for "prose with occasional punctuation" — every plain text file with a `#` comment or an `*` bullet in it. Sniffing would steal `text_file` -matches and be confidently wrong. The only way in is -`DecodedFile(file, FileType::markdown)`; callers route on the file name, which -is what they already have. **A `.md` still opens as `text_file` by default**, -and that is correct for a format that is by construction valid plain text. +matches and be confidently wrong. + +So the file name does it instead: `open_strategy::file_type_by_name` reads the +extension off `File::disk_path` and offers markdown once the bytes have already +decoded as text, ahead of the csv/json/xml probes. A name only ever *adds* a +candidate — a `.md` holding a zip is still a zip — and a file with no name on +disk has no hint, so `File::from_memory` still needs +`DecodedFile(file, FileType::markdown)`. `NoMarkdownFile` exists only for the `as_markdown_file()` cast: every other format's `No*File` is also what detection throws, and nothing rejects here — diff --git a/src/odr/internal/markdown/PLAN.md b/src/odr/internal/markdown/PLAN.md index 32a1c54ea..e6412aae4 100644 --- a/src/odr/internal/markdown/PLAN.md +++ b/src/odr/internal/markdown/PLAN.md @@ -78,20 +78,14 @@ rule the csv plan sets for the sheet path, and for the same reason: `Text::content()` is UTF-8 to every binding, so passing legacy bytes through and letting a browser sort it out is not available to us. -**Detection is by caller, not by content.** `open_strategy` is entirely -content-driven — magic plus speculative probes — and has no extension path -anywhere. Markdown has no signature, and a content probe for it is a probe for -"prose with occasional punctuation", which is every plain text file with a `#` -comment or an `*` bullet in it. Sniffing would steal `text_file` matches and be -confidently wrong. So: `detect_by_content` stays **false**, markdown never joins -the speculative chain in `list_file_types` (`open_strategy.cpp:272`), and the -only way in is `DecodedFile(file, FileType::markdown)` (`file.hpp:341`) via a -new branch in `open_file` next to the text/csv/json ones -(`open_strategy.cpp:146`). Callers route on the filename, which is what they -already have. - -The consequence to accept: `.md` still opens as `text_file` by default. That is -correct behaviour for a format that is, by construction, valid plain text. +**Detection is by name, not by content.** Markdown has no signature, and a +content probe for it is a probe for "prose with occasional punctuation", which +is every plain text file with a `#` comment or an `*` bullet in it. Sniffing +would steal `text_file` matches and be confidently wrong, so `detect_by_content` +stays **false** and markdown never joins the speculative chain. The extension +offers it instead, once the bytes have decoded as text — see +[`AGENTS.md`](AGENTS.md) and #760. `File::from_memory` has no name, so it still +needs `DecodedFile(file, FileType::markdown)`. **There is no `NoMarkdownFile`.** Every other format's exception exists because detection rejects. Nothing rejects here: md4c is total — any UTF-8 byte diff --git a/src/odr/internal/open_strategy.cpp b/src/odr/internal/open_strategy.cpp index 55c8559ba..8a91b98a2 100644 --- a/src/odr/internal/open_strategy.cpp +++ b/src/odr/internal/open_strategy.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -29,6 +30,7 @@ #include #include +#include namespace odr::internal { @@ -50,6 +52,19 @@ template auto priority_comparator(const std::vector &priority) { }; } +/// The type @p file's name claims that no content probe can produce +/// (`detect_by_content == false`), or `unknown`. A name only ever adds a +/// candidate the bytes already allow — it never claims them. +FileType file_type_by_name(const abstract::File &file) { + const std::optional path = file.disk_path(); + if (!path.has_value()) { + return FileType::unknown; + } + const FileType type = file_type_by_file_extension(path->extension()); + return capabilities_by_file_type(type).detect_by_content ? FileType::unknown + : type; +} + /// Whether @p file is the ooxml that was asked for. An encrypted one names no /// inner type until it is decrypted, so it answers for whichever was asked. bool is_the_requested_ooxml(const ooxml::OfficeOpenXmlFile &file, @@ -418,6 +433,16 @@ open_strategy::list_file_types(const std::shared_ptr &file, } catch (...) { ODR_VERBOSE(logger, "failed to open as xml"); } + + // last, so it outranks the probes: markdown has no signature and every + // text file is valid markdown, leaving the name the only thing that can + // say so + if (const FileType by_name = file_type_by_name(*file); + by_name != FileType::unknown) { + ODR_VERBOSE(logger, + "name says " << file_type_to_string(by_name) << ", adding"); + result.push_back(by_name); + } } catch (...) { ODR_VERBOSE(logger, "failed to open as text"); } @@ -525,6 +550,19 @@ open_strategy::open_file(const std::shared_ptr &file, auto text = std::make_shared(file); + // before the probes: a markdown file parsing as csv is still markdown, + // and the name is the only thing that can say so + if (const FileType by_name = file_type_by_name(*file); + by_name != FileType::unknown) { + ODR_VERBOSE(logger, + "name says " << file_type_to_string(by_name) << ", try it"); + try { + return open_file_as(file, by_name, logger); + } catch (...) { + ODR_VERBOSE(logger, "failed to open as what the name says"); + } + } + try { ODR_VERBOSE(logger, "try open as csv"); return std::make_unique(text); diff --git a/test/src/odr_test.cpp b/test/src/odr_test.cpp index 65fc256f2..726590dba 100644 --- a/test/src/odr_test.cpp +++ b/test/src/odr_test.cpp @@ -4,6 +4,8 @@ #include #include +#include + #include #include @@ -67,6 +69,33 @@ TEST(odr, types_wpd) { EXPECT_EQ(mimetype(path, logger), "application/vnd.wordperfect"); } +/// Markdown has no signature, so only the name can offer it. +TEST(odr, types_md) { + const auto logger = Logger::create_stdio("odr-test", LogLevel::verbose); + + const auto path = TestData::test_file_path("odr-public/md/feature-matrix.md"); + const auto types = list_file_types(path, logger); + ASSERT_FALSE(types.empty()); + EXPECT_EQ(types.front(), FileType::text_file); + EXPECT_EQ(types.back(), FileType::markdown); + + // the name only adds a candidate; opening by path takes it + EXPECT_EQ(open(path, logger).file_type(), FileType::markdown); + EXPECT_EQ(mimetype(path, logger), "text/markdown"); +} + +/// A name claims nothing on its own — the bytes still decide. +TEST(odr, a_misnamed_file_is_what_its_bytes_are) { + const auto logger = Logger::create_stdio("odr-test", LogLevel::verbose); + + const auto path = TestData::test_file_path("odr-public/odt/about.odt"); + EXPECT_EQ(open(path, logger).file_type(), FileType::opendocument_text); + + // no name at all, so no hint: the same bytes come back as plain text + const DecodedFile from_memory(File::from_memory("# heading\n"), logger); + EXPECT_EQ(from_memory.file_type(), FileType::text_file); +} + TEST(FileTypeTable, covers_every_file_type_exactly_once) { const std::vector expected = every_file_type(); const std::vector actual = all_file_types(); @@ -264,11 +293,15 @@ TEST(FileTypeCapabilities, declaration_matches_the_engines) { continue; } - // whatever detection sees, the table has to admit to + // whatever detection sees, the table has to admit to — from the bytes, + // or from the name for a type that has no signature to find const std::vector detected = list_file_types(test_file.absolute_path, logger); if (std::ranges::find(detected, type) != std::ranges::end(detected)) { - EXPECT_TRUE(declared.detect_by_content) << test_file.short_path; + EXPECT_TRUE(declared.detect_by_content || + file_type_by_file_extension( + Path(test_file.absolute_path).extension()) == type) + << test_file.short_path; } // an encrypted OOXML package decodes as its own file type