Bug 20564 - Error 500 displays when uploading patron images with a zipped file
Summary: Error 500 displays when uploading patron images with a zipped file
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: PC All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Josef Moravec
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-04-11 13:44 UTC by Lucy Vaux-Harvey
Modified: 2018-09-20 14:10 UTC (History)
14 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 20564: Don't use system to unzip files (1.77 KB, patch)
2018-07-10 13:49 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20564: Don't use system to unzip files (1.77 KB, patch)
2018-08-11 11:32 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 20564: Don't use system to unzip files (1.83 KB, patch)
2018-09-18 22:25 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 20564: Don't use system to unzip files (1.87 KB, patch)
2018-09-19 07:32 UTC, Josef Moravec
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucy Vaux-Harvey 2018-04-11 13:44:47 UTC
When uploading a zipped file of patron images an Error 500 message displays on screen at cgi-bin/koha/tools/picture-upload.pl.  The error displays regardless of whether the upload was successful or not and means that the user does not see the expected confirmation message.
Comment 1 Jonathan Druart 2018-04-11 17:31:39 UTC
Hi Lucy,
Which exact version of Koha are you using? Do you have access to the logs on the server?
Comment 2 Lucy Vaux-Harvey 2018-04-12 08:22:24 UTC
Version 17.11.03 and I do have access to server logs
Comment 3 Jonathan Druart 2018-04-12 16:24:06 UTC
Which error related to this problem do you see in the logs?
Comment 4 sunil 2018-05-02 09:43:11 UTC
Similar error faced by me and some other users also in past, See: https://lists.katipo.co.nz/public/koha/2017-July/048523.html

I think this error comes after a security update of apache web server: https://serverfault.com/questions/853103/500-internal-server-ah02429-response-header-name
Comment 5 Jonathan Druart 2018-05-03 20:43:04 UTC
Lucy, which version of Apache are you using?
Comment 6 Lucy Vaux-Harvey 2018-05-08 11:01:31 UTC
Hi Jonathan, thanks for taking a look.  I've re-tested but can't reproduce the error on the 17.11.03 site.

However, I do still get the error for another site where this was reported:
Version 16.11.03
Apache/2.4.10 (Debian)
Error logged as: 
[Tue May 08 11:46:42.647237 2018] [http:error] [pid 21812:tid 140624423646976] [client 2.25.8.236:60806] AH02429: Response header name '  inflating' contains invalid characters, aborting request, referer: https://bcol-staff.koha-ptfs.co.uk/cgi-bin/koha/tools/picture-upload.pl
Comment 7 Eugene Espinoza 2018-05-19 07:30:44 UTC
Hi all! Getting also the same error when batch uploading patron images. However, when I enabled plack in the server, the images were uploaded successfully.
Comment 8 Indranil Das Gupta 2018-05-22 00:28:41 UTC
Confirming that the problem persists on a package installed 17.11.05 (non-plack) with Apache 2.4.10.
Comment 9 Jonathan Druart 2018-05-22 14:36:25 UTC
Then install plack :)
Comment 10 Kyle M Hall 2018-07-10 13:04:36 UTC
(In reply to Jonathan Druart from comment #9)
> Then install plack :)

The script in question does not run under plack.
Comment 11 Nick Clemens 2018-07-10 13:41:49 UTC
The issue seems to be the call to system():

tools/upload-cover-image.pl:
107         unless ( system( "unzip", $filename, '-d', $dirname ) == 0 ) {
108             $error = 'UZIPFAIL';
109         }

This prints the reults to STDOUT, that is where the '   inflating' that upsets apache comes from
Comment 12 Nick Clemens 2018-07-10 13:49:59 UTC
Created attachment 76807 [details] [review]
Bug 20564: Don't use system to unzip files

To test:
1 - Setup a zip file for patron images and cover images
2 - Test each tool
3 - Get a '500 error' after upload (note images do upload and attach)
4 - Apply patch
5 - Restart all the things
6 - Test agtain, uploads should complete and results display
Comment 13 Lucy Vaux-Harvey 2018-07-12 15:43:29 UTC
I've tried testing this patch on one of the sandboxes but can't access the Upload patron images tool at http://pro.user05-koha.sandbox.biblibre.eu/cgi-bin/koha/tools/picture-upload.pl
Software error is:
Global symbol "$filename" requires explicit package name (did you forget to declare "my $filename"?) at /home/koha/src/tools/picture-upload.pl line 118.
Execution of /home/koha/src/tools/picture-upload.pl aborted due to compilation errors.
Comment 15 Saiful Amin 2018-08-11 05:09:02 UTC
(In reply to Nick Clemens from comment #12)
> Created attachment 76807 [details] [review] [review]
> Bug 20564: Don't use system to unzip files
> 
> To test:
> 1 - Setup a zip file for patron images and cover images
> 2 - Test each tool
> 3 - Get a '500 error' after upload (note images do upload and attach)
> 4 - Apply patch
> 5 - Restart all the things
> 6 - Test agtain, uploads should complete and results display

This is great patch, except for a typo.

It worked only after I changed this line in tools/picture-upload.pl [17.11.5]:
    qx/unzip $filename -d $dirname/;

with this:
    qx/unzip $tempfile -d $dirname/;

The patch can be modified accordingly before it can be signed off.
Comment 16 Nick Clemens 2018-08-11 11:32:22 UTC
Created attachment 77725 [details] [review]
Bug 20564: Don't use system to unzip files

To test:
1 - Setup a zip file for patron images and cover images
2 - Test each tool
3 - Get a '500 error' after upload (note images do upload and attach)
4 - Apply patch
5 - Restart all the things
6 - Test agtain, uploads should complete and results display
Comment 17 Nick Clemens 2018-08-11 11:33:05 UTC
(In reply to Saiful Amin from comment #15)
 
> This is great patch, except for a typo.
Oh, ha ha, whoops

Silly copy paste error

Patch updated, thank you
Comment 18 ByWater Sandboxes 2018-09-18 22:25:41 UTC
Created attachment 79068 [details] [review]
Bug 20564: Don't use system to unzip files

To test:
1 - Setup a zip file for patron images and cover images
2 - Test each tool
3 - Get a '500 error' after upload (note images do upload and attach)
4 - Apply patch
5 - Restart all the things
6 - Test agtain, uploads should complete and results display

Signed-off-by: George Williams <george@nekls.org>
Comment 19 M. Tompsett 2018-09-19 02:31:08 UTC
Changing to signed off, since it didn't get marked as such, even though it is clear that it was.
Comment 20 Josef Moravec 2018-09-19 07:32:37 UTC
Created attachment 79072 [details] [review]
Bug 20564: Don't use system to unzip files

To test:
1 - Setup a zip file for patron images and cover images
2 - Test each tool
3 - Get a '500 error' after upload (note images do upload and attach)
4 - Apply patch
5 - Restart all the things
6 - Test agtain, uploads should complete and results display

Signed-off-by: George Williams <george@nekls.org>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 21 Nick Clemens 2018-09-19 14:39:06 UTC
Awesome work all!

Pushed to master for 18.11
Comment 22 Martin Renvoize 2018-09-20 11:07:34 UTC
Pushed to 18.05.x for 18.05.04
Comment 23 Fridolin SOMERS 2018-09-20 14:10:59 UTC
Pushed to 17.11.x for 17.11.10