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.73k stars 256 forks source link

PHP 7.4 E_NOTICE on read error #78

Closed Piskvor closed 4 years ago

Piskvor commented 4 years ago

After upgrade to 7.4, this ErrorException started popping up: Notice: fread(): read of 8192 bytes failed with errno=9 Bad file descriptor Apparently this is handled in library, but the emitted E_NOTICE, converted to ErrorException by a global handler, propagates into app and crashes it.

Workaround: Mute the E_NOTICE in #77 , building upon #76.

Shortened trace:


    0 => [
        'file' => '.../vendor/phenx/php-font-lib/src/FontLib/Table/DirectoryEntry.php',
        'line' => 92,
        'function' => 'read',
        'class' => "FontLib\BinaryStream",
        'type' => '->',
    ],
    1 => [
        'file' => '.../vendor/phenx/php-font-lib/src/FontLib/TrueType/File.php',
        'line' => 251,
        'function' => 'encode',
        'class' => "FontLib\Table\DirectoryEntry",
        'type' => '->',
    ],
    2 => [
        'file' => '.../vendor/dompdf/dompdf/lib/Cpdf.php',
        'line' => 2839,
        'function' => 'encode',
        'class' => "FontLib\TrueType\File",
        'type' => '->',
    ],
    '...' => [],
];
enumag commented 4 years ago

I checked this a bit deeper and I don't think ignoring the error is the right solution here. The bad file descriptor issue happens when you try to read from a file which was opened in write-mode. In my case it opened /tmp/Lato-Regular.ttf.tmp.5ec769b518ba4 with mode "wb" and then tried to read from this file.

Relevant part of the stack trace:

      ./vendor/phenx/php-font-lib/src/FontLib/BinaryStream.php:149 { …}
      ./vendor/phenx/php-font-lib/src/FontLib/Table/DirectoryEntry.php:92 { …}
      ./vendor/phenx/php-font-lib/src/FontLib/TrueType/File.php:251 { …}
      ./vendor/dompdf/dompdf/lib/Cpdf.php:2839 { …}
      ./vendor/dompdf/dompdf/src/Adapter/CPDF.php:1006 { …}
      ./vendor/dompdf/dompdf/src/FontMetrics.php:310 { …}
      ./vendor/dompdf/dompdf/src/FrameReflower/Text.php:197 { …}
      ./vendor/dompdf/dompdf/src/FrameReflower/Text.php:372 { …}
      ./vendor/dompdf/dompdf/src/FrameDecorator/AbstractFrameDecorator.php:895 { …}
      ./vendor/dompdf/dompdf/src/FrameReflower/Block.php:845 { …}
      ./vendor/dompdf/dompdf/src/FrameDecorator/AbstractFrameDecorator.php:895 { …}
      ./vendor/dompdf/dompdf/src/FrameReflower/Page.php:141 { …}
      ./vendor/dompdf/dompdf/src/FrameDecorator/AbstractFrameDecorator.php:895 { …}
      ./vendor/dompdf/dompdf/src/Dompdf.php:847 { …}
enumag commented 4 years ago

Ah apparently it has been fixed already: https://github.com/dompdf/dompdf/commit/9a4d215c6ab8823908ebe7b17cc12ffea5cd07d5

bsweeney commented 4 years ago

Appreciate the update @enumag. This does seem to be a usage-based issue and not necessarily a but to fix in php-font-lib. The method naming is perhaps a bit unfortunate since the difference between load and open aren't immediately obvious unless you're reading the source. But I don't think that negates the usefulness of the notice.

I think the update to Dompdf should sufficiently address the issue for most use cases so I'm not inclined to remove the notices.