dompdf / php-font-lib

A library to read, parse, export and make subsets of different types of font files.
GNU Lesser General Public License v2.1
1.74k stars 255 forks source link

Fix crashing google fonts or other TTF font files #121

Closed wpmvangroesen closed 11 months ago

wpmvangroesen commented 1 year ago

We encountered a lot of issues with unpack errors in BinaryStream (see: #47 ) for some of the fonts hosted by Google. The real fix would probably be to better understand the differences in the table definitions. We tried to check this using ttfdump or manually checking the font files, but we didn't find any big issues there. The codebase is pretty old and difficult to debug. So i don't know if anyone would be interested to investigate or fix this in a better way.

This commit solves the problem for us. The library is not crashing and fonts are being rendered in DOMPDF.

Matergi commented 1 year ago

this fixed the bug

daugaard47 commented 1 year ago

Please merge this commit.

daugaard47 commented 1 year ago

If using Laravel here is how I'm adding this fix as a patch until this is merged. FYI, I'm using barryvdh/laravel-dompdf.

Step 1: Create a Patches Directory

Step 2: Create the Patch File

  1. Inside the patches directory, create a file named php-font-lib-readInt16-fix.patch.
  2. Open the file and paste the following patch content:

    
    --- a/vendor/phenx/php-font-lib/src/FontLib/BinaryStream.php
    +++ b/vendor/phenx/php-font-lib/src/FontLib/BinaryStream.php
    @@ -147,9 +147,11 @@
    }
    
    public function readInt16() {
    -    $a = unpack("nn", $this->read(2));
    +    $data = $this->read(2);
    +    if (!$data) return null;
    +
    +    $a = unpack("nn", $data);
     $v = $a["n"];
    -
     if ($v >= 0x8000) {
       $v -= 0x10000;
     }

**Step 3: Create the Patching Script**

1. Inside the patches directory, create a file named `apply-patch.php`.
2. Open the file and paste the following PHP script:

<?php

$sourceFile = 'vendor/phenx/php-font-lib/src/FontLib/BinaryStream.php'; $backupFile = 'vendor/phenx/php-font-lib/src/FontLib/BinaryStream.php.bak'; $patchFile = 'patches/php-font-lib-readInt16-fix.patch';

if (!file_exists($backupFile)) { copy($sourceFile, $backupFile); }

exec("patch --forward -p1 < $patchFile 2>&1", $output, $returnVar);

if ($returnVar === 0) { echo "Patch applied successfully.\n"; } elseif (str_contains(implode("\n", $output), 'Skipping patch')) { echo "Patch was already applied. Skipping.\n"; } else { echo "Failed to apply patch.\n"; exit(1); }


**Step 4: Update `composer.json`**

1. Open your `composer.json` file.
2. Add the following lines to the scripts section:

"post-install-cmd": [ "@php patches/apply-patch.php" ], "post-update-cmd": [ "@php patches/apply-patch.php" ],



**Step 5: Apply the Patch**
- Run `composer install` or `composer update` to apply the patch.

This will apply the patch every time you run `composer install` or `composer update`, ensuring that the issue is fixed until this has been merged. Make sure to remove the patch after this packaged has been updated.
wpmvangroesen commented 1 year ago

Nice comment for laravel users. You can also patch this with composer-patches (if using composer).

Add this to your composer.json:

{
   "require": {
    "cweagans/composer-patches": "^1.7"
  },
  "extra": {
    "enable-patching": "true",
    "patches": {
      "phenx/php-font-lib": {
        "Fix crashing google fonts":
          "https://github.com/beerntea/php-font-lib/commit/3b586cc9e78bc5415e32e6f5e42e88e500cfd3d2.patch"
      }
    }
  }
}
hanifbd7 commented 11 months ago

Hello wpmvangroesen I did try with your code as bellow. But still getting this error "unpack(): Type n: not enough input, need 2, have 0" My code runs in php 8.2+ and Laravel 10.

 "minimum-stability": "dev",
    "prefer-stable": true,
    "composer-patches":{
        "require": {
         "cweagans/composer-patches": "^1.7"
       },
       "extra": {
         "enable-patching": "true",
         "patches": {
           "phenx/php-font-lib": {
             "Fix crashing google fonts":
               "https://github.com/beerntea/php-font-lib/commit/3b586cc9e78bc5415e32e6f5e42e88e500cfd3d2.patch"
           }
         }
       }
     }
wpmvangroesen commented 11 months ago

@hanifbd7 can you give some more details about the code? For ex what fonts are you using, what does your dompdf code look like, etc.

If we can reproduce it i can look into it!

bsweeney commented 11 months ago

128 addresses underlying causes of the unpack errors (at least, for those that have been identified). I'm not inclined to include this change since it just masks a parsing error in php-font-lib.

wpmvangroesen commented 11 months ago

Of course! Thanks for looking into this and fixing it. Do you have a timeline for the release of 0.5.5 ? :-)

bsweeney commented 11 months ago

Current expectation is "as soon as possible" ... I'm trying to finish up the issues slated for the release. Only a few more issues left to investigate, and then some prep work for the release.