Fix for two bugs related to Unicode translation support by Font objects #667
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Symptom was that some documents' contents was rendering as a bunch of
control characters. These are the untranslated strings. This was
happening because for two different reasons, these strings weren't being
translated \Smalot\PdfParser\Font::decodeContent() in some circumstances.
First fix is to \Smalot\PdfParser\Font::loadTranslateTable():
- Fixed bug where bfchar sections weren't loaded due to mistake in regexp.
- It now uses `*` instead of `+` and thus supports translation tables with
lines like `<0000><0000>`. (Required `<0000> <0000>` before.)
Second fix is for documents that attach their Font objects to the Pages
object instead of each Page object:
- \Smalot\PdfParser\Page now has a setFonts() method
- \Smalot\PdfParser\Pages now declares its $fonts variable
- \Smalot\PdfParser\Pages::getPages() now applies the object's fonts to each child Page
- \Smalot\PdfParser\Pages::getFonts() copied from Page class
Changed \Smalot\PdfParser\RawData\RawDataParser::getHeaderValue() to be a static method. \Smalot\PdfParser\RawData\RawDataParser::decodeStream() now has an optional additional parameter $objRefArr (passed by getIndirectObject()). Added \Smalot\PdfParser\RawData\DataHelper /home/alastair/src/unixnut_pdfparser (git): diff --git c/src/Smalot/PdfParser/RawData/DataHelper.php i/src/Smalot/PdfParser/RawData/DataHelper.php new file mode 100644 index 0000000..2f5d42f --- /dev/null +++ i/src/Smalot/PdfParser/RawData/DataHelper.php @@ -0,0 +1,64 @@ +<?php + +/** + * This file is based on code of tecnickcom/TCPDF PDF library. + * + * Original author Nicola Asuni ([email protected]) and + * contributors (https://github.com/tecnickcom/TCPDF/graphs/contributors). + * + * @see https://github.com/tecnickcom/TCPDF + * + * Original code was licensed on the terms of the LGPL v3. + * + * ------------------------------------------------------------------------------ + * + * @file This file is part of the PdfParser library. + * + * @author Alastair Irvine <[email protected]> + * + * @Date 2024-01-12 + * + * @license LGPLv3 + * + * @url <https://github.com/smalot/pdfparser> + * + * PdfParser is a pdf library written in PHP, extraction oriented. + * Copyright (C) 2017 - Sébastien MALOT <[email protected]> + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. + * If not, see <http://www.pdfparser.org/sites/default/LICENSE.txt>. + */ + +namespace Smalot\PdfParser\RawData; + +class DataHelper +{ + /* + * Decode a string of the form "15_0" into an array. + * + * @param string $objRef The object ID + * @return array object number and generation + * + * @throws \Exception if @p $objRef is invalid + */ + public static function decodeRef(string $objRef): array + { + $objRefArr = \explode('_', $objRef); + if (2 !== \count($objRefArr)) { + throw new \Exception('Invalid object reference for $obj.'); + } + + return $objRefArr; + } +} diff --git c/src/Smalot/PdfParser/RawData/RawDataParser.php i/src/Smalot/PdfParser/RawData/RawDataParser.php index 7763089..433a67a 100644 --- c/src/Smalot/PdfParser/RawData/RawDataParser.php +++ i/src/Smalot/PdfParser/RawData/RawDataParser.php @@ -87,7 +87,7 @@ class RawDataParser * * @throws \Exception */ - protected function decodeStream(string $pdfData, array $xref, array $sdic, string $stream): array + protected function decodeStream(string $pdfData, array $xref, array $sdic, string $stream, array $objRefArr = null): array { // get stream length and filters $slength = \strlen($stream); @@ -524,10 +524,7 @@ class RawDataParser * build indirect object header */ // $objHeader = "[object number] [generation number] obj" - $objRefArr = explode('_', $objRef); - if (2 !== \count($objRefArr)) { - throw new \Exception('Invalid object reference for $obj.'); - } + $objRefArr = DataHelper::decodeRef($objRef); $objHeaderLen = $this->getObjectHeaderLen($objRefArr); @@ -558,7 +555,7 @@ class RawDataParser $offset = $element[2]; // decode stream using stream's dictionary information if ($decoding && ('stream' === $element[0]) && null != $header) { - $element[3] = $this->decodeStream($pdfData, $xref, $header[1], $element[1]); + $element[3] = $this->decodeStream($pdfData, $xref, $header[1], $element[1], $objRefArr); } $objContentArr[$i] = $element; $header = isset($element[0]) && '<<' === $element[0] ? $element : null; @@ -760,8 +757,8 @@ class RawDataParser $offset += \strlen($matches[0]); // we get stream length here to later help preg_match test less data - $streamLen = (int) $this->getHeaderValue($headerDic, 'Length', 'numeric', 0); - $skip = false === $this->config->getRetainImageContent() && 'XObject' == $this->getHeaderValue($headerDic, 'Type', '/') && 'Image' == $this->getHeaderValue($headerDic, 'Subtype', '/'); + $streamLen = (int) self::getHeaderValue($headerDic, 'Length', 'numeric', 0); + $skip = false === $this->config->getRetainImageContent() && 'XObject' == self::getHeaderValue($headerDic, 'Type', '/') && 'Image' == self::getHeaderValue($headerDic, 'Subtype', '/'); $pregResult = preg_match( '/(endstream)[\x09\x0a\x0c\x0d\x20]/isU', @@ -814,7 +811,7 @@ class RawDataParser * * @return string|array|null value of obj header's section, or default value if none found, or its type doesn't match $type param */ - private function getHeaderValue(?array $headerDic, string $key, string $type, $default = '') + public static function getHeaderValue(?array $headerDic, string $key, string $type, $default = '') { if (false === \is_array($headerDic)) { return $default;
\Smalot\PdfParser\RawData\RawDataParser::getRawObject() now uses (almost) correct quoting semantics. Does not yet support octal elements, e.g. `\037` (see \Smalot\PdfParser\Font::decodeOctal()) Note that \Smalot\PdfParser\Page::extractDecodedRawData() still needs fixing. Both functions should probably use a common helper function.
Relies on three primary classes:
- \Smalot\PdfParser\Encryption\Info
- \Smalot\PdfParser\Encryption\FileKey
- \Smalot\PdfParser\Encryption\Stream
This namespace also has a number of exception classes.
\Smalot\PdfParser\RawData\RawDataParser interacts with decryption
support via:
- $decryptionHelper variable
- decodeStream() now handles decryption after header processing and
before the object filters are invoked
- parseData() calls setupDecryption() if the document has an Encrypt object
Also added is \Smalot\PdfParser\Utils with a number of static utility
methods for binary string processing, etc.
k00ni
reviewed
Jan 19, 2024
There was a problem hiding this comment.
@unixnut Thank your for this rather big pull request!
First of all, I am super busy currently, so my answers are delayed. Nevertheless, I will try to help to get this merged. Maybe @j0k3r, @GreyWyvern or someone else wants to help out here.
Here are my first questions:
- Can we split this PR into smaller chunks which are easier to review? We are talking about 1.5k new lines.
- Do you plan to add unit tests to cover your new changes?
- Your code is about decryption and encryption, but in the title its about Unicode translation. Please elaborate on that a bit.
- You mention two bugs this PR fixes. Please reference them.
I am looking forward to your response.
|
I'm on vacation atm, but I can take a look at this on Monday. 👍 |
After a first pass of the code, this is what I'd like to know as well. I feel like for this many lines, the description as to what this PR does needs to be much much longer! :D What's the purpose of this encryption/decryption code? Does it supercede the previous PR #653? I will keep studying it. |
|
Please merge in master branch, to get rid of these coding style issues. |
|
@k00ni @GreyWyvern my mistake, I forgot GitHub consumes all commits on a branch for a PR. This PR is supposed to be for the first commit only |
Proceed as you see fit. I assume either close this one and create another PR with your changes or simply remove all additional changes in this PR and force push in order to update it. |
|
I think I would probably cancel this PR and start a clean one, but that's just me. :) |
|
Please open new PRs with smaller sets of code changes. Thanks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Symptom was that some documents' contents was rendering as a bunch of control characters. These are the untranslated strings. This was happening because for two different reasons, these strings weren't being translated \Smalot\PdfParser\Font::decodeContent() in some circumstances.
First fix is to \Smalot\PdfParser\Font::loadTranslateTable():
*instead of+and thus supports translation tables with lines like<0000><0000>. (Required<0000> <0000>before.)Second fix is for documents that attach their Font objects to the Pages object instead of each Page object:
Type of pull request
About
Checklist for code / configuration changes
In case you changed the code/configuration, please read each of the following checkboxes as they contain valuable information:
By the way, you don't have to provide a full fledged PDF file to demonstrate a fix. Instead a unit test may be sufficient sometimes,
please have a look at FontTest for example code.
Code changes without any tests are likely to be rejected. If you dont know how to write tests, no problem, tell us upfront and we may add them ourselves or discuss other ways.
fixes #1234to outline that you are providing a fix for the issue#1234.(see here for more information).
This will tell us, that it is not ready for merge, but you want to discuss certain issues.