Fix array offset error in FileUtils when the file doesn't exist #424
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.
Brief Summary of Changes
I ran into a very weird issue on my Laravel project. When the file doesn't exist, I get this error while trying to upload the file:
This is the culprit method:
cloudinary_php/src/Utils/FileUtils.php
Line 130 in 5b7d548
What happens is that
$err = error_get_last();on line 139 returnsNULL.This is weird because when I forked the project and added tests locally to check how it behaves with a non-existing file, it worked as expected.
I discovered that it's because of how Laravel handles error handling. It uses a custom error handler which intercepts all errors (including warnings), and PHP's built-in error system has no clue about what happened. In the end,
error_get_lastreturnsNULL.The library assumes
error_get_lastwill contain an array with amessagekey. But this is not the case in every PHP environment and framework.This PR adds a fallback to address this issue. It doesn't change default behavior.
How to reproduce?
Create a fresh Laravel app, install cloudinary php, add a cloudinary url, and try to upload a non-existing file.
I published a test app to show and test the behavior, here.
What does this PR address?
Are tests included?
Reviewer, please note:
Checklist: