Bug 19439 - Some error responses from opac/unapi get lost in eval
Summary: Some error responses from opac/unapi get lost in eval
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Marcel de Rooy
QA Contact: Josef Moravec
URL:
Keywords: Academy
Depends on: 17797
Blocks:
  Show dependency treegraph
 
Reported: 2017-10-09 13:13 UTC by Marcel de Rooy
Modified: 2019-06-27 09:24 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 19439: Remove eval with exit statements from opac/unapi (2.49 KB, patch)
2017-10-10 13:55 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19439: (Follow-up) Remove four spaces from lines 133-149 (2.08 KB, patch)
2017-10-10 13:55 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19439: Remove eval with exit statements from opac/unapi (2.51 KB, patch)
2017-10-15 08:29 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19439: (Follow-up) Remove four spaces from lines 133-149 (2.09 KB, patch)
2017-10-15 08:29 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 19439: (Follow-up) Remove four spaces from lines 133-149 (2.15 KB, patch)
2017-12-08 06:33 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19439: Remove eval with exit statements from opac/unapi (2.56 KB, patch)
2017-12-08 06:34 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 19439: (Follow-up) Remove four spaces from lines 133-149 (2.15 KB, patch)
2017-12-08 06:34 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 Marcel de Rooy 2017-10-09 13:13:34 UTC

    
Comment 1 Marcel de Rooy 2017-10-10 13:55:20 UTC
Created attachment 67872 [details] [review]
Bug 19439: Remove eval with exit statements from opac/unapi

The script contains some error responses that get lost when using them
in an eval statement. (Since exit should not be used within an eval
construction.) The eval is removed now.

Test plan:
[1] Before applying this patch, you could look at the current responses
    from unapi for the calls in the next steps. Apply the patch now.
[2] Call http(s)://[your-server]/cgi-bin/koha/unapi with only a format
    parameter.
    Check if you have a 400 status response. (In Firefox, look at Network
    tab of Developer Tools.)
    Note: Have to admit (reluctantly) that MS Edge shows the status code in
    the browser rightaway in contrast with Firefox.
[3] Call unapi with a format=marcxml&id=999
    The wrong id parameter should trigger a 404 response.
[4] Call unapi with format=marcxml&id=koha:biblionumber:[notexist]
    where notexist is a biblionumber that not exists.
    This should trigger again a 404 response.
[5] Call unapi with format=marcxmlx&id=koha:biblionumber:[exist]
    where exist should be a good biblionumber.
    The wrong format should trigger a 406 response.
[6] Bonus: The 500 response can be tested by manipulating a XSLT file.
    Create invalid xml in the file for marcxml (identity.xsl in intranet
    xslt folder).
    Call unapi with format=marcxml&id=koha:biblionumber:[exist].
    You should get a 500 response and have warnings in your logfile.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Marcel de Rooy 2017-10-10 13:55:25 UTC
Created attachment 67873 [details] [review]
Bug 19439: (Follow-up) Remove four spaces from lines 133-149

Speaks for itself. No test plan.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 3 Katrin Fischer 2017-10-15 08:29:41 UTC
Created attachment 68157 [details] [review]
Bug 19439: Remove eval with exit statements from opac/unapi

The script contains some error responses that get lost when using them
in an eval statement. (Since exit should not be used within an eval
construction.) The eval is removed now.

Test plan:
[1] Before applying this patch, you could look at the current responses
    from unapi for the calls in the next steps. Apply the patch now.
[2] Call http(s)://[your-server]/cgi-bin/koha/unapi with only a format
    parameter.
    Check if you have a 400 status response. (In Firefox, look at Network
    tab of Developer Tools.)
    Note: Have to admit (reluctantly) that MS Edge shows the status code in
    the browser rightaway in contrast with Firefox.
[3] Call unapi with a format=marcxml&id=999
    The wrong id parameter should trigger a 404 response.
[4] Call unapi with format=marcxml&id=koha:biblionumber:[notexist]
    where notexist is a biblionumber that not exists.
    This should trigger again a 404 response.
[5] Call unapi with format=marcxmlx&id=koha:biblionumber:[exist]
    where exist should be a good biblionumber.
    The wrong format should trigger a 406 response.
[6] Bonus: The 500 response can be tested by manipulating a XSLT file.
    Create invalid xml in the file for marcxml (identity.xsl in intranet
    xslt folder).
    Call unapi with format=marcxml&id=koha:biblionumber:[exist].
    You should get a 500 response and have warnings in your logfile.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 4 Katrin Fischer 2017-10-15 08:29:44 UTC
Created attachment 68158 [details] [review]
Bug 19439: (Follow-up) Remove four spaces from lines 133-149

Speaks for itself. No test plan.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 Katrin Fischer 2017-10-15 08:30:25 UTC
Learned something new about what Koha can do - nice!
Comment 6 Marcel de Rooy 2017-11-10 11:01:35 UTC
Jonathan: Would you consider this one as a bugfix too ?
Comment 7 Jonathan Druart 2017-11-26 18:19:47 UTC
Yes, but too late for 17.11.00 now.
Comment 8 Josef Moravec 2017-12-08 06:33:47 UTC
Created attachment 69623 [details] [review]
Bug 19439: (Follow-up) Remove four spaces from lines 133-149

Speaks for itself. No test plan.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 9 Josef Moravec 2017-12-08 06:34:16 UTC
Created attachment 69624 [details] [review]
Bug 19439: Remove eval with exit statements from opac/unapi

The script contains some error responses that get lost when using them
in an eval statement. (Since exit should not be used within an eval
construction.) The eval is removed now.

Test plan:
[1] Before applying this patch, you could look at the current responses
    from unapi for the calls in the next steps. Apply the patch now.
[2] Call http(s)://[your-server]/cgi-bin/koha/unapi with only a format
    parameter.
    Check if you have a 400 status response. (In Firefox, look at Network
    tab of Developer Tools.)
    Note: Have to admit (reluctantly) that MS Edge shows the status code in
    the browser rightaway in contrast with Firefox.
[3] Call unapi with a format=marcxml&id=999
    The wrong id parameter should trigger a 404 response.
[4] Call unapi with format=marcxml&id=koha:biblionumber:[notexist]
    where notexist is a biblionumber that not exists.
    This should trigger again a 404 response.
[5] Call unapi with format=marcxmlx&id=koha:biblionumber:[exist]
    where exist should be a good biblionumber.
    The wrong format should trigger a 406 response.
[6] Bonus: The 500 response can be tested by manipulating a XSLT file.
    Create invalid xml in the file for marcxml (identity.xsl in intranet
    xslt folder).
    Call unapi with format=marcxml&id=koha:biblionumber:[exist].
    You should get a 500 response and have warnings in your logfile.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 10 Josef Moravec 2017-12-08 06:34:35 UTC
Created attachment 69625 [details] [review]
Bug 19439: (Follow-up) Remove four spaces from lines 133-149

Speaks for itself. No test plan.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 11 Marcel de Rooy 2017-12-08 06:40:48 UTC
(In reply to Josef Moravec from comment #10)
> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Thx Josef. Welcome in the QA team !
Comment 12 Jonathan Druart 2017-12-11 14:35:44 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 13 Nick Clemens (kidclamp) 2017-12-14 18:18:09 UTC
Pushed to stable for 17.11.01, awesome work all!
Comment 14 Fridolin Somers 2017-12-21 10:05:48 UTC
Depends on Bug 17797 not in 17.05.x