Description
Dobrica Pavlinusic
2019-10-02 19:31:27 UTC
Problem occurs only if we use exit inside eval block like: eval { warn "in eval"; exit; }; warn "after eval"; Under CGI, this would print just "in eval". Bacause of interaction of CGI::Compile which wraps system exit under plack we get both lines. Created attachment 93527 [details] [review] Bug 23723: using exit inside eval to stop sending output to browser doesn't work under plack When fixing Bug 23589 Theodoros Theodoropoulos noticed that we are sending headers and html after pdf output to browser. Using exit inside eval block doesn't stop plack from generating headers and html page after exit since CGI::Compile will catch exit but doesn't stop emiting output. Example is: eval { warn "in eval"; exit; }; warn "after eval"; Under CGI, this would print just "in eval", but under plack we get both lines and thus generate additional header and html after we already sent pdf data. I did check all places in code where we use eval and it seems that only place where we have also exit is this one, so situation is not as bad as I though initially. Created attachment 93541 [details] [review] Bug 23723: using exit inside eval to stop sending output to browser doesn't work under plack When fixing Bug 23589 Theodoros Theodoropoulos noticed that we are sending headers and html after pdf output to browser. Using exit inside eval block doesn't stop plack from generating headers and html page after exit since CGI::Compile will catch exit but doesn't stop emiting output. Example is: eval { warn "in eval"; exit; }; warn "after eval"; Under CGI, this would print just "in eval", but under plack we get both lines and thus generate additional header and html after we already sent pdf data. Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> Dobrica, in the patch you submitted, you tackle the issue only in opac-discharge.pl The same code exists also in discharge.pl (where it was originally discovered). Please include the fix for that as well in your patch in order to sign it off. Thanks in advance! Created attachment 93551 [details] [review] Bug 23723: using exit inside eval to stop sending output to browser doesn't work under plack Also fixes ./src/members/discharge.pl Extending the patch was trivial, so I gave it a go myself. Please check if it works for you too (it worked for me). Brendan, I'm changing status to "Needs Sign Off" (the original patch indeed tackled the problem, but not in all places). [I'm not a senior koha dev (like most of you), so please forgive me if I did something wrong. Full credits for identifying the real cause and fixing the problem go to Dobrica!] Created attachment 93552 [details] [review] Bug 23723: using exit inside eval to stop sending output to browser doesn't work under plack When fixing Bug 23589 Theodoros Theodoropoulos noticed that we are sending headers and html after pdf output to browser. Using exit inside eval block doesn't stop plack from generating headers and html page after exit since CGI::Compile will catch exit but doesn't stop emiting output. Example is: eval { warn "in eval"; exit; }; warn "after eval"; Under CGI, this would print just "in eval", but under plack we get both lines and thus generate additional header and html after we already sent pdf data. Current status: Needs Signoff Patch should include also commit message, so I replaced it with full version which you can now sign-off :-) Created attachment 93556 [details] [review] Bug 23723: using exit inside eval to stop sending output to browser doesn't work under plack When fixing Bug 23589 Theodoros Theodoropoulos noticed that we are sending headers and html after pdf output to browser. Using exit inside eval block doesn't stop plack from generating headers and html page after exit since CGI::Compile will catch exit but doesn't stop emiting output. Example is: eval { warn "in eval"; exit; }; warn "after eval"; Under CGI, this would print just "in eval", but under plack we get both lines and thus generate additional header and html after we already sent pdf data. Signed-off-by: Theodoros Theodoropoulos <theod@lib.auth.gr> I used ByWater Solutions' sandboxes because they seem to use Plack (required for the bug to appear). Seems to work OK, but QA may complain that there is no test plan for the patch. Because the bug relates only to code found in discharges, I would propose something like: 1/ make sure Plack is used in the system 2/ go to System Preferences and set useDischarge: Allow 3/ create a patron (or select a patron) and go to discharges -> generate discharge 4/ the produced file is not a proper PDF as is contains headers and html code in the end 5/ Apply patch (restart starman/apache ?) 6/ go to discharges-> generate discharge 7/ the bug is fixed and produced file is proper PDF (Something similar could be described for opac-discharge) ByWater Solutions' sandbox sign-off procedure did not change the status to Signed off... Doing it manually. Thx Dobrica (missing you here in Marseille) and Theodoros! I must be missing something - because my PDF look great without the patch :( Created attachment 93580 [details] [review] Bug 23723: using exit inside eval to stop sending output to browser doesn't work under plack When fixing Bug 23589 Theodoros Theodoropoulos noticed that we are sending headers and html after pdf output to browser. Using exit inside eval block doesn't stop plack from generating headers and html page after exit since CGI::Compile will catch exit but doesn't stop emiting output. Example is: eval { warn "in eval"; exit; }; warn "after eval"; Under CGI, this would print just "in eval", but under plack we get both lines and thus generate additional header and html after we already sent pdf data. Signed-off-by: Theodoros Theodoropoulos <theod@lib.auth.gr> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Katrin, try to open it in Notepad++ or vi (or another editor). You will see the headers and html at the end of the file. With the patch, the resulting file has no such garbage and it has 1/3 of the size In 18.11 (and earlier versions) the resulting file was not even readable by a PDF reader. For some reason in master the added garbage do not invalidate the produced PDF. Maybe has to do with some perl module update? Nice work! Pushed to master for 19.11.00 Pushed to 19.05.x for 19.05.05 |