Bug 41619 - Add `Koha::CSV`
Summary: Add `Koha::CSV`
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Marcel de Rooy
URL:
Keywords: release-notes-needed
Depends on:
Blocks: 41620 41622 41625 41626 41627 41628 41629 41630 41631 41632 41633
  Show dependency treegraph
 
Reported: 2026-01-14 19:35 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2026-02-27 20:13 UTC (History)
9 users (show)

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


Attachments
Bug 41619: Add Koha::CSV (11.22 KB, patch)
2026-01-14 19:50 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 41619: Add Koha::CSV (11.25 KB, patch)
2026-01-15 14:05 UTC, David Nind
Details | Diff | Splinter Review
Bug 41619: Add Koha::CSV (11.35 KB, patch)
2026-01-16 07:37 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 41619: (Follow-up) Consider Logical Defined-Or in test (2.81 KB, patch)
2026-02-19 12:39 UTC, Jake Deery
Details | Diff | Splinter Review
Bug 41619: (Follow-up) Consider Logical Defined-Or in test (2.85 KB, patch)
2026-02-25 19:43 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 41619: (Follow-up) Fix for t/Koha/CSV.t (1.56 KB, patch)
2026-02-27 11:09 UTC, Jake Deery
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2026-01-14 19:35:10 UTC
I propose we add a wrapper class on top of `Text::CSV` that takes care of applying all the required settings we might need. It should provide a way to override those settings (e.g. separator character, formula, etc).
Comment 1 Tomás Cohen Arazi (tcohen) 2026-01-14 19:50:15 UTC
Created attachment 191466 [details] [review]
Bug 41619: Add Koha::CSV

This patch introduces Koha::CSV, a wrapper around Text::CSV_XS that
provides consistent defaults and methods for CSV generation across Koha.

The class:
- Inherits CSV delimiter from CSVDelimiter system preference
- Enforces security defaults (binary=1, formula='empty')
- Provides convenient methods for common CSV operations
- Allows per-instance configuration overrides

This establishes a foundation for standardizing CSV exports throughout
Koha, eliminating inconsistencies in CSV generation and providing a
testable, maintainable approach to CSV handling.

Test plan:
1. Apply patch
2. Run:
   $ ktd --shell
  k$ prove t/Koha/CSV.t
=> SUCCESS: Tests pass!
3. Tests cover:
   - Default initialization with CSVDelimiter preference
   - Preference value inheritance (comma, semicolon, tabulation)
   - Constructor parameter overrides
   - add_row() method with various data types
   - combine() and string() methods
   - print() to filehandle
   - Proper quote escaping and empty field handling
4. Sign off :-D
Comment 2 Tomás Cohen Arazi (tcohen) 2026-01-15 12:43:25 UTC
I used AI to analyze which `Text::CSV` methods we use across the codebase, and so required being added to `Koha::CSV`. Forgot to mention in the commit.
Comment 3 David Nind 2026-01-15 14:05:09 UTC
Created attachment 191491 [details] [review]
Bug 41619: Add Koha::CSV

This patch introduces Koha::CSV, a wrapper around Text::CSV_XS that
provides consistent defaults and methods for CSV generation across Koha.

The class:
- Inherits CSV delimiter from CSVDelimiter system preference
- Enforces security defaults (binary=1, formula='empty')
- Provides convenient methods for common CSV operations
- Allows per-instance configuration overrides

This establishes a foundation for standardizing CSV exports throughout
Koha, eliminating inconsistencies in CSV generation and providing a
testable, maintainable approach to CSV handling.

Test plan:
1. Apply patch
2. Run:
   $ ktd --shell
  k$ prove t/Koha/CSV.t
=> SUCCESS: Tests pass!
3. Tests cover:
   - Default initialization with CSVDelimiter preference
   - Preference value inheritance (comma, semicolon, tabulation)
   - Constructor parameter overrides
   - add_row() method with various data types
   - combine() and string() methods
   - print() to filehandle
   - Proper quote escaping and empty field handling
4. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 Marcel de Rooy 2026-01-16 07:37:44 UTC
Created attachment 191551 [details] [review]
Bug 41619: Add Koha::CSV

This patch introduces Koha::CSV, a wrapper around Text::CSV_XS that
provides consistent defaults and methods for CSV generation across Koha.

The class:
- Inherits CSV delimiter from CSVDelimiter system preference
- Enforces security defaults (binary=1, formula='empty')
- Provides convenient methods for common CSV operations
- Allows per-instance configuration overrides

