cconard96 / glpi-screenshot-plugin

Take a screenshot or screen recording directly from GLPI and attach it to a ticket, change or problem
GNU General Public License v2.0
5 stars 0 forks source link

Doesnt Work with 9.5.3 #12

Closed chichtacos closed 3 years ago

chichtacos commented 3 years ago

Hi,

I just install the plugin on my GLPI 9.5.3. All works fine until the "Upload" button.

When I click on it, nothing appens and the screenshot is gone. I'm using Google Chrome last version FYI.

Thx a lot !

cconard96 commented 3 years ago

Can you check if there are any related errors in files/_log/php-errors.log?

chichtacos commented 3 years ago

Yep ! Just made a new test :

[2020-12-04 14:01:02] glpiphplog.WARNING: *** PHP Warning (2): fopen(C:\xampp\htdocs\glpi-preprod/files/_tmp/Screenshot 2020-12-04 14:01:01-3A66C.jpg): failed to open stream: No such file or directory in C:\xampp\htdocs\glpi-preprod\plugins\screenshot\ajax\screenshot.php at line 65 Backtrace : plugins\screenshot\ajax\screenshot.php:65 fopen()

[2020-12-04 14:01:02] glpiphplog.WARNING: *** PHP Warning (2): fwrite() expects parameter 1 to be resource, bool given in C:\xampp\htdocs\glpi-preprod\plugins\screenshot\ajax\screenshot.php at line 66 Backtrace : plugins\screenshot\ajax\screenshot.php:66 fwrite()

[2020-12-04 14:01:02] glpiphplog.WARNING: *** PHP Warning (2): fclose() expects parameter 1 to be resource, bool given in C:\xampp\htdocs\glpi-preprod\plugins\screenshot\ajax\screenshot.php at line 67 Backtrace : plugins\screenshot\ajax\screenshot.php:67 fclose()

frederic-dumont commented 3 years ago

Did you try to change the screenshot format here : Home > Setup > General > Screenshot > Screenshot Format set to JPG in place of PNG

chichtacos commented 3 years ago

Yes it's already set in JPEG.

cconard96 commented 3 years ago

That's odd. It looks like it is failing to save the initial upload before checking the file type and moving it to the correct file directory. Can you verify the permissions on the _tmp folder, and test a manual upload of an image/file?

chichtacos commented 3 years ago

No error with a manual upload (.jpg file) : image

And it's a brand new glpi install with default rights :)

cconard96 commented 3 years ago

Could you try making the following changes and try another screenshot and see if it works or if you get an error popup?

diff --git a/ajax/screenshot.php b/ajax/screenshot.php
index 73ba909..364b8f1 100644
--- a/ajax/screenshot.php
+++ b/ajax/screenshot.php
@@ -62,9 +62,15 @@ if (isset($_POST['img'])) {

    $data = file_get_contents($_POST['img']);
    // Save image to tmp
-   $file = fopen(GLPI_TMP_DIR . '/' . $file_name, 'wb');
-   fwrite($file, $data);
-   fclose($file);
+   if (!is_writable(GLPI_TMP_DIR)) {
+      Session::addMessageAfterRedirect('GLPI temp folder is not writable', false, ERROR);
+      die(400);
+   }
+   $bytes_written = file_put_contents(GLPI_TMP_DIR . '/' . $file_name, $data);
+   if ($bytes_written === false) {
+      Session::addMessageAfterRedirect('Screenshot upload failed', false, ERROR);
+      die(400);
+   }

    // Add document and link. Adding the document will cleanup the temp file for us.
    $doc = new Document();
chichtacos commented 3 years ago

Hi !

I've got the "Screenshot upload failed" message.

chichtacos commented 3 years ago

Hi,

Just Solved my issue by changing this line in ajax\screenshot.php :

Change 57 : $file_name = 'Screenshot ' . $_SESSION['glpi_currenttime'] . '-' . sprintf('%05X', random_int(0, 1048575)) . '.' . $ext; By 57 : $file_name = 'Screenshot ' . '-' . sprintf('%05X', random_int(0, 1048575)) . '.' . $ext;

I figured out that $_SESSION['glpi_currenttime'] display ":" characters in the file name. And Windows doesn't like those characters.

If it can help you improve your script :)

cconard96 commented 3 years ago

@chichtacos Thank you for that information. I typically use Linux when developing so I wasn't aware of that restriction. I'll make a patch for that later today.