Skip to content

GD readers : Fixed valid files rejected because of PHP deprecations - #15

Open
mkantautas wants to merge 2 commits into
PHPOffice:masterfrom
mkantautas:fix/wmf-null-mapmode
Open

mkantautas wants to merge 2 commits into
PHPOffice:masterfrom
mkantautas:fix/wmf-null-mapmode

Conversation

@mkantautas

@mkantautas mkantautas commented Oct 24, 2026 •

Copy link
Copy Markdown

A standard WMF file (no placeable header) without a META_SETMAPMODE record leaves $mapMode null in getWindowBounds(), and $unitsPerInch[$mapMode] ?? 1440 uses null as an array offset, which PHP 8.5 deprecates. The error handlers of both GD readers turned every reported error into WMFException('Reader : Invalid file'), deprecations included, so the file failed to load. In the EMF reader, a deprecation raised while drawing EMF+ records would make the file fall back to its EMF records.

  • PhpOffice\WMF\Reader\WMF\GD & PhpOffice\WMF\Reader\EMF\GD : the error handlers return false for E_DEPRECATED and E_USER_DEPRECATED, so PHP reports them and the file loads.
  • PhpOffice\WMF\Reader\WMF\GD : the units per inch are looked up with $mapMode ?? 0. Files without a map mode keep 1440 units per inch, now in the DEFAULT_UNITS_PER_INCH constant, also used for a placeable header without units.
  • Changelog entries under "Bug fixes" in docs/changes/0.2.1.md.

This is item 2 of #13.

Tests: testLoadStandardWMF loads burger.wmf without its placeable header with error_reporting(E_ALL) and checks that no error is raised; it fails on PHP 8.5 without the fix. testLoadWithDeprecation (WMF and EMF readers) triggers an E_USER_DEPRECATED while reading and checks that the file loads, and that the EMF+ records of libemf2svg/emf/test-000.emf are still drawn; it fails without the error handler change. Full suite on PHP 8.5.8, 8.1.32 and 7.4.33: the only failures are tests that need the imagick extension (not installed locally), the same set as on master. phpstan and php-cs-fixer report no issues.

Progi1984
Progi1984 previously approved these changes Nov 9, 2026
@Progi1984

Copy link
Copy Markdown
Member

Thanks for the fix! It does solve the PHP 8.5 failure for WMF files without a META_SETMAPMODE record. The full test suite passes on PHP 8.5.11 (601 tests, 15 skipped).

A few remarks:

1. The root cause is still there (src/WMF/Reader/WMF/GD.php:187)
The error handler in loadContent() turns every PHP error into WMFException('Reader : Invalid file'), including E_DEPRECATED. So any deprecation PHP raises inside reader code makes a valid file fail to load. That is exactly what happened here. Rather than adding
null guards one at a time, the handler could return false for E_DEPRECATED / E_USER_DEPRECATED and let PHP handle them:

  if ($errno === E_DEPRECATED || $errno === E_USER_DEPRECATED) {
      return false;
  }

2. The EMF reader has the same issue (src/WMF/Reader/EMF/GD.php:236)
Its error handler works the same way, so the same kind of PHP 8.5+ deprecation can make it reject valid EMF/EMF+ files. For EMF+, this can mean a silent fallback to plain EMF rendering.

3. Default units per inch when no map mode is set (src/WMF/Reader/WMF/GD.php:320)
Without META_SETMAPMODE, the default map mode is MM_TEXT, which is 96 units per inch in the existing table. The code falls back to 1440 (twips), so a 200×100 window extent renders at 10×5 px instead of 150×75 px. This behaviour predates this PR, but the new test
now locks it in. Is 1440 intentional here?

4. Minor: duplicated fallback
1440 appears twice in the new ternary and once more in readHeader() (line 254, ?: 1440). This would do the same with a single default:

  $unitsPerInch[$mapMode ?? 0] ?? 1440

Or move the value into a constant.

5. Changelog
Could you add an entry under "Bug fixes" in docs/changes/0.2.1.md, as in #11 and #12?

- `PhpOffice\WMF\Reader\WMF\GD` & `PhpOffice\WMF\Reader\EMF\GD` : the error handler returns false for `E_DEPRECATED` & `E_USER_DEPRECATED`, so a deprecation no longer rejects a valid file or makes an EMF+ file fall back to its EMF records
- `PhpOffice\WMF\Reader\WMF\GD` : the default of 1440 units per inch is a constant, used by the placeable header and by the window
- `testLoadStandardWMF` checks that `burger.wmf` without its placeable header (no `META_SETMAPMODE` record) loads without any error, and replaces `testLoadStandardWMFWithoutMapMode`
- Changelog entries in `docs/changes/0.2.1.md`
@mkantautas mkantautas changed the title PhpOffice\WMF\Reader\WMF\GD : Fixed standard WMF files without map mode on PHP 8.5 GD readers : Fixed valid files rejected because of PHP deprecations Nov 9, 2026
@mkantautas

Copy link
Copy Markdown
Author

Thanks for the review. Pushed f7d4c60:

  1. Both error handlers return false for E_DEPRECATED and E_USER_DEPRECATED. testLoadWithDeprecation (WMF and EMF readers) triggers an E_USER_DEPRECATED while reading: the file loads and PHP records the deprecation. I kept the lookup fix too, so these files raise no deprecation on PHP 8.5.
  2. Same change in src/WMF/Reader/EMF/GD.php. The EMF test loads libemf2svg/emf/test-000.emf and checks that the EMF+ records are still drawn.
  3. I kept 1440. MS-WMF 2.3.2.3 META_PLACEABLE, Inch: "By convention, an image is considered to be recorded at 1440 logical units (twips) per inch." None of the sample WMF files (burger, chicken, fish, vegetable) has a META_SETMAPMODE record, so testLoadStandardWMF already went through this fallback (its comment said MM_ANISOTROPIC; I corrected it). Their placeable headers say 1000 units per inch: burger.wmf is 165×147 px with its header, 115×102 without it at 1440, and would be 1718×1527 at 96. LibreOffice's reader has "Default is 1440, but it is set to 96 to show the wmf larger" and then caps the width at 1024 px (wmfreader.cxx). If you want MM_TEXT's 96 anyway, I'll switch it.
  4. $unitsPerInch[$mapMode ?? 0] ?? self::DEFAULT_UNITS_PER_INCH, and readHeader() uses the same constant.
  5. Two entries under "Bug fixes" in docs/changes/0.2.1.md.

testLoadStandardWMFWithoutMapMode is gone: testLoadStandardWMF now loads with error_reporting(E_ALL) and checks that no error was raised, which fails on PHP 8.5 without the fix.

Tests on PHP 8.5.8, 8.1.32 and 7.4.33: the only failures are tests that need imagick (not installed here), the same set as on master. phpstan and php-cs-fixer report no issues.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants