Summary: | opac/tracklinks.pl renders 404 incorrectly | ||
---|---|---|---|
Product: | Koha | Reporter: | David Cook <dcook> |
Component: | Architecture, internals, and plumbing | Assignee: | David Cook <dcook> |
Status: | Failed QA --- | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | normal | ||
Priority: | P5 - low | CC: | jonathan.druart, m.de.rooy, paul.derscheid |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: |
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=11581 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30803 |
||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | 23329 | ||
Bug Blocks: | |||
Attachments: |
Bug 30261 - Render opac 404 for opac/tracklinks.pl
Bug 30261 - Render opac 404 for opac/tracklinks.pl |
Description
David Cook
2022-03-10 03:28:52 UTC
Created attachment 131533 [details] [review] Bug 30261 - Render opac 404 for opac/tracklinks.pl This patch updates C4::Output::output_error to take an additional argument which contains an "interface" parameter, so that opac/tracklinks.pl can generate an OPAC error instead of an Intranet error. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/test.pl 2. Note normal Intranet 404 error 3. Go to http://localhost:8080/cgi-bin/koha/tracklinks.pl?uri=https%3A%2F%2Ftest.com&biblionumber=29 4. Note normal Opac 404 error 4b. For extra points, note that tracklinks.pl returns a 404 instead of a 302 to the 404.pl page 5. Change 'TrackClicks' syspref to "Track" 6. Go to http://localhost:8080/cgi-bin/koha/tracklinks.pl?uri=https%3A%2F%2Ftest.com&biblionumber=29 7. Note normal Opac 404 error 7b. For extra points, note that tracklinks.pl returns a 404 instead of a 302 to the 404.pl page 8. Rejoice! Created attachment 134921 [details] [review] Bug 30261 - Render opac 404 for opac/tracklinks.pl This patch updates C4::Output::output_error to take an additional argument which contains an "interface" parameter, so that opac/tracklinks.pl can generate an OPAC error instead of an Intranet error. Test plan: 0. Apply patch and koha-plack --restart kohadev 1. Go to http://localhost:8081/test.pl 2. Note normal Intranet 404 error 3. Go to http://localhost:8080/cgi-bin/koha/tracklinks.pl?uri=https%3A%2F%2Ftest.com&biblionumber=29 4. Note normal Opac 404 error 4b. For extra points, note that tracklinks.pl returns a 404 instead of a 302 to the 404.pl page 5. Change 'TrackClicks' syspref to "Track" 6. Go to http://localhost:8080/cgi-bin/koha/tracklinks.pl?uri=https%3A%2F%2Ftest.com&biblionumber=29 7. Note normal Opac 404 error 7b. For extra points, note that tracklinks.pl returns a 404 instead of a 302 to the 404.pl page 8. Rejoice! Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Passes all tests. No unexpected behaviour could be identified. Current status: Signed Off Thanks Paul :) QAing Processing additional checks * Commit title does not start with 'Bug XXXXX: ' - 2dc735c225 You should know by now, haha There is something more going on here behind the scenes since we applied the errordocument changes to plack.psgi. Whatever we are doing now in the arguments to output_error like: - output_error( $cgi, '404' ); + output_error( $cgi, '404', { interface => 'opac' } ); is completely useless. Why? 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! In other words, the bug description is invalid too. There is actually no problem when it is a 404. But we found another problem. We should fix the hardcoded 404 for other errors. (In reply to Marcel de Rooy from comment #5) > Processing additional checks > > * Commit title does not start with 'Bug XXXXX: ' - 2dc735c225 > > You should know by now, haha Huh... that is weird! Must've been an off day. (In reply to Marcel de Rooy from comment #6) > There is something more going on here behind the scenes since we applied the > errordocument changes to plack.psgi. > > Whatever we are doing now in the arguments to output_error like: > - output_error( $cgi, '404' ); > + output_error( $cgi, '404', { interface => 'opac' } ); > is completely useless. > Why? > > 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! > > In other words, the bug description is invalid too. There is actually no > problem when it is a 404. > > But we found another problem. We should fix the hardcoded 404 for other > errors. I'll have another look at this one. Can you open a different bug for what you describe? Feel free to CC me in and I'll take a look. (In reply to David Cook from comment #8) > Can you open a different bug for what you describe? Feel free to CC me in > and I'll take a look. Sure. Bug 30803 is yours :) |