Bug 30803 - output_error should not assume a 404 status
Summary: output_error should not assume a 404 status
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: David Cook
QA Contact: Testopia
URL:
Keywords:
Depends on: 23329
Blocks: 41599
  Show dependency treegraph
 
Reported: 2022-05-19 06:47 UTC by Marcel de Rooy
Modified: 2026-01-13 09:15 UTC (History)
2 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: ---
Comma delimited list of Sponsors:
Crowdfunding goal: 0
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 30803: Don't hardcode 404 into output_error (1.40 KB, patch)
2026-01-13 02:01 UTC, David Cook
Details | Diff | Splinter Review
Bug 30803: Don't hardcode 404 into output_error (1.47 KB, patch)
2026-01-13 09:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30803: Add tests (1.69 KB, patch)
2026-01-13 09:14 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30803: Raise exception if $error is not passed (1.88 KB, patch)
2026-01-13 09:14 UTC, Jonathan Druart
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 2022-05-19 06:47:03 UTC
See comments on bug 30261.

> There is a fundamental flaw in output_error:
>     output_with_http_headers $query, $cookie, $template->output, 'html',
> '404 Not Found';
> Although we pass $error, it always outputs a 404. I tried sending a 403, and
> it does not work!
> 
> The 404 from output_error is caught by Plack middleware and it will output
> the 404 error document. In this case the OPAC 404 error document!
Comment 1 David Cook 2026-01-13 02:01:06 UTC
Created attachment 191332 [details] [review]
Bug 30803: Don't hardcode 404 into output_error

This patch removes the hardcoded "404 Not Found" status
code and reason phrase from output_error and instead
uses just the code passed into the function.

Test Plan:
0. Apply patch and koha-plack --restart kohadev
1. Go to http://localhost:8080/cgi-bin/koha/tracklinks.pl?uri=https%3A%2F%2Ftest.com&biblionumber=29
2. Note normal OPAC 404 error

Bonus points:
1. Change 404 to 403 in the first output_error in opac/tracklinks.pl
2. koha-plack --restart kohadev
3. Go to http://localhost:8080/cgi-bin/koha/tracklinks.pl?uri=https%3A%2F%2Ftest.com&biblionumber=29
4. Note normal OPAC 403 error
Comment 2 David Cook 2026-01-13 02:02:22 UTC
Adding a dependency on bug 41599 because this fix would break reports/acquisitions_stats.pl in an error condition it seems...
Comment 3 David Cook 2026-01-13 02:11:45 UTC
Actually, the thing with bug 41599 is a chicken and egg problem.

It looks like "Possible SQL injection" will return a 200 error code which is fine for now. So may as well make bug 41599 depend on this one...
Comment 4 Jonathan Druart 2026-01-13 09:14:30 UTC
Created attachment 191339 [details] [review]
Bug 30803: Don't hardcode 404 into output_error

This patch removes the hardcoded "404 Not Found" status
code and reason phrase from output_error and instead
uses just the code passed into the function.

Test Plan:
0. Apply patch and koha-plack --restart kohadev
1. Go to http://localhost:8080/cgi-bin/koha/tracklinks.pl?uri=https%3A%2F%2Ftest.com&biblionumber=29
2. Note normal OPAC 404 error

Bonus points:
1. Change 404 to 403 in the first output_error in opac/tracklinks.pl
2. koha-plack --restart kohadev
3. Go to http://localhost:8080/cgi-bin/koha/tracklinks.pl?uri=https%3A%2F%2Ftest.com&biblionumber=29
4. Note normal OPAC 403 error

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 5 Jonathan Druart 2026-01-13 09:14:31 UTC
Created attachment 191340 [details] [review]
Bug 30803: Add tests

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 6 Jonathan Druart 2026-01-13 09:14:32 UTC
Created attachment 191341 [details] [review]
Bug 30803: Raise exception if $error is not passed

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Jonathan Druart 2026-01-13 09:15:01 UTC
Bug 41599 must be pushed along with this one.