Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Silent PHP error uploading a specific image #483

Closed
carrythebanner opened this issue Apr 25, 2023 · 3 comments
Closed

Silent PHP error uploading a specific image #483

carrythebanner opened this issue Apr 25, 2023 · 3 comments

Comments

@carrythebanner
Copy link
Collaborator

Create a new event, then try to upload the image below. You'll get the standard Your event has been updated! success message, and the manage_event request returns 200, but it's actually failing with a PHP error and the image is not saved to the event.

Fatal  error: Uncaught exception 'fValidationException' with message 
'The file  uploaded is not an image' in /opt/backend/vendor/flourish/fUpload.php  on line 339
--

1 | {main}(  )               | .../manage_event.php:0
2 | build_json_response(  )  | .../manage_event.php:314
3 | upload_attached_file(  ) | .../manage_event.php:266
4 | fUpload->move(  )        | .../manage_event.php:87

Munster

@carrythebanner carrythebanner changed the title PHP error uploading an image Silent PHP error uploading a specific image Apr 25, 2023
@ionous
Copy link
Contributor

ionous commented Apr 26, 2023

it looks like when the validation fail, the php tries to move (non existent) image anyway, the move throws.

        $file_message = $uploader->validate('file', TRUE);
        if ($file_message != null) {
            $messages = array('file' => $file_message);
        } // <--- should be an "else" here:
        global $IMAGEDIR;
        $file = $uploader->move($IMAGEDIR, 'file');  // < --- throws the exception
        $event->setImage($file->getName());

it seems ob_start captures exceptions... the exception text probably gets written into the response as plain text, but is ignored because the response was 200 ( and wouldnt be in the calendar's special error json format )

as for why the validation failed for the image, i'm not sure. maybe some jpg variant this version of php isn't familiar with?

ionous added a commit to ionous/shift-docs that referenced this issue Apr 26, 2023
( which stops the exception from throwing, and properly returns an 4xx error with error json as expected. )
ionous added a commit to ionous/shift-docs that referenced this issue Apr 26, 2023
@ionous ionous mentioned this issue Apr 26, 2023
carrythebanner added a commit that referenced this issue Apr 26, 2023
@carrythebanner
Copy link
Collaborator Author

#485 makes it so the error is no longer silent.

Not sure why this specific image failed — no obvious issues with it. I ran file on it in Terminal and nothing looked fishy. It worked fine for me when converted to PNG, or even just re-exported as a new JPG (using macOS Preview). Might be an odd JPG variant, or have some weird metadata, or something along those lines. The original reporter wasn't sure of the image provenance (grabbed it from the web somewhere) so I'm good to close this now that the error reports more visibly.

@ionous
Copy link
Contributor

ionous commented Apr 27, 2023

( probably related to this flourish issue:
flourishlib/flourish-classes#198
)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants