Summary: | Remove dead code in tools/letter.tt that looks like a form that would POST without an op | ||
---|---|---|---|
Product: | Koha | Reporter: | Phil Ringnalda <phil> |
Component: | Tools | Assignee: | Phil Ringnalda <phil> |
Status: | RESOLVED FIXED | QA Contact: | Paul Derscheid <paul.derscheid> |
Severity: | normal | ||
Priority: | P3 | CC: | jonathan.druart, lucas, paul.derscheid |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37823 | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
24.11.00
|
|
Circulation function: | |||
Bug Depends on: | 36192 | ||
Bug Blocks: | 37728 | ||
Attachments: |
Bug 37785: Remove dead code in tools/letter.tt that looks like a form that would POST without an op
Bug 37785: Remove dead code in tools/letter.tt that looks like a form that would POST without an op Bug 37785: Remove dead code in tools/letter.tt that looks like a form that would POST without an op |
Description
Phil Ringnalda
2024-08-30 04:07:47 UTC
Created attachment 170899 [details] [review] Bug 37785: Remove dead code in tools/letter.tt that looks like a form that would POST without an op We intend not to have forms with method="post" without an op variable (so we can check that the op starts with "cud-" as part of the CSRF protection), but because of bug 37728 some were missed. The two in tools/letter.tt are blocks of never-used code which would display a message confirming that you saved a notice, or that a notice was deleted after you confirmed that you wanted to delete it, but neither one has ever been executed. Now, the names of the ops don't match, because they are cud-add_validate etc. and would have to explicitly set a param for add_validate, but even before the CSRF change to cud- ops, they explicitly unset their $op so that as they say "# we return to the default screen for the next operation". Prior to that, they just did "print $input->redirect("letter.pl");" No test plan is possible, since this code has never once done anything. Sponsored-by: Chetco Community Public Library Hum, yes... Better to fix the confirmation messages but I do agree it's out of the scope of this bug. Created attachment 170959 [details] [review] Bug 37785: Remove dead code in tools/letter.tt that looks like a form that would POST without an op We intend not to have forms with method="post" without an op variable (so we can check that the op starts with "cud-" as part of the CSRF protection), but because of bug 37728 some were missed. The two in tools/letter.tt are blocks of never-used code which would display a message confirming that you saved a notice, or that a notice was deleted after you confirmed that you wanted to delete it, but neither one has ever been executed. Now, the names of the ops don't match, because they are cud-add_validate etc. and would have to explicitly set a param for add_validate, but even before the CSRF change to cud- ops, they explicitly unset their $op so that as they say "# we return to the default screen for the next operation". Prior to that, they just did "print $input->redirect("letter.pl");" No test plan is possible, since this code has never once done anything. Sponsored-by: Chetco Community Public Library Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 171266 [details] [review] Bug 37785: Remove dead code in tools/letter.tt that looks like a form that would POST without an op We intend not to have forms with method="post" without an op variable (so we can check that the op starts with "cud-" as part of the CSRF protection), but because of bug 37728 some were missed. The two in tools/letter.tt are blocks of never-used code which would display a message confirming that you saved a notice, or that a notice was deleted after you confirmed that you wanted to delete it, but neither one has ever been executed. Now, the names of the ops don't match, because they are cud-add_validate etc. and would have to explicitly set a param for add_validate, but even before the CSRF change to cud- ops, they explicitly unset their $op so that as they say "# we return to the default screen for the next operation". Prior to that, they just did "print $input->redirect("letter.pl");" No test plan is possible, since this code has never once done anything. Sponsored-by: Chetco Community Public Library Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Pushed for 24.11! Well done everyone, thank you! |