Fix chr() deprecation in ASCII85 last-tuple decoding#793
Fix chr() deprecation in ASCII85 last-tuple decoding#793k00ni merged 2 commits intosmalot:masterfrom
Conversation
PR smalot#779 fixed the chr() deprecation warning in the main decoding loop but missed the "last tuple" switch/case block (lines 233-248) which handles incomplete groups at the end of the encoded data. The same issue applies: bit-shifted values ($tuple >> 16, $tuple >> 8) can exceed 255, triggering: chr(): Providing a value not in-between 0 and 255 is deprecated Apply `& 0xFF` bitmask to extract individual bytes, consistent with the fix in the main loop.
|
@Ciki Thank you for the PR. If I remember correctly, the changes in #779 where suggested somewhere to fix PHP 8.5 related deprecations. I have limited knowledge in this area so I had to check your information. The following two pages describe bit-wise operations quite a bit:
Code looks good. Could you provide one of the PDFs which triggered these so we can use it in our test suite? I will keep this open for a few days in case someone else wants to comment. Or do you need it merged right away? |
|
@k00ni Thanks for the review! The original PDF that triggered this contained client data, so I generated a minimal reproduction PDF instead. File: ascii85-last-tuple-overflow.pdf It's a valid single-page PDF (657 bytes) with one ASCII85Decode-filtered content stream. The stream is crafted so that the last ASCII85 group (4-char incomplete tuple) produces values exceeding 255 when bit-shifted:
With the fix ( No rush on merging — take your time. We're running a fork in the meantime. |
…o PDF Demo PDF would trigger the deprecation warnings without the fixes of this PR.
|
@Ciki I added your PDF to the test samples and a mini test which loads it. Without your fixes one would see the deprecations. There were a few warnings (at least in PHP 8.5, example): Those are not because of your changes as far as I can tell, therefore ignored for now. Thank you for taking the time preparing the pull request. It was a joy not having to ask for every detail but to get it prepared and on point. |
Problem
PR #779 fixed the
chr()deprecation warning in the main ASCII85 decoding loop but missed the "last tuple" switch/case block (handling incomplete groups at the end of encoded data).The same issue applies — bit-shifted values (
$tuple >> 16,$tuple >> 8) can exceed 255, triggering:Observed in production when parsing PDFs with ASCII85-encoded streams that have a partial trailing group.
Fix
Apply
& 0xFFbitmask to extract individual bytes before passing tochr(), consistent with the approach used in the main loop fix from #779.& 0xFFis the standard bitwise idiom for extracting a single byte from a multi-byte integer (equivalent to% 256but more idiomatic).Changes
Same pattern applied to
case 3andcase 2.