Bug 6747 - Additional check in opac-export
Summary: Additional check in opac-export
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: PATCH-Sent (DO NOT USE) minor
Assignee: Marcel de Rooy
QA Contact: Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-18 10:41 UTC by Marcel de Rooy
Modified: 2012-10-25 23:09 UTC (History)
2 users (show)

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

Attachments
Patch (1.17 KB, patch)
2011-08-18 10:44 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Revised patch (3.75 KB, patch)
2011-08-22 12:25 UTC, Marcel de Rooy 		Details | Diff | Splinter Review
Signed-off patch (3.80 KB, patch)
2011-08-24 16:31 UTC, Owen Leonard 		Details | Diff | Splinter Review
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2011-08-18 10:41:15 UTC 
If GetMarcBiblio function returns undef, the opac-export script does not handle it properly.
Trivial patch sent.
Comment 1 Marcel de Rooy 2011-08-18 10:44:31 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2011-08-18 10:49:44 UTC 
Simple test: 
Run <yourserver>/cgi-bin/koha/opac-export.pl?format=utf8&op=export&bib=99999999 with the bib number not existing in your db (without this patch). 
You will get a 500 error.
Apply the patch and do it again.
Do you get the error message Record not found in your browser now?
Comment 3 Owen Leonard 2011-08-18 14:40:27 UTC 
It's hard to imagine how this might happen in the real world, but all error-checking is a good thing. What I'm wondering is whether we should redirect to the 404 page instead of sending a plain error page?
Comment 4 Marcel de Rooy 2011-08-22 12:24:37 UTC 
(In reply to comment #3)
> It's hard to imagine how this might happen in the real world, but all
> error-checking is a good thing. What I'm wondering is whether we should
> redirect to the 404 page instead of sending a plain error page?

Found several 500 errors on deleted records; they are generated by Googlebot. 

Sending revised patch with 404 and some additional revisions now. While testing, I noticed that formats ris and endnote did not generate something useful. But that is not the point of this report..
Comment 5 Marcel de Rooy 2011-08-22 12:25:46 UTC Comment hidden (obsolete)
Comment 6 Owen Leonard 2011-08-24 16:31:30 UTC 
Created attachment 5120 [details] [review]
Signed-off patch

Thanks for making that change. I think it's more consistent to use the existing 404 error.
Comment 7 Ian Walls 2011-08-25 17:26:20 UTC 
Mostly spacing changes, with some default values added to potentially undefined values.  Marking as Passed QA.
Comment 8 Chris Cormack 2011-08-26 19:18:33 UTC 
Pushed to master, please test
Comment 9 Chris Nighswonger 2011-10-26 18:02:27 UTC 
The fix for this bug was published in the 3.4.5 release. If you were the reporter of this bug, please take time to verify the fix and update the status of this bug report accordingly. If the bug is fixed to your satisfaction, please close this report.