Open
Conversation
Previously, the error handlers in IconvTranscoder and MbTranscoder were not restored on error because exception was raised. And what is worse, MbTranscoder popped the error handler stack even when it did not push into it. Let’s fix the former by restoring in a finally block (not catching the raised exception), and the latter by restoring only conditionally.
jtojnar
added a commit
to fossar/selfoss
that referenced
this pull request
Dec 16, 2020
Trying to access non-existent array fields is a warning in PHP 8. Though this error is masked by ddeboer/transcoder#5 in the integration tests when mbstring extension is available. Also, Dice uses `ReflectionParameter::getClass()`, which is deprecated in PHP 8. Since we have not yet set an error handler at that point because it needs a `Logger` class instantiated by Dice, let’s disable deprecation warnings temporarily. Unfortunately, we cannot just re-enable them and log them in the error handler since fatfree’s error handler is extremely inflexible bcosca/fatfree#999.
Author
|
This is an issue for selfoss so it would be nice if you could release transcoder 1.1.0 including this fix. (Ideally, the release would not change supported PHP versions since selfoss aims to support PHP 5.6 trough 8.0 – unfortunately, I still have not gotten around to releasing the 2.19 release, which is supposed to be the last to support PHP 5.6.) |
jtojnar
force-pushed
the
pair-error-handler-restoration
branch
from
2243d28 to
622114b
Compare
Author
|
@ddeboer Any chance of getting this merged? |
jtojnar
added a commit
to fossar/selfoss
that referenced
this pull request
May 6, 2021
The transcoder library contains a serious bug that removes selfoss’s error handlers. Since upstream is not responsive, we will switch to our fork for now: ddeboer/transcoder#5
Author
|
Since there has been no response, I forked the project: |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Previously, the error handlers in IconvTranscoder and MbTranscoder were not restored on error because exception was raised. And what is worse, MbTranscoder popped the error handler stack even when it did not push into it.
Let’s fix the former by restoring in a finally block (not catching the raised exception), and the latter by restoring only conditionally.