This establishes a foundation for standardizing CSV exports throughout
Koha, eliminating inconsistencies in CSV generation and providing a
testable, maintainable approach to CSV handling.

Test plan:
1. Apply patch
2. Run:
   $ ktd --shell
  k$ prove t/Koha/CSV.t
=> SUCCESS: Tests pass!
3. Tests cover:
   - Default initialization with CSVDelimiter preference
   - Preference value inheritance (comma, semicolon, tabulation)
   - Constructor parameter overrides
   - add_row() method with various data types
   - combine() and string() methods
   - print() to filehandle
   - Proper quote escaping and empty field handling
4. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Lucas Gass (lukeg) 2026-02-06 22:21:50 UTC
Nice work everyone!

Pushed to main for 26.05
Comment 6 Jonathan Druart 2026-02-09 12:49:00 UTC
xt/author/Text_CSV_Various.t .. 1/23     # Files using Text::CSV without formula protection:
    #   Koha/CSV.pm
    # 
    # CSV formula injection protection is required for security.
    # Add 'formula => "empty"' to Text::CSV constructor options.
    # Valid formula modes: 'empty' (recommended), 'die', 'croak', 'diag', or numeric 1-5

    #   Failed test 'All Text::CSV usage includes formula injection protection'
    #   at xt/author/Text_CSV_Various.t line 157.
    #          got: '1'
    #     expected: '0'
    # Looks like you failed 1 test of 2.
Comment 7 Jake Deery 2026-02-19 12:39:00 UTC
Created attachment 193461 [details] [review]
Bug 41619: (Follow-up) Consider Logical Defined-Or in test

This patch fixes xt/author/Text_CSV_Various.t by using a regex that can consider logical defined-or statements (|| and //, etc) when checking which formula has been used.

TO TEST:
a)  run prove -vv xt/author/Text_CSV_Various.t
    *)  notice the test fails
== APPLY PATCH ==
b)  repeat step a)
    *)  notice the test now passes
c)  inspect diff for xt/author/Text_CSV_Various.t
    *)  notice we are now using a wildcard (.*) between => and the first ' to skip variable declarations
d)  inspect diff for Koha/CSV.pm
    *)  notice we are now hard-coding 'empty' as a defined-or in case someone decides not to define a format at the class level
        (which was not being done before anyway and is now defaulted to undefined)
== SIGN OFF ==
Comment 8 Jake Deery 2026-02-19 12:42:00 UTC
I've been getting this, too. I've resolved it by tweaking the regex and moving the hard-coded 'empty' to the Text::CSV_XS call. Others may have a different idea of how to resolve this, but my work is attached nonetheless.

Ta,
Jake (D)
Comment 9 Lucas Gass (lukeg) 2026-02-25 19:43:29 UTC
Created attachment 193939 [details] [review]
Bug 41619: (Follow-up) Consider Logical Defined-Or in test

This patch fixes xt/author/Text_CSV_Various.t by using a regex that can consider logical defined-or statements (|| and //, etc) when checking which formula has been used.

TO TEST:
a)  run prove -vv xt/author/Text_CSV_Various.t
    *)  notice the test fails
== APPLY PATCH ==
b)  repeat step a)
    *)  notice the test now passes
c)  inspect diff for xt/author/Text_CSV_Various.t
    *)  notice we are now using a wildcard (.*) between => and the first ' to skip variable declarations
d)  inspect diff for Koha/CSV.pm
    *)  notice we are now hard-coding 'empty' as a defined-or in case someone decides not to define a format at the class level
        (which was not being done before anyway and is now defaulted to undefined)
== SIGN OFF ==

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 10 Lucas Gass (lukeg) 2026-02-25 19:48:53 UTC
Follow-up pushed to main. Thanks Jake!
Comment 11 Lucas Gass (lukeg) 2026-02-26 14:54:14 UTC
The follow up broke /kohadevbox/koha/t/Koha/CSV.t
Comment 12 Jake Deery 2026-02-27 11:09:15 UTC
Created attachment 194099 [details] [review]
Bug 41619: (Follow-up) Fix for t/Koha/CSV.t

This patch fixes an omission that caused t/Koha/CSV.t to fail. To test, prove t/Koha/CSV.t and notice how it now passes.
Comment 13 Lucas Gass (lukeg) 2026-02-27 20:13:25 UTC
additional follow-up pushed to main. Thanks for the quick follow-up Jake!