From b4d9063368258aeccf08c291523f96822371c53f Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 27 Apr 2026 17:49:57 +0100 Subject: [PATCH] Bug 41918: (QA follow-up) Document SQL marker contract and TOCTOU window Two small documentation improvements following on from the duplicate- running guard: - Koha::Report::prep_report now spells out the tracking-comment format in its POD. Koha::Reports->running parses this comment, and external log/audit tooling that consumes Koha SQL relied on the previous format (/* saved_sql.id: */). Calling out the new format explicitly makes the public contract visible to anyone reading the POD or grepping for it, and warns future contributors that changing the wrapper will break running() too. - reports/guided_reports.pl gets an inline comment explaining the TOCTOU window between the count check and the actual report execution. The window is intentional - the guard exists to stop accidental double-clicks and two-tab submissions, not to be a serializing lock - so worst case is one extra concurrent run, which is acceptable and cheaper than the alternative. No behaviour change. Test plan: 1) prove t/db_dependent/Koha/Reports.t still passes. 2) perldoc Koha::Report -> prep_report section now mentions the saved_sql.id / user_id wrapper format. Signed-off-by: Lucas Gass --- Koha/Report.pm | 11 ++++++++++- reports/guided_reports.pl | 6 ++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/Koha/Report.pm b/Koha/Report.pm index b8092f90bf..13c8a68f48 100644 --- a/Koha/Report.pm +++ b/Koha/Report.pm @@ -163,7 +163,16 @@ sub new_from_mana { =head3 prep_report Prep the report and return executable sql with parameters embedded and a list of header types -for building batch action links in the template +for building batch action links in the template. + +The returned SQL is suffixed with a tracking comment of the form + + /* { saved_sql.id: } { user_id: } */ + +L parses this comment out of +C to detect concurrent runs of the same +report. External log/audit tooling that consumes Koha SQL should treat +this format as part of the public contract. =cut diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl index 414372bfb6..2d2346d054 100755 --- a/reports/guided_reports.pl +++ b/reports/guided_reports.pl @@ -827,6 +827,12 @@ if ( $op eq 'run' ) { my $duplicate_running_reports_per_user_limit = C4::Context->config('duplicate_running_reports_per_user_limit'); my $duplicate_running_reports; if ( $duplicate_running_reports_per_user_limit && C4::Context->userenv ) { + + # This is a TOCTOU check: between the count below and the actual + # query execution further down, a parallel request can slip past the + # limit. Worst case is one extra concurrent run, which is acceptable + # for the intended use case (preventing accidental double-clicks / + # multi-tab submissions) and avoids the cost of a serializing lock. $duplicate_running_reports = Koha::Reports->running( { report_id => $report_id,