diff --git a/.gitattributes b/.gitattributes index 9bbc00b..8e8743e 100644 --- a/.gitattributes +++ b/.gitattributes @@ -24,3 +24,8 @@ # --strict` fails when the two disagree, and a build is meant to be the install # the lock describes. /composer.lock export-ignore + +# The patch lock is this repository's install state — the patch, its checksum and +# the depth it applied at. A consumer's install never reads it, and it records +# one machine's install rather than the package's. +/patches.lock.json export-ignore diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index a32ae18..5262cb3 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -231,8 +231,12 @@ jobs: } ' + # The schema gate is not run here. The runner image ships a .NET SDK, so + # the test would not skip itself and would fail this job over a document + # that is nobody's business but the gate's — and the analysis job below + # needs this one to have passed. The gate has a job of its own. - name: Test - run: vendor/bin/pest + run: vendor/bin/pest --exclude-testsuite=ooxml # A coverage report on its own says little; this fails if the reported # figure is a placeholder, or if the code stops being covered at all. @@ -250,6 +254,7 @@ jobs: if: matrix.php == '8.5' run: | vendor/bin/pest --coverage --min=91 \ + --exclude-testsuite=ooxml \ --coverage-clover=build/coverage/clover.xml \ --coverage-html=build/coverage/html \ --log-junit build/coverage/junit.xml @@ -262,6 +267,43 @@ jobs: path: build/coverage retention-days: 7 + # The documents this library writes, checked against the schema. + # + # A job of its own because the gate needs the .NET SDK at a version this + # repository chooses. The runner image ships one, so the test would not skip + # itself there either; the jobs above leave the suite out rather than run the + # gate against whatever the image happens to carry, and this one installs the + # version the validator is built for. + ooxml: + name: ooxml + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + + - uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # v2 + with: + php-version: '8.5' + coverage: none + tools: composer:v2 + + - name: Install + run: composer install --prefer-dist --no-interaction --no-progress + + # The version the validator is built for. A machine with a newer runtime + # runs it unchanged, so this is the floor and not a ceiling. + - uses: actions/setup-dotnet@67a3573c9a986a3f9c594539f4ab511d57bb3ce9 # v4 + with: + dotnet-version: '8.0' + + # This one suite and not the whole of it: the other jobs leave it out, and + # this is where the gate runs. + # + # `--fail-on-skipped` because a skipped test is a green tick for a check that + # never ran. The job installs the SDK, so the test cannot skip here — and if + # that ever stops being true, the job says so instead of passing. + - name: The documents are valid Office Open XML + run: vendor/bin/pest --testsuite=ooxml --fail-on-skipped + # Static analysis on SonarCloud. # # Its own job rather than a step in the test matrix, so that a finding and a @@ -316,9 +358,14 @@ jobs: # Both reports the analysis reads, produced in one run rather than the # second one being repeated. The HTML report is skipped; Sonar wants the # machine-readable one. + # + # The schema gate is left out for the reason the test job gives: it has a + # job of its own, and the analysis is about the code rather than about the + # documents it writes. - name: Coverage and test results for the analysis run: | vendor/bin/pest --coverage --min=91 \ + --exclude-testsuite=ooxml \ --coverage-clover=build/coverage/clover.xml \ --log-junit build/coverage/junit.xml @@ -407,7 +454,7 @@ jobs: run: git --no-pager diff --stat composer.lock - name: The suite still passes - run: vendor/bin/pest + run: vendor/bin/pest --exclude-testsuite=ooxml # A runtime dependency that stops supporting the version the library claims # is the failure worth catching, and the minimum job cannot see it while it diff --git a/.gitignore b/.gitignore index 602868f..f7f1d05 100644 --- a/.gitignore +++ b/.gitignore @@ -15,6 +15,11 @@ # The documents and previews examples/build.php writes. /examples/out/ +# What the OOXML validator's build produces. The validator is a .NET program and +# its compiler writes beside the sources. +/tools/ooxml-validator/bin/ +/tools/ooxml-validator/obj/ + # Scratch space for the tests and the checking scripts. /tmp/ @@ -44,3 +49,6 @@ # Parallel agent worktrees .worktrees/ + +# The patch lock is this repository's install state, not the package's. +/patches.lock.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 74332a8..18beb94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,122 @@ All notable changes to this project are documented here. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and the project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.3.1] - 2026-10-31 + +### Added + +- **The documents this library writes are checked against the schema.** A `.docx` is + Office Open XML, and a document that breaks the schema is a document that is not + what it claims to be — Word opens it because Word is lenient where the schema is + not, and nothing else in the suite could see it. The check is Microsoft's own + validator, the one the Open XML SDK Productivity Tool uses, and it lives in + `tools/ooxml-validator`. + + It is run over the examples — the whole corpus of documents this library writes, + built by `examples/build.php` — rather than over documents the test writes + itself, because a gate over a selection is a gate over the selection. + + It is a gate and not a report: the test fails on a document the validator + rejects, and a test that asks the validator about a document with an error in it + proves that it can fail. The test skips itself where the .NET SDK is not, and + says what is missing; a job of its own installs the SDK, so the gate cannot be + skipped there, and the jobs that run the rest of the suite leave it out. + + **The gate lands green.** Over the twenty-nine documents the build writes, + Microsoft's validator reported a thousand and two errors. With the fixes upstream + in PHPWord's writer it reported a hundred and fifty-one: eighty-eight belonged to + PHPWord, forty-four to the three committed templates, and nineteen to this + library's own. **With all five upstream pull requests, the templates written by + the fixed library, and this library's own fix, it reports zero.** + + The gate comes first, so that the fixes are proved to have landed rather than + believed to have. + +- **`examples/build.php --no-previews` builds the documents and leaves the pictures + alone.** Drawing the previews takes longer than everything else the build does, + and the gate that reads the documents wants the documents. + +- **The project that installs this one has to allow + [`cweagans/composer-patches`][composer-patches].** The fixes are in five pull + requests upstream — + [#2964](https://github.com/PHPOffice/PHPWord/pull/2964), + [#2965](https://github.com/PHPOffice/PHPWord/pull/2965), + [#2966](https://github.com/PHPOffice/PHPWord/pull/2966), + [#2968](https://github.com/PHPOffice/PHPWord/pull/2968) and + [#2969](https://github.com/PHPOffice/PHPWord/pull/2969), all against + [#2963](https://github.com/PHPOffice/PHPWord/issues/2963) — and until they are + released they reach this library as a patch applied to the installed package. + The plugin applies it, and Composer runs it only when the project allows it. + Without that line the documents are invalid Office Open XML again, silently. + +- **A template is bounded before a part of it is read.** A template is somebody else's + file, and a zip says how its contents are laid out and nothing about how much room + they will take up: forty kilobytes of a part is forty megabytes of paragraph, and the + entry count is whatever the writer felt like. The write path read a template without + any of the limits the read path has, so a template of sixty-five kilobytes could + hold a part that inflates to sixty-four megabytes. + + The number of parts and the size of any one of them are bounded now, with the read + path's numbers — 4096 parts, 256 MiB each — and a template that is over either is + refused with a `MarkdownWord\Exception\UnreadableTemplate` that says how far over + it is. A caller who wants them tighter says so, with + `MarkdownWord\Template\TemplateOptions`. + +### Changed + +- **The examples build the way the analyser wants them to.** The build script named + the directory the Markdown sources live in twenty times, and loaded the autoloader + with `require` rather than `require_once`. Neither changed what the build writes. + +### Fixed + +- **A link's run properties are written in the order the schema names them.** The + properties of a hyperlink's run were written by hand — PHPWord's `Link` element + holds one plain string and cannot carry a label with emphasis in it — and they + were written in the order they came to mind: the weight, the slant, the strike, + the underline, the colour, the size and the typeface. + + `w:rPr` is a sequence in ECMA-376's `wml.xsd` and not a set, so a reader walks + its children in the order they are written and stops at the first one it did not + expect. The typeface comes before the weight, and the underline comes after the + size. Word opens a document that gets this wrong, because Word is lenient where + the schema is not; a reader that walks the sequence does not. + + Microsoft's own validator reported `The element has unexpected child element + 'w:color'` nineteen times across the example documents, every one of them a + link whose label was styled. The order is the schema's now: `w:rFonts`, `w:b`, + `w:i`, `w:strike`, `w:color`, `w:sz`, `w:u`. + +- **The PHPWord that writes the documents is patched.** PHPWord 1.4.0 writes the + children of `w:rPr`, of `w:pPr`, of `w:tblPr`, of `w:sectPr` and of `w:style` + in an order of its own, writes a page size in twips as a fraction, and writes a + few elements the format does not allow where it puts them. A document it writes + is therefore not valid OOXML, and every document this library writes is a + document it writes. + + The fixes are in five pull requests upstream, and until they are released the + library carries them as a patch — the same way it has carried PHPPresentation's + fixes until now. The patch touches eighteen files of the Word2007 writer and + nothing else: it is a patch against 1.4.0 and not against the branch, so it + carries the fixes and not the rest of what upstream has done since. + +- **The three templates are written by the fixed library.** `invoice.dotx`, + `meridian-qbr.dotx` and `plain-letterhead.dotx` were written by PHPWord before + the fixes, so they carried four of its defects inside them — a page size in + twips with a fraction, a style whose properties were in the wrong order, the + settings in the wrong order and an inline picture in a header. Every document + rendered from a template inherited them, which is why forty-four of the errors + the gate found were in the templates and not in the code that writes them. + + They are written by the fixed library now, and they score 0 errors each. **The + header logo of `meridian-qbr.dotx` and `plain-letterhead.dotx` is the picture the + examples carry today** — the old templates carried an earlier one, left behind + when the asset itself changed. + +- **The message that says a document has too many parts reports zero parts.** The + archive was closed before the number was read, and closing an archive resets the + count to zero, so the message said `0` whatever the document held. + ## [0.3.0] - 2026-10-30 ### Added @@ -1176,6 +1292,8 @@ on it: `README.md` has the full list, and `Reverse\Options` is where each one is configurable. +[0.3.1]: https://github.com/markdown-office/markdown-word/releases/tag/v0.3.1 +[composer-patches]: https://github.com/cweagans/composer-patches [0.2.2]: https://github.com/markdown-office/markdown-word/releases/tag/v0.2.2 [0.3.0]: https://github.com/markdown-office/markdown-word/releases/tag/v0.3.0 [0.2.1]: https://github.com/markdown-office/markdown-word/releases/tag/v0.2.1 diff --git a/README.md b/README.md index 3fdd8f1..398af8a 100644 --- a/README.md +++ b/README.md @@ -91,6 +91,27 @@ install of this package puts no command in `vendor/bin`. The command line is a separate program — [`mdoffice`][mdoffice] — whose commands and options are in the [documentation][command-line]. +**The project that installs this one has to allow [`cweagans/composer-patches`][composer-patches].** +The documents this library writes are Word and Open XML, and the released PHPWord writes +a few of them in an order the schema does not allow — the properties of a run, of a +paragraph, of a table, of a section and of a style, and a few elements that belong +somewhere else. The fixes are in pull requests upstream, and until they are released +they reach this library as a patch applied to the installed package. The plugin applies +that patch, and Composer runs it only when the project allows it: + +```json +{ + "config": { + "allow-plugins": { + "cweagans/composer-patches": true + } + } +} +``` + +Without that line the install stops with Composer's own message naming +`cweagans/composer-patches`. The documents are valid Office Open XML only with it. + See [Requirements](#requirements) for what PHP and which extensions it needs. Where to get help: [the issue tracker][issues]. @@ -138,6 +159,7 @@ composer test:unit # the unit tests alone composer test:spec # just the conformance suite composer test:roundtrip # just the round-trip suite composer test:readme # just the examples on this page +composer test:ooxml # just the schema gate over the documents composer test:coverage # with a report in build/coverage php smoke.php # does the library work on this version of PHP? php stress.php # malformed input across every configuration @@ -161,6 +183,7 @@ Tests are written with [Pest][pest] 5, against PHPUnit 13. | Workflow | What it does | | --- | --- | | `tests` | `composer validate --strict`, `composer audit`, the README examples, then the suite on PHP 8.4 and 8.5 with a coverage report | +| `ooxml` (a job in `tests`) | the documents this library writes, checked against the OOXML schema by Microsoft's own validator, which needs the .NET SDK | | `sonarcloud` (a job in `tests`) | static analysis on [SonarCloud][sonarcloud], failing the build when the quality gate is red | PHP 8.2 is in the matrix too, but the test suite cannot run there: **Pest 5 @@ -989,6 +1012,35 @@ Normalising the type on the way in answers both at once, so the template on disk never opened for writing — the copy the renderer works from is ours, and it is removed when the renderer goes. +### What a template may hold + +A template is somebody else's file, and a zip says how its contents are laid out and +nothing about how much room they will take up: forty kilobytes of a part is forty +megabytes of paragraph, and the entry count is whatever the writer felt like. So the +template is bounded before a part of it is read — the number of parts it may have, and +the size of any one of them once it is inflated. + +The bounds are the read path's, so a document is as welcome as a template: + +| | the bound | +| --- | --- | +| the number of parts in the archive | 4096 | +| the size of one part, inflated | 256 MiB | + +A template that is over either is refused with an +`MarkdownWord\Exception\UnreadableTemplate`, and the message says how far over it +is. A caller who wants them tighter says so: + +```php +use MarkdownWord\Template\MarkdownTemplate; +use MarkdownWord\Template\TemplateOptions; + +$template = new MarkdownTemplate( + 'report-template.dotx', + limits: (new TemplateOptions())->withMaxPartBytes(8 * 1024 * 1024), +); +``` + ### The template contract A template says two things, and both are Word constructs: @@ -1959,6 +2011,7 @@ LGPL-3.0. [commonmark]: https://github.com/thephpleague/commonmark [command-line]: https://markdown-office.github.io/command-line/basic/ [issues]: https://github.com/markdown-office/markdown-word/issues +[composer-patches]: https://github.com/cweagans/composer-patches [mdoffice]: https://github.com/markdown-office/mdoffice [sonarcloud]: https://sonarcloud.io/summary/new_code?id=markdown-office_markdown-word [skill]: https://markdown-office.github.io/skills/ diff --git a/composer.json b/composer.json index 937e2b0..b1e99bc 100644 --- a/composer.json +++ b/composer.json @@ -22,13 +22,14 @@ ], "require": { "php": "^8.2", + "cweagans/composer-patches": "^2.0", "ext-dom": "*", "ext-gd": "*", "ext-mbstring": "*", "ext-zip": "*", "league/commonmark": "^2.10", "markdown-office/commonmark-generic-directives": "^0.3.0", - "phpoffice/phpword": "^1.4", + "phpoffice/phpword": "~1.4.0", "symfony/yaml": "^7.4 || ^8.1" }, "require-dev": { @@ -37,6 +38,17 @@ "suggest": { "ext-imagick": "Embeds SVG as a vector. Word holds an SVG only beside a raster of it, so something has to draw the raster; without this an SVG raises instead of going in." }, + "extra": { + "patches": { + "phpoffice/phpword": [ + { + "description": "PHPWord 1.4.0 writes the children of `w:rPr`, of `w:pPr`, of `w:tblPr`, of `w:sectPr` and of `w:style` in an order of its own, and writes a few elements the format does not allow there. The patch puts them where ECMA-376 names them. The version is held to the 1.4 line rather than the 1 series: the patch is against 1.4.0, and a later release of the line would carry a different writer.", + "url": "https://raw.githubusercontent.com/markdown-office/PHPWord/62f68049d287e5a52575f2ae694c2e76f4a2002a/patches/phpoffice-phpword-1.4.0-ooxml.patch", + "sha256": "193163898e7690b6eaf51086d415809616b7e25ec4090b58ce7966f66ae1203a" + } + ] + } + }, "autoload": { "psr-4": { "MarkdownWord\\": "src/" @@ -50,6 +62,7 @@ "config": { "sort-packages": true, "allow-plugins": { + "cweagans/composer-patches": true, "pestphp/pest-plugin": true } }, @@ -59,6 +72,7 @@ "test:spec": "pest --testsuite=conformance", "test:roundtrip": "pest --testsuite=roundtrip", "test:readme": "pest --testsuite=readme", + "test:ooxml": "pest --testsuite=ooxml", "test:coverage": "pest --coverage --min=91 --coverage-clover=build/coverage/clover.xml --coverage-html=build/coverage/html", "check": [ "@composer validate --strict", diff --git a/composer.lock b/composer.lock index b9ee406..4c64a7c 100644 --- a/composer.lock +++ b/composer.lock @@ -4,8 +4,131 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "fab2692dc0051d7de26e79611416a476", + "content-hash": "821ee431bf54fcf6880c642662896637", "packages": [ + { + "name": "cweagans/composer-configurable-plugin", + "version": "2.0.0", + "source": { + "type": "git", + "url": "https://github.com/cweagans/composer-configurable-plugin.git", + "reference": "15433906511a108a1806710e988629fd24b89974" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/cweagans/composer-configurable-plugin/zipball/15433906511a108a1806710e988629fd24b89974", + "reference": "15433906511a108a1806710e988629fd24b89974", + "shasum": "" + }, + "require": { + "php": ">=8.0.0" + }, + "require-dev": { + "codeception/codeception": "~4.0", + "codeception/module-asserts": "^2.0", + "composer/composer": "~2.0", + "php-coveralls/php-coveralls": "~2.0", + "php-parallel-lint/php-parallel-lint": "^1.0.0", + "phpro/grumphp": "^1.8.0", + "sebastian/phpcpd": "^6.0", + "squizlabs/php_codesniffer": "^3.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "cweagans\\Composer\\": "src" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "BSD-3-Clause" + ], + "authors": [ + { + "name": "Cameron Eagans", + "email": "me@cweagans.net" + } + ], + "description": "Provides a lightweight configuration system for Composer plugins.", + "support": { + "issues": "https://github.com/cweagans/composer-configurable-plugin/issues", + "source": "https://github.com/cweagans/composer-configurable-plugin/tree/2.0.0" + }, + "funding": [ + { + "url": "https://github.com/cweagans", + "type": "github" + } + ], + "time": "2023-02-12T04:58:58+00:00" + }, + { + "name": "cweagans/composer-patches", + "version": "2.0.0", + "source": { + "type": "git", + "url": "https://github.com/cweagans/composer-patches.git", + "reference": "bfa6018a5f864653d9ed899b902ea72f858a2cf7" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/cweagans/composer-patches/zipball/bfa6018a5f864653d9ed899b902ea72f858a2cf7", + "reference": "bfa6018a5f864653d9ed899b902ea72f858a2cf7", + "shasum": "" + }, + "require": { + "composer-plugin-api": "^2.0", + "cweagans/composer-configurable-plugin": "^2.0", + "ext-json": "*", + "php": ">=8.0.0" + }, + "require-dev": { + "codeception/codeception": "~4.0", + "codeception/module-asserts": "^2.0", + "codeception/module-cli": "^2.0", + "codeception/module-filesystem": "^2.0", + "composer/composer": "~2.0", + "php-coveralls/php-coveralls": "~2.0", + "php-parallel-lint/php-parallel-lint": "^1.0.0", + "phpro/grumphp": "^1.8.0", + "sebastian/phpcpd": "^6.0", + "squizlabs/php_codesniffer": "^4.0" + }, + "type": "composer-plugin", + "extra": { + "_": "The following two lines ensure that composer-patches is loaded as early as possible.", + "class": "cweagans\\Composer\\Plugin\\Patches", + "plugin-modifies-downloads": true, + "plugin-modifies-install-path": true + }, + "autoload": { + "psr-4": { + "cweagans\\Composer\\": "src" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "BSD-3-Clause" + ], + "authors": [ + { + "name": "Cameron Eagans", + "email": "me@cweagans.net" + } + ], + "description": "Provides a way to patch Composer packages.", + "support": { + "issues": "https://github.com/cweagans/composer-patches/issues", + "source": "https://github.com/cweagans/composer-patches/tree/2.0.0" + }, + "funding": [ + { + "url": "https://github.com/cweagans", + "type": "github" + } + ], + "time": "2025-10-30T23:44:22+00:00" + }, { "name": "dflydev/dot-access-data", "version": "v3.0.3", diff --git a/examples/README.md b/examples/README.md index 51222b9..b204b44 100644 --- a/examples/README.md +++ b/examples/README.md @@ -20,6 +20,10 @@ Previews need LibreOffice; without it the documents still build and the script s so rather than failing. Where a stem has more than one format behind it, the format goes into the preview's name as well. +`--no-previews` builds the documents and leaves the pictures alone. Drawing them +takes longer than everything else the build does, and the gate that reads the +documents wants the documents. + `out/` is emptied at the start of every build. It holds nothing that is not made here, and a build that only writes what it still names leaves behind every name it has stopped naming — which is how a template written in the `${…}` dialect that has diff --git a/examples/build.php b/examples/build.php index 809442c..4543c14 100644 --- a/examples/build.php +++ b/examples/build.php @@ -12,7 +12,7 @@ * LibreOffice is installed, so the result can be checked without Word. */ -require __DIR__ . '/../vendor/autoload.php'; +require_once __DIR__ . '/../vendor/autoload.php'; use League\CommonMark\Extension\FrontMatter\FrontMatterExtension; use MarkdownWord\Configuration; @@ -70,8 +70,11 @@ final class BadPackage extends RuntimeException const DOCUMENT_TYPE = 'application/vnd.openxmlformats-officedocument.wordprocessingml.document.main+xml'; const TEMPLATE_TYPE = 'application/vnd.openxmlformats-officedocument.wordprocessingml.template.main+xml'; +/** Where the Markdown sources live, relative to this file. */ +const MARKDOWN = '/markdown'; + /** The house logo, put in the template's header. */ -const LOGO = __DIR__ . '/markdown/assets/example-logo-meridian.png'; +const LOGO = __DIR__ . MARKDOWN . '/assets/example-logo-meridian.png'; /** * A paragraph that stands for the content control until `controlIn()` has put one there. @@ -89,7 +92,7 @@ final class BadPackage extends RuntimeException $out = $root . '/out'; $docs = $out . '/documents'; $previews = $out . '/previews'; -$assets = $root . '/markdown/assets'; +$assets = $root . MARKDOWN . '/assets'; $templates = $root . '/templates'; // Everything in `out/` is produced by this script and nothing else, and it is @@ -151,6 +154,15 @@ final class BadPackage extends RuntimeException exit(0); } +// The previews are a picture of each document's first page, and drawing them takes +// longer than everything else this script does. A build that wants only the +// documents says so. +// +// The previews are still emptied with the rest of `out/`: a preview of a document +// that is no longer built is a picture of nothing, which is the same thing the +// directory is emptied for. +$withPreviews = !in_array('--no-previews', $argv ?? [], true); + $missing = array_keys(array_filter($templatePaths, static fn (string $t): bool => !is_file($t))); if ($missing !== []) { @@ -184,7 +196,7 @@ final class BadPackage extends RuntimeException // Images in the examples are relative to the Markdown file. $config = Configuration::create()->withOptions([ 'images' => Options::IMAGE_EMBED, - 'imageBasePath' => $root . '/markdown', + 'imageBasePath' => $root . MARKDOWN, ]); $path = $docs . '/' . $name . DOCX_EXTENSION; @@ -199,7 +211,7 @@ final class BadPackage extends RuntimeException $path = $docs . '/07-no-decoration' . DOCX_EXTENSION; (new MarkdownToWord( - (string) file_get_contents($root . '/markdown/01-kitchen-sink.md'), + (string) file_get_contents($root . MARKDOWN . '/01-kitchen-sink.md'), Configuration::create()->withoutDecoration(), ))->save($path); $built[] = ['07-no-decoration', 'The kitchen sink with every default switched off.', $path]; @@ -208,7 +220,7 @@ final class BadPackage extends RuntimeException $path = $docs . '/08-house-style' . DOCX_EXTENSION; (new MarkdownToWord( - (string) file_get_contents($root . '/markdown/01-kitchen-sink.md'), + (string) file_get_contents($root . MARKDOWN . '/01-kitchen-sink.md'), houseStyle(), ))->save($path); $built[] = ['08-house-style', 'The kitchen sink in a custom house style.', $path]; @@ -252,7 +264,7 @@ final class BadPackage extends RuntimeException */ function markdownOf(string $root, string $name): string { - return (string) file_get_contents($root . '/markdown/' . $name . '.md'); + return (string) file_get_contents($root . MARKDOWN . '/' . $name . '.md'); } // Frontmatter @@ -278,7 +290,7 @@ function markdownOf(string $root, string $name): string // 11 — the block is read, and it configures the render. $path = $docs . '/11-frontmatter' . DOCX_EXTENSION; (new MarkdownToWord( - (string) file_get_contents($root . '/markdown/11-frontmatter.md'), + (string) file_get_contents($root . MARKDOWN . '/11-frontmatter.md'), Configuration::create(), $frontmatterParser, ))->save($path); @@ -287,7 +299,7 @@ function markdownOf(string $root, string $name): string // 12 — the same Markdown with no block, for the pair. $path = $docs . '/12-frontmatter-none' . DOCX_EXTENSION; (new MarkdownToWord( - (string) file_get_contents($root . '/markdown/12-frontmatter-none.md'), + (string) file_get_contents($root . MARKDOWN . '/12-frontmatter-none.md'), Configuration::create(), $frontmatterParser, ))->save($path); @@ -296,7 +308,7 @@ function markdownOf(string $root, string $name): string // 13 — the block is outranked by the configuration passed in code. $path = $docs . '/13-frontmatter-override' . DOCX_EXTENSION; (new MarkdownToWord( - (string) file_get_contents($root . '/markdown/13-frontmatter-override.md'), + (string) file_get_contents($root . MARKDOWN . '/13-frontmatter-override.md'), // Deliberately the opposite of the block: no borders, no heading cap, a plain // grey Arial H1 with no air around it. Where the two disagree, this wins. Configuration::create()->withOptions([ @@ -356,7 +368,7 @@ function markdownOf(string $root, string $name): string // whole point of the shape — the document says what its own values are, and the // template says what those values look like. A PHP array here would be a second place // for the same fact, and changing one would not change the other. -$corporateMarkdown = (string) file_get_contents($root . '/markdown/22-corporate-report.md'); +$corporateMarkdown = (string) file_get_contents($root . MARKDOWN . '/22-corporate-report.md'); foreach ([ '22-corporate-report' => $dotxPath, @@ -416,7 +428,7 @@ function markdownOf(string $root, string $name): string // source holds as bytes is staged into a file of the library's own, and that file // belongs to the conversion — the resolver gives it back when the document is // written, and not before. -$resolver = new TemplateFile($root . '/markdown', roots: [$root, getcwd() ?: '.']); +$resolver = new TemplateFile($root . MARKDOWN, roots: [$root, getcwd() ?: '.']); (new MarkdownTemplate( $resolver->resolve( @@ -440,7 +452,7 @@ function markdownOf(string $root, string $name): string // it is an absolute directory and the source is committed, so it is passed here and // resolves `assets/logo.png` against `examples/markdown`. -$images = (string) file_get_contents($root . '/markdown/15-images.md'); +$images = (string) file_get_contents($root . MARKDOWN . '/15-images.md'); foreach ([ 'embed' => Options::IMAGE_EMBED, @@ -452,7 +464,7 @@ function markdownOf(string $root, string $name): string $images, Configuration::create()->withOptions([ 'images' => $value, - 'imageBasePath' => $root . '/markdown', + 'imageBasePath' => $root . MARKDOWN, ]), $frontmatterParser, ))->save($path); @@ -467,8 +479,8 @@ function markdownOf(string $root, string $name): string // without it the logo comes out as its own alt text where the picture should be. $path = $docs . '/' . '24-furniture' . DOCX_EXTENSION; (new MarkdownToWord( - (string) file_get_contents($root . '/markdown/24-furniture.md'), - Configuration::create()->withOptions(['imageBasePath' => $root . '/markdown']), + (string) file_get_contents($root . MARKDOWN . '/24-furniture.md'), + Configuration::create()->withOptions(['imageBasePath' => $root . MARKDOWN]), $frontmatterParser, ))->save($path); @@ -521,7 +533,7 @@ function markdownOf(string $root, string $name): string $path = $docs . '/29-objects' . $format->extension(); $converter = new MarkdownToWord($objects, Configuration::create()->withOptions([ 'images' => Options::IMAGE_EMBED, - 'imageBasePath' => $root . '/markdown', + 'imageBasePath' => $root . MARKDOWN, ]), $frontmatterParser); $converter->convertTo($format, $path); @@ -564,10 +576,10 @@ function markdownOf(string $root, string $name): string // resolves against the working directory and the picture is not found, which // falls back to the alt text and produces a document with no picture in it. (new MarkdownToWord( - (string) file_get_contents($root . '/markdown/19-vector.md'), + (string) file_get_contents($root . MARKDOWN . '/19-vector.md'), Configuration::create()->withOptions([ 'images' => Options::IMAGE_EMBED, - 'imageBasePath' => $root . '/markdown', + 'imageBasePath' => $root . MARKDOWN, ]), $frontmatterParser, ))->save($path); @@ -598,13 +610,13 @@ function markdownOf(string $root, string $name): string // `.docx` is built here too, so the three are the same conversion and not three // documents that happen to share a heading. -$formats = (string) file_get_contents($root . '/markdown/20-formats.md'); +$formats = (string) file_get_contents($root . MARKDOWN . '/20-formats.md'); foreach (Format::cases() as $format) { $path = $docs . '/20-formats' . $format->extension(); $converter = new MarkdownToWord($formats, Configuration::create()->withOptions([ 'images' => Options::IMAGE_EMBED, - 'imageBasePath' => $root . '/markdown', + 'imageBasePath' => $root . MARKDOWN, ]), $frontmatterParser); $converter->convertTo($format, $path); @@ -640,7 +652,9 @@ function markdownOf(string $root, string $name): string echo "\n"; } -renderPreviews($built, $previews); +if ($withPreviews) { + renderPreviews($built, $previews); +} /** * The report template's own styles, pointed at by name. diff --git a/examples/templates/invoice.dotx b/examples/templates/invoice.dotx index 1cf42e7..b97b4a5 100644 Binary files a/examples/templates/invoice.dotx and b/examples/templates/invoice.dotx differ diff --git a/examples/templates/meridian-qbr.dotx b/examples/templates/meridian-qbr.dotx index 0f72de0..fa73c81 100644 Binary files a/examples/templates/meridian-qbr.dotx and b/examples/templates/meridian-qbr.dotx differ diff --git a/examples/templates/plain-letterhead.dotx b/examples/templates/plain-letterhead.dotx index 2eae20d..52b349f 100644 Binary files a/examples/templates/plain-letterhead.dotx and b/examples/templates/plain-letterhead.dotx differ diff --git a/phpunit.xml.dist b/phpunit.xml.dist index c167bab..4e3c821 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -32,6 +32,15 @@ tests/Readme + + + tests/Ooxml + diff --git a/src/Exception/UnreadableTemplate.php b/src/Exception/UnreadableTemplate.php new file mode 100644 index 0000000..4eb3f3b --- /dev/null +++ b/src/Exception/UnreadableTemplate.php @@ -0,0 +1,16 @@ +numFiles > $options->maxEntries) { + // The count is read before the archive is closed: closing it resets the + // number of files to zero. + $entries = $zip->numFiles; + $package->close(); throw new UnreadableDocument(sprintf( 'The document has %d parts in it, which is more entries than the %d allowed.', - $zip->numFiles, + $entries, $options->maxEntries, )); } diff --git a/src/Template/MarkdownTemplate.php b/src/Template/MarkdownTemplate.php index 00b983a..88f27d7 100644 --- a/src/Template/MarkdownTemplate.php +++ b/src/Template/MarkdownTemplate.php @@ -12,6 +12,7 @@ use MarkdownWord\Exception\NoContentControl; use MarkdownWord\MarkdownToWord; use MarkdownWord\Exception\TemplateNotFound; +use MarkdownWord\Exception\UnreadableTemplate; use MarkdownWord\Files\FileFinder; use MarkdownWord\Parser\CommonMarkParser; use MarkdownWord\Parser\MarkdownParserInterface; @@ -45,10 +46,11 @@ * of its body, or says so and refuses, per {@see ContentFallback}. Filling the fields is * independent of it: a template can have values and nowhere to put the prose. * - * A template that is not there is a {@see TemplateNotFound}. Every other way this can - * fail is a {@see FileNotWritable} — the template was fine and the machine was not — - * except {@see NoContentControl}, which is the template's to fix and is off unless asked - * for. + * A template that is not there is a {@see TemplateNotFound}, and a template that holds + * more than this library will read is an {@see UnreadableTemplate}. Every other way + * this can fail is a {@see FileNotWritable} — the template was fine and the machine was + * not — except {@see NoContentControl}, which is the template's to fix and is off unless + * asked for. * * The staged document is moved into place rather than written there, so the * template on disk is never opened for writing: a path that is a hard link to it @@ -114,7 +116,10 @@ final class MarkdownTemplate * looked for in the directories of themes beside the document and beside * the working directory, and a picture against `imageBasePath` or the * working directory. + * @param TemplateOptions|null $limits What the template may hold before it is + * read. Null for the defaults, which are the read path's. * @throws TemplateNotFound when the template is not a file. + * @throws UnreadableTemplate when the template holds more than the limits allow. */ public function __construct( private readonly string $template, @@ -125,6 +130,7 @@ public function __construct( ?string $theme = null, ?string $directory = null, ?FileFinder $files = null, + private readonly ?TemplateOptions $limits = null, ) { if (!is_file($template)) { throw new TemplateNotFound(sprintf('The template "%s" does not exist.', $template)); @@ -133,7 +139,7 @@ public function __construct( // Settled before the processor is handed the file: a `.dotx` is normalised to // a document's package here, so the processor reads the body of whichever part // the declarations name and writes the finished document back out as one. - $this->package = TemplatePackage::of($template); + $this->package = TemplatePackage::of($template, $this->limits ?? new TemplateOptions()); $this->processor = new TemplateProcessor($this->package->path()); // The elements rendered below are copied into a different document, so diff --git a/src/Template/TemplateOptions.php b/src/Template/TemplateOptions.php new file mode 100644 index 0000000..8fd87a8 --- /dev/null +++ b/src/Template/TemplateOptions.php @@ -0,0 +1,40 @@ +maxEntries); + } + + /** The same limits with a different bound on the number of parts. */ + public function withMaxEntries(int $entries): self + { + return new self($this->maxPartBytes, $entries); + } +} diff --git a/src/Template/TemplatePackage.php b/src/Template/TemplatePackage.php index c699d73..2514049 100644 --- a/src/Template/TemplatePackage.php +++ b/src/Template/TemplatePackage.php @@ -5,6 +5,7 @@ namespace MarkdownWord\Template; use MarkdownWord\Exception\FileNotWritable; +use MarkdownWord\Exception\UnreadableTemplate; use MarkdownWord\WordPackage; use ZipArchive; @@ -47,13 +48,17 @@ private function __construct( * The copy outlives a single render, because {@see MarkdownTemplate} is rendered * into repeatedly and every one of those renders needs it. * - * @throws FileNotWritable when the normalised copy cannot be staged or written. + * @throws FileNotWritable when the template cannot be opened or the normalised + * copy cannot be staged or written. + * @throws UnreadableTemplate when the template holds more than it may. */ - public static function of(string $template): self + public static function of(string $template, TemplateOptions $options = new TemplateOptions()): self { $zip = new ZipArchive(); - if ($zip->open($template) !== true) { + // `CHECKCONS` refuses an archive whose central directory disagrees with its + // own contents. The read path opens a document the same way. + if ($zip->open($template, ZipArchive::CHECKCONS) !== true) { throw new FileNotWritable(sprintf( 'The template "%s" could not be opened. A Word template is a .docx or .dotx file, which is a zip archive.', $template, @@ -61,6 +66,8 @@ public static function of(string $template): self } try { + self::check($zip, $template, $options); + $isTemplate = WordPackage::isTemplate($zip); $declared = WordPackage::contentTypesOf($zip); } finally { @@ -92,6 +99,43 @@ public function __destruct() } } + /** + * What the archive may hold, before a part of it is read. + * + * The sizes are the ones the central directory declares, which cost one lookup + * each and no decompression. Every entry is measured, and not only the ones this + * library reads: the template is opened again by PHPWord's own processor, and a + * part that is read there is read whole. + * + * @throws UnreadableTemplate When the archive holds more parts than are allowed, + * or a part that declares to be larger than is allowed. + */ + private static function check(ZipArchive $zip, string $template, TemplateOptions $options): void + { + if ($zip->numFiles > $options->maxEntries) { + throw new UnreadableTemplate(sprintf( + 'The template "%s" has %d parts in it, which is more entries than the %d allowed.', + $template, + $zip->numFiles, + $options->maxEntries, + )); + } + + for ($index = 0; $index < $zip->numFiles; $index++) { + $stat = $zip->statIndex($index); + $size = is_array($stat) ? (int) ($stat['size'] ?? 0) : 0; + + if ($size > $options->maxPartBytes) { + throw new UnreadableTemplate(sprintf( + 'The template "%s" holds a part of %d bytes, which is larger than the %d allowed.', + $template, + $size, + $options->maxPartBytes, + )); + } + } + } + /** * Copied rather than edited in place, because the template belongs to whoever * handed it over and the caller may hold a hard link to it. @@ -109,9 +153,7 @@ private static function rewrite(string $template, string $declared): string )); } - // Each step that can fail removes the file it made before saying so, rather - // than leaving a half-written copy of somebody else's template in the - // temporary directory for something to find later. + // Each step that can fail removes the file it made before saying so. if (!copy($template, $temporary)) { @unlink($temporary); @@ -120,7 +162,7 @@ private static function rewrite(string $template, string $declared): string $zip = new ZipArchive(); - if ($zip->open($temporary) !== true) { + if ($zip->open($temporary, ZipArchive::CHECKCONS) !== true) { @unlink($temporary); throw new FileNotWritable(sprintf( diff --git a/src/Writer/HyperlinkPass.php b/src/Writer/HyperlinkPass.php index 9da2cf8..eb8cf05 100644 --- a/src/Writer/HyperlinkPass.php +++ b/src/Writer/HyperlinkPass.php @@ -182,24 +182,36 @@ private function buildRun(\DOMDocument $dom, string $text, array $style): \DOMEl } /** + * The properties of a run, in the order the schema names them. + * + * `w:rPr` is the sequence `EG_RPrBase` in ECMA-376's `wml.xsd` and not a set, + * so a reader walks the children in the order they are written and stops at + * the first one it did not expect: the typeface comes before the weight, and + * the underline comes after the size. + * + * Word opens a document that gets this wrong, because Word is lenient where + * the schema is not. A reader that walks the sequence — a validator, a + * converter — does not. + * * @param array $style */ private function buildRunProperties(\DOMDocument $dom, array $style): \DOMElement { $properties = $dom->createElementNS(self::W_NS, 'w:rPr'); + if (!empty($style['name'])) { + $fonts = $dom->createElementNS(self::W_NS, 'w:rFonts'); + $fonts->setAttributeNS(self::W_NS, 'w:ascii', (string) $style['name']); + $fonts->setAttributeNS(self::W_NS, 'w:hAnsi', (string) $style['name']); + $properties->appendChild($fonts); + } + foreach (['bold' => 'b', 'italic' => 'i', 'strikethrough' => 'strike'] as $key => $element) { if (!empty($style[$key])) { $properties->appendChild($dom->createElementNS(self::W_NS, 'w:' . $element)); } } - if (isset($style['underline']) && $style['underline'] !== 'none') { - $underline = $dom->createElementNS(self::W_NS, 'w:u'); - $underline->setAttributeNS(self::W_NS, 'w:val', (string) $style['underline']); - $properties->appendChild($underline); - } - if (!empty($style['color'])) { $color = $dom->createElementNS(self::W_NS, 'w:color'); $color->setAttributeNS(self::W_NS, 'w:val', ltrim((string) $style['color'], '#')); @@ -212,11 +224,10 @@ private function buildRunProperties(\DOMDocument $dom, array $style): \DOMElemen $properties->appendChild($size); } - if (!empty($style['name'])) { - $fonts = $dom->createElementNS(self::W_NS, 'w:rFonts'); - $fonts->setAttributeNS(self::W_NS, 'w:ascii', (string) $style['name']); - $fonts->setAttributeNS(self::W_NS, 'w:hAnsi', (string) $style['name']); - $properties->appendChild($fonts); + if (isset($style['underline']) && $style['underline'] !== 'none') { + $underline = $dom->createElementNS(self::W_NS, 'w:u'); + $underline->setAttributeNS(self::W_NS, 'w:val', (string) $style['underline']); + $properties->appendChild($underline); } return $properties; diff --git a/tests/Ooxml/gate.php b/tests/Ooxml/gate.php new file mode 100644 index 0000000..8fe43fd --- /dev/null +++ b/tests/Ooxml/gate.php @@ -0,0 +1,221 @@ +beforeEach(function (): void { + $missing = whyTheValidatorCannotRun(); + + if ($missing !== '') { + test()->markTestSkipped($missing); + } +}); + +it('writes documents the schema accepts', function () { + $corpus = theCorpus(); + $answer = runTheValidator($corpus); + + expect($answer['code'])->toBe(0, sprintf( + "The validator rejected %d of the %d documents the build wrote:%s%s", + countDocumentsWithErrors($answer['out']), + count($corpus), + PHP_EOL, + $answer['out'], + )); +}); + +it('finds an error in a document that has one', function () { + // The gate is worth nothing if it cannot fail, so this one asks the validator + // about a document with an error in it. The error is an element no schema names; + // the document still opens, so what refuses it is the schema and not a broken + // archive. + $document = Scratch::path('broken', '.docx'); + + file_put_contents($document, toDocx("# Title\n\nA paragraph.\n")); + + TemplateFactory::addRawPart( + $document, + 'word/document.xml', + str_replace('', '', TemplateFactory::xmlOf($document)), + ); + + $answer = runTheValidator([$document]); + + expect($answer['code'])->toBe(1, 'The validator did not refuse the document:' . PHP_EOL . $answer['out']) + ->and($answer['out'])->toContain('notAnElement'); +}); + +/** + * The documents the examples build. + * + * The build is run here rather than the corpus being read off the disk, because a + * corpus left over from an earlier commit is a corpus of documents the code under + * test did not write. + * + * @return list + */ +function theCorpus(): array +{ + static $corpus = null; + + if ($corpus !== null) { + return $corpus; + } + + $root = dirname(__DIR__, 2); + + $build = runACommand(sprintf( + '%s %s --no-previews 2>&1', + escapeshellarg(PHP_BINARY), + escapeshellarg($root . '/examples/build.php'), + )); + + if ($build['code'] !== 0) { + throw new RuntimeException('The examples did not build:' . PHP_EOL . $build['out']); + } + + $documents = glob($root . '/examples/out/documents/*.docx') ?: []; + + sort($documents); + + if ($documents === []) { + throw new RuntimeException('The build wrote no documents:' . PHP_EOL . $build['out']); + } + + return $corpus = $documents; +} + +/** + * What the validator says about a set of files. + * + * The exit code is the answer: 0 when every file is clean, 1 when one of them is + * not, and 2 when there was nothing to check. The output says which file and why, + * which is what a fix has to be pointed at. + * + * @param list $files + * + * @return array{code: int, out: string} + */ +function runTheValidator(array $files): array +{ + return runACommand(sprintf( + '%s run --project %s --no-build -- %s 2>&1', + escapeshellarg((string) commandOnPath('dotnet')), + escapeshellarg(theBuiltValidator()), + implode(' ', array_map('escapeshellarg', $files)), + )); +} + +/** + * The validator, built and ready. + * + * The build restores the package it depends on from the network, so it is done once + * rather than before every document. + * + * @return string The path to the project `dotnet run` is given. + */ +function theBuiltValidator(): string +{ + static $project = null; + + if ($project !== null) { + return $project; + } + + $project = dirname(__DIR__, 2) . '/tools/ooxml-validator'; + + $build = runACommand(sprintf( + '%s build %s --nologo --verbosity quiet 2>&1', + escapeshellarg((string) commandOnPath('dotnet')), + escapeshellarg($project), + )); + + if ($build['code'] !== 0) { + throw new RuntimeException('The OOXML validator did not build:' . PHP_EOL . $build['out']); + } + + return $project; +} + +/** + * Why the validator cannot run on this machine, or an empty string when it can. + * + * A test that fails where it cannot run says nothing about the code — so the test + * skips itself and says what is missing instead. + */ +function whyTheValidatorCannotRun(): string +{ + static $answer = null; + + if ($answer !== null) { + return $answer; + } + + $dotnet = commandOnPath('dotnet'); + + if ($dotnet === null) { + return $answer = 'The .NET SDK is not on this machine, so the OOXML validator cannot be run.'; + } + + $lines = []; + $status = 0; + + exec(escapeshellarg($dotnet) . ' --list-sdks 2>&1', $lines, $status); + + if ($status !== 0 || $lines === []) { + return $answer = 'This machine has the .NET runtime but no .NET SDK, so the validator cannot be built.'; + } + + return $answer = ''; +} + +/** + * How many documents the validator found errors in. + * + * Counted from the report rather than from the exit code, which says only that + * something is wrong. The line that names the count in total ends in `error(s)` + * as well, and it is not a document. + */ +function countDocumentsWithErrors(string $report): int +{ + $found = 0; + + foreach (explode(PHP_EOL, $report) as $line) { + if (preg_match('#^.*\.docx: [1-9][0-9]* error\(s\)$#', $line) === 1) { + $found++; + } + } + + return $found; +} + +/** + * What a command did, and what it said. + * + * @return array{code: int, out: string} + */ +function runACommand(string $command): array +{ + $lines = []; + $code = 0; + + exec($command, $lines, $code); + + return ['code' => $code, 'out' => implode(PHP_EOL, $lines)]; +} diff --git a/tests/Pest.php b/tests/Pest.php index 82949d7..108b045 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -313,6 +313,40 @@ function toDocx(string $markdown, ?Configuration $config = null): string ); } +/** + * The command on this machine's PATH, or nothing. + * + * The PATH is walked rather than the shell asked, because `command -v` is a POSIX + * command and a Windows command prompt does not have it. The path is returned + * rather than a yes or a no, because a caller that has it wants to run it. + */ +function commandOnPath(string $command): ?string +{ + $path = getenv('PATH'); + + if ($path === false) { + return null; + } + + foreach (explode(PATH_SEPARATOR, $path) as $directory) { + if ($directory === '') { + continue; + } + + $candidate = rtrim($directory, '/\\') . DIRECTORY_SEPARATOR . $command; + + if (PHP_OS_FAMILY === 'Windows') { + $candidate .= '.exe'; + } + + if (is_file($candidate) && is_executable($candidate)) { + return $candidate; + } + } + + return null; +} + // `tmp/pest` is emptied after every test, failure or not, so a run leaves // nothing behind. The hook goes through `pest()`: in Pest 5 a bare `afterEach()` // written here binds to this file, which is not a test file, so it would never diff --git a/tests/Readme/examples-tables.php b/tests/Readme/examples-tables.php index b6937e8..7881c73 100644 --- a/tests/Readme/examples-tables.php +++ b/tests/Readme/examples-tables.php @@ -87,7 +87,12 @@ function firstTableHeaderCell(string $xml): string // every page the table spills onto. It is the whole of what a row can be told: // the slot is handed a `RowStyle`, which has no appearance of its own — an // earlier version of this example asked it for a shading and it was dropped. - expect(tableExample('16-styled-tables'))->toMatch('##'); + // + // The value is `on` and not `1`. Both are legal in the file — ECMA-376's + // `ST_OnOff` allows `true`, `false`, `on`, `off`, `0` and `1` — but the Open + // XML SDK reads this element with an enumeration of `on` and `off` alone, and + // refuses the others. + expect(tableExample('16-styled-tables'))->toMatch('##'); }); it('takes the cell typography from the cell slot', function () { diff --git a/tests/Unit/api-shape.php b/tests/Unit/api-shape.php index 5e364f5..45e1efb 100644 --- a/tests/Unit/api-shape.php +++ b/tests/Unit/api-shape.php @@ -3,6 +3,7 @@ declare(strict_types=1); use MarkdownWord\Converter; +use MarkdownWord\Input; use MarkdownWord\MarkdownToWord; use MarkdownWord\WordToMarkdown; use MarkdownWord\Tests\Support\Scratch; @@ -66,8 +67,18 @@ function bothDirections(): array expect($bytes)->toBeString(); expect(is_file($target))->toBeTrue(); - // Whatever it produced is the same whether returned or written. - expect((string) file_get_contents($target))->toBe($bytes); + // Whatever it produced is the same whether returned or written. A document + // is compared by its parts and not by its bytes: a zip carries the time it + // was written and `docProps/core.xml` carries the time the document was + // made, so two correct conversions of the same input differ most of the + // time. + $written = (string) file_get_contents($target); + + if (Input::looksLikeDocument($written)) { + expect(documentParts($written))->toBe(documentParts($bytes)); + } else { + expect($written)->toBe($bytes); + } } }); diff --git a/tests/Unit/dead-code.php b/tests/Unit/dead-code.php index 39185ed..d4cda93 100644 --- a/tests/Unit/dead-code.php +++ b/tests/Unit/dead-code.php @@ -117,6 +117,7 @@ function spelledOut(string $source): string 'code block shading goes through a paragraph style, not a Font' => 'used for code block shading', 'the round trip loses what Word does not record' => 'the round trip is exact', 'a link wrapping a bare image is marked by a flag nothing read' => 'imageLabel', + 'a document with its run properties out of order is one Word refuses to open' => 'run properties are out of that order is one Word refuses to open', 'the version is written down in only one place' => 'The one place the version is written down', 'shipping the lock file makes an install reproducible' => 'a reproducible install is worth', 'one list of style keys covers every slot' => 'The keys a style slot\'s array form understands', @@ -421,6 +422,18 @@ function spelledOut(string $source): string ->and($styles->heading(0))->toBe($styles->get(Styles::HEADING_1)); }); +it('keeps the install state out of the package', function () { + // `patches.lock.json` records the patch composer-patches applied on one + // machine, and it wins over the manifest: a file left in the repository + // installs that patch rather than the one the manifest names. Every + // `composer install` writes it, so it is ignored rather than committed. + $root = dirname(__DIR__, 2); + $tracked = []; + exec(sprintf('git -C %s ls-files -- patches.lock.json 2>&1', escapeshellarg($root)), $tracked); + + expect($tracked)->toBe([]); +}); + it('reports a template that is not there as the caller\'s mistake', function () { // A missing template is a file the caller named and can name differently, so // it belongs with the other "that is not what the conversion needs" failures diff --git a/tests/Unit/dist-archive.php b/tests/Unit/dist-archive.php index 441b9fc..55d46d5 100644 --- a/tests/Unit/dist-archive.php +++ b/tests/Unit/dist-archive.php @@ -102,6 +102,13 @@ function distArchiveFiles(): array expect(distArchiveFiles())->not->toContain('composer.lock'); }); +it('does not ship the patch lock, which records one machine\'s install', function () { + // The patch lock says which patch was applied, at which checksum and at what + // depth. It is this repository's install state, not the package's, and a + // consumer's install never reads it. + expect(distArchiveFiles())->not->toContain('patches.lock.json'); +}); + it('keeps the lock file in the repository, which this repository\'s installs pin to', function () { // Both rules are the reason. No consumer ever reads the lock, which is what the // export-ignore is for, but this repository's own installs are meant to be the diff --git a/tests/Unit/hyperlink-run-properties.php b/tests/Unit/hyperlink-run-properties.php new file mode 100644 index 0000000..6dac448 --- /dev/null +++ b/tests/Unit/hyperlink-run-properties.php @@ -0,0 +1,104 @@ + + */ +function hyperlinkRunPropertyNames(string $markdown, ?Configuration $config = null): array +{ + $path = Scratch::path('hyperlink-run-properties'); + + saveDocument($markdown, $path, $config); + + $dom = new DOMDocument(); + libxml_use_internal_errors(true); + $dom->loadXML(TemplateFactory::xmlOf($path), LIBXML_NOCDATA); + libxml_clear_errors(); + + $xpath = new DOMXPath($dom); + $xpath->registerNamespace('w', 'http://schemas.openxmlformats.org/wordprocessingml/2006/main'); + + $hyperlink = $xpath->query('//w:hyperlink')?->item(0); + + if (!$hyperlink instanceof DOMElement) { + throw new RuntimeException('The document has no hyperlink.'); + } + + $names = []; + + foreach ($xpath->query('./w:r[1]/w:rPr/*', $hyperlink) ?: [] as $child) { + if ($child instanceof DOMElement) { + $names[] = $child->localName; + } + } + + return $names; +} + +/** + * A link font carrying every property the writer puts in a run, so the whole of + * what the document says about the run is the order of its properties. + */ +function hyperlinkRunPropertiesConfig(): Configuration +{ + return (new Configuration()) + ->withOptions(['deferredHyperlinks' => true]) + ->withStyles([ + Styles::LINK_FONT => [ + 'name' => 'Consolas', + 'size' => 11, + 'color' => 'B22222', + 'bold' => true, + 'italic' => true, + 'strikethrough' => true, + 'underline' => 'single', + ], + ]); +} + +it('writes the run properties of a link in the order the schema names them', function () { + $properties = hyperlinkRunPropertyNames( + '[label](https://example.com)', + hyperlinkRunPropertiesConfig(), + ); + + expect($properties)->toBe(['rFonts', 'b', 'i', 'strike', 'color', 'sz', 'u']); +}); + +it('writes the run properties of a link with emphasis in the order the schema names them', function () { + // The path a label with emphasis takes on its own: the label cannot be one + // plain string, so the runs are recorded and written afterwards. + $properties = hyperlinkRunPropertyNames('[*label*](https://example.com)'); + + expect($properties)->toBe(['i', 'color', 'u']); +}); + +it('writes the typeface of a link before the properties that follow it', function () { + // `w:rFonts` is the second element the schema names, and a run here carries + // nothing before it, so the typeface is written first. + $properties = hyperlinkRunPropertyNames('[label](https://example.com)', hyperlinkRunPropertiesConfig()); + + expect($properties[0])->toBe('rFonts'); +}); + +it('writes the underline of a link after the size of its run', function () { + // `w:u` is named after the size and before the effect, and it is the last of + // the properties a link carries, so it is written last. + $properties = hyperlinkRunPropertyNames('[label](https://example.com)', hyperlinkRunPropertiesConfig()); + + expect($properties[array_key_last($properties)])->toBe('u'); +}); diff --git a/tests/Unit/render-tools.php b/tests/Unit/render-tools.php index d20b1e8..a9d66d9 100644 --- a/tests/Unit/render-tools.php +++ b/tests/Unit/render-tools.php @@ -110,17 +110,6 @@ function noTools(): string return toolsHolding(); } -/** - * Whether a command is on the path. - */ -function commandOnPath(string $command): bool -{ - $status = 0; - exec('command -v ' . escapeshellarg($command) . ' 2>/dev/null', $ignored, $status); - - return $status === 0; -} - /** * A directory that stands for an installed application. * @@ -404,7 +393,7 @@ function packageAsDocument(string $source, string $target): array test()->markTestSkipped('Microsoft Word is only here on macOS, so the render cannot be checked.'); } - if (! commandOnPath('osascript')) { + if (commandOnPath('osascript') === null) { test()->markTestSkipped('AppleScript is not here, so Word cannot be driven.'); } @@ -437,7 +426,7 @@ function packageAsDocument(string $source, string $target): array test()->markTestSkipped('Microsoft Word is only here on macOS, so the render cannot be checked.'); } - if (! commandOnPath('osascript')) { + if (commandOnPath('osascript') === null) { test()->markTestSkipped('AppleScript is not here, so Word cannot be driven.'); } diff --git a/tests/Unit/reverse-file-fixes.php b/tests/Unit/reverse-file-fixes.php index 8b87656..6e00d4e 100644 --- a/tests/Unit/reverse-file-fixes.php +++ b/tests/Unit/reverse-file-fixes.php @@ -522,6 +522,17 @@ function fileFixInChildProcess(string $code, string $memoryLimit = '16M'): array expect(fn () => Package::open($document, ReverseOptions::fromArray(['maxEntries' => 4]))) ->toThrow(UnreadableDocument::class, 'more entries than'); + // The message says how many parts the archive holds, and the number it says is + // the archive's own: closing the archive resets the count to zero, and a + // message that says zero sends the reader looking for the wrong thing. + $zip = new ZipArchive(); + $zip->open($document); + $entries = $zip->numFiles; + $zip->close(); + + expect(fn () => Package::open($document, ReverseOptions::fromArray(['maxEntries' => 4]))) + ->toThrow(UnreadableDocument::class, sprintf('has %d parts in it', $entries)); + $package = Package::open($document); $root = $package->document()->documentElement?->localName; $package->close(); diff --git a/tests/Unit/template-bounds.php b/tests/Unit/template-bounds.php new file mode 100644 index 0000000..dc34e08 --- /dev/null +++ b/tests/Unit/template-bounds.php @@ -0,0 +1,122 @@ + TemplatePackage::of($path, (new TemplateOptions())->withMaxEntries(10))) + ->toThrow(UnreadableTemplate::class, 'more entries than the 10 allowed'); +}); + +it('refuses a template that holds a part larger than is allowed', function () { + $path = templateWithABigPart(1024 * 1024); + + expect(fn () => TemplatePackage::of($path, (new TemplateOptions())->withMaxPartBytes(1024))) + ->toThrow(UnreadableTemplate::class, 'larger than the 1024 allowed'); +}); + +it('says how many parts a template holds when it refuses it', function () { + $path = templateWithManyParts(20); + + // The number the message carries is the archive's own: a message that says zero + // sends the reader looking for the wrong thing. + $zip = new ZipArchive(); + $zip->open($path); + $entries = $zip->numFiles; + $zip->close(); + + expect(fn () => TemplatePackage::of($path, new TemplateOptions(maxEntries: 10))) + ->toThrow(UnreadableTemplate::class, sprintf('has %d parts in it', $entries)); +}); + +it('reads a template that is within the limits', function () { + // The other side of the guard: a limit that refuses everything refuses the + // templates that are fine. + $path = templateWithManyParts(20); + + expect(fn () => TemplatePackage::of($path, new TemplateOptions(maxEntries: 100, maxPartBytes: 1024 * 1024))) + ->not->toThrow(UnreadableTemplate::class); +}); + +it('refuses a template that is too much through the template itself', function () { + $path = templateWithABigPart(1024 * 1024); + + expect(fn () => new MarkdownTemplate($path, limits: new TemplateOptions(maxPartBytes: 1024))) + ->toThrow(UnreadableTemplate::class); +}); + +/** + * The declarations of a package whose body is a template. + */ +function templateDeclarations(): string +{ + return '' + . '' + . '' + . '' + . '' + . ''; +} + +/** + * A package with the given number of parts beyond the ones it needs. + */ +function templateWithManyParts(int $parts): string +{ + $path = Scratch::path('many-parts', '.dotx'); + + file_put_contents($path, ''); + + $zip = new ZipArchive(); + $zip->open($path, ZipArchive::OVERWRITE); + $zip->addFromString('[Content_Types].xml', templateDeclarations()); + $zip->addFromString( + 'word/document.xml', + '' + . '', + ); + + for ($index = 0; $index < $parts; $index++) { + $zip->addFromString('word/media/image' . $index . '.txt', 'x'); + } + + $zip->close(); + + return $path; +} + +/** + * A package with one part of the given size. + */ +function templateWithABigPart(int $bytes): string +{ + $path = Scratch::path('big-part', '.dotx'); + + file_put_contents($path, ''); + + $zip = new ZipArchive(); + $zip->open($path, ZipArchive::OVERWRITE); + $zip->addFromString('[Content_Types].xml', templateDeclarations()); + $zip->addFromString( + 'word/document.xml', + '' + . '', + ); + $zip->addFromString('word/media/big.bin', str_repeat(' ', $bytes)); + + $zip->close(); + + return $path; +} diff --git a/tools/ooxml-validator/Program.cs b/tools/ooxml-validator/Program.cs new file mode 100644 index 0000000..bbfdb15 --- /dev/null +++ b/tools/ooxml-validator/Program.cs @@ -0,0 +1,114 @@ +// Validates Office Open XML files against the schema, with Microsoft's own +// validator — the one the Open XML SDK Productivity Tool uses. +// +// The exit code is the answer: 0 when every file is clean, 1 when one of them is +// not, 2 when the tool was asked to do nothing it could do. A gate built on +// anything else would be a gate on this suite's own opinion of the format rather +// than on the format. + +using DocumentFormat.OpenXml; +using DocumentFormat.OpenXml.Packaging; +using DocumentFormat.OpenXml.Validation; + +if (args.Length == 0) +{ + Console.Error.WriteLine("usage: ooxml-validator [--version ] ..."); + return 2; +} + +// Office2019 rather than the oldest version the SDK knows: the SDK validates +// against the schema of the version it is given, and a document written for a +// modern Word should be read as a modern Word would read it. Every file this +// suite writes has been checked against every version the SDK knows, and the +// errors are the same at all of them. +var version = FileFormatVersions.Office2019; +var files = new List(); + +// The files are gathered while the options are read, because the value of +// `--version` is an option's value and not the name of a file. +for (var i = 0; i < args.Length; i++) +{ + if (args[i] == "--version" && i + 1 < args.Length) + { + var value = args[++i]; + if (!Enum.TryParse(value, ignoreCase: true, out version)) + { + Console.Error.WriteLine($"not a version the SDK knows: {value}"); + return 2; + } + + continue; + } + + if (args[i].StartsWith("--")) + { + Console.Error.WriteLine($"unknown option: {args[i]}"); + return 2; + } + + files.Add(args[i]); +} + +if (files.Count == 0) +{ + Console.Error.WriteLine("no files to validate"); + return 2; +} + +var errors = 0; + +foreach (var file in files) +{ + if (!File.Exists(file)) + { + Console.Error.WriteLine($"{file}: no such file"); + errors++; + continue; + } + + var extension = Path.GetExtension(file).ToLowerInvariant(); + + try + { + errors += extension switch + { + ".docx" or ".docm" or ".dotx" or ".dotm" => Validate(file, WordprocessingDocument.Open(file, false), version), + ".pptx" or ".pptm" or ".potx" or ".potm" => Validate(file, PresentationDocument.Open(file, false), version), + _ => Unknown(file), + }; + } + catch (Exception exception) + { + // A file the SDK cannot open is a file the gate cannot pass, so it counts + // as an error rather than as a reason to give up. + Console.Error.WriteLine($"{file}: could not be opened: {exception.Message}"); + errors++; + } +} + +Console.WriteLine(errors == 0 + ? $"{files.Count} file(s) checked against {version}: no errors" + : $"{files.Count} file(s) checked against {version}: {errors} error(s)"); + +return errors == 0 ? 0 : 1; + +static int Validate(string file, OpenXmlPackage package, FileFormatVersions version) +{ + var found = new OpenXmlValidator(version).Validate(package).ToList(); + + Console.WriteLine($"{file}: {found.Count} error(s)"); + + foreach (var error in found) + { + Console.WriteLine($" [{error.ErrorType}] {error.Description}"); + Console.WriteLine($" part={error.Part?.Uri} path={error.Path?.XPath}"); + } + + return found.Count; +} + +static int Unknown(string file) +{ + Console.Error.WriteLine($"{file}: not a Word or PowerPoint file"); + return 1; +} diff --git a/tools/ooxml-validator/README.md b/tools/ooxml-validator/README.md new file mode 100644 index 0000000..e78c508 --- /dev/null +++ b/tools/ooxml-validator/README.md @@ -0,0 +1,38 @@ +# The OOXML validator + +A console program that opens a `.docx` or a `.pptx` and reports what Microsoft's +Open XML SDK makes of it. It is the oracle the gate in this repository is measured +against, so it is not a dependency of the library — nothing the library ships +depends on it, and the library's own suite does not need it to run. + +## Running it + +```bash +dotnet run --project tools/ooxml-validator -- path/to/document.docx +dotnet run --project tools/ooxml-validator -- --version Office2007 path/to/document.docx +``` + +The exit code is the answer: `0` when every file is clean, `1` when one of them is +not, `2` when there was nothing to check. The errors are printed one per line with +the part they are in and the path to them, which is what a fix needs to be pointed +at. + +## Why this and not the schema + +The schema in ECMA-376 is the definition of the format, and a document that breaks +it is a document that is not what it claims to be. The SDK reads the same schema, +and it is the reader the Open XML SDK Productivity Tool — the tool Microsoft's own +documentation points at — uses. A gate built on it is a gate on the format rather +than on this suite's opinion of it. + +The SDK is also stricter than the schema in a few places, and those places are +named in the pull requests rather than papered over: `w:tblHeader` and +`w:bidiVisual` carry an on/off value the SDK reads as `on` or `off` alone, where +the schema's `ST_OnOff` also allows `1` and `0`. + +## Where it runs + +The gate that uses it is a job in `.github/workflows/tests.yml`, which installs the +.NET SDK with `actions/setup-dotnet` before the suite runs. A machine without the +.NET SDK can still run the rest of the suite; the test that uses this tool skips +itself there and says why. diff --git a/tools/ooxml-validator/ooxml-validator.csproj b/tools/ooxml-validator/ooxml-validator.csproj new file mode 100644 index 0000000..963b2cb --- /dev/null +++ b/tools/ooxml-validator/ooxml-validator.csproj @@ -0,0 +1,25 @@ + + + + Exe + + net8.0 + + Major + enable + enable + OoxmlValidator + ooxml-validator + + + + + + + +