[Bug 40915] New: SIP message parsing with empty fields not perfect
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 Bug ID: 40915 Summary: SIP message parsing with empty fields not perfect Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: SIP2 Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org SIP message parsing can create empty field values that later cause DBIx::Class constraint errors when passed to Koha operations. The current parsing logic in C4::SIP::Sip::MsgType::_initialize() doesn't provide debug logging to help identify when critical fields (patron_id, item_id) are empty, making it difficult to troubleshoot "Odd number of elements in anonymous hash" errors in SIP server logs. This includes some scenarios which cause fields to be treated as (empty) arrayrefs, which flood the logs with this kind of errors: ``` DBIx::Class::ResultSource::_minimal_valueset_satisfying_constraint(): Unable to satisfy requested constraint 'primary', missing values for column(s): 'itemnumber' at /usr/share/koha/lib/Koha/Objects.pm line 95 DBIx::Class::ResultSource::_minimal_valueset_satisfying_constraint(): Unable to satisfy requested constraint 'primary', missing values for column(s): 'borrowernumber' at /kohadevbox/koha/Koha/Objects.pm line 95 ``` -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | CC| |jesse@bywatersolutions.com, | |kyle@bywatersolutions.com, | |lisette@bywatersolutions.co | |m, tomascohen@gmail.com Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 187120 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187120&action=edit Bug 40915: Add comprehensive regression tests for SIP MsgType message parsing and debug logging These tests verify that C4::SIP::Sip::MsgType correctly parses SIP messages without creating problematic arrayref field values, and validates enhanced debug logging for empty critical fields. Test Coverage: • Message parsing with correct protocol versions (PATRON_INFO/HOLD/RENEW use v2, CHECKOUT uses v1) • Empty patron_id and item_id field detection with debug logging • Malformed delimiter handling (double delimiters, trailing empty fields) • Multiple consecutive empty fields in single message • Confusing delimiter patterns that could break parsing • Verification that parsed fields are strings, not arrayrefs • Working siplog mock infrastructure in C4::SIP::Sip::MsgType Arrayref Prevention Tests: Tests verify message parsing DOESN'T create empty arrayrefs that would later cause "Odd number of elements in anonymous hash" errors when passed to Koha::Objects->find() operations. Enhanced Debug Logging Tests: Tests validate that empty critical fields (patron_id, item_id) are detected during parsing and generate appropriate debug messages to help troubleshoot downstream constraint errors. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 187121 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187121&action=edit Bug 40915: Add enhanced debug logging for empty critical fields in SIP message parsing This patch adds debug logging to C4::SIP::Sip::MsgType::_initialize() to help identify when critical fields (patron_id, item_id) are parsed as empty values. Empty critical fields in SIP messages can later cause DBIx::Class constraint errors when passed to Koha::Objects->find() operations, generating confusing 'Odd number of elements in anonymous hash' errors in SIP server logs. The enhanced logging helps administrators identify the root cause by logging when empty patron_id or item_id fields are detected during message parsing. To test: 1. Apply regression tests patch first 2. Run: $ ktd --shell k$ prove t/db_dependent/SIP/MsgType.t => FAIL: Debug logging tests fail (no enhanced logging present) 3. Apply this patch 4. Run: $ ktd --shell k$ prove t/db_dependent/SIP/MsgType.t => SUCCESS: All tests pass, debug logging works correctly 5. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@openfifth.c | |o.uk Summary|SIP message parsing with |SIP message parsing with |empty fields not perfect |empty fields edge cases -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187120|0 |1 is obsolete| | --- Comment #3 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 187182 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187182&action=edit Bug 40915: Add comprehensive regression tests for SIP MsgType message parsing and debug logging These tests verify that C4::SIP::Sip::MsgType correctly parses SIP messages without creating problematic arrayref field values, and validates enhanced debug logging for empty critical fields. Test Coverage: • Message parsing with correct protocol versions (PATRON_INFO/HOLD/RENEW use v2, CHECKOUT uses v1) • Empty patron_id and item_id field detection with debug logging • Malformed delimiter handling (double delimiters, trailing empty fields) • Multiple consecutive empty fields in single message • Confusing delimiter patterns that could break parsing • Verification that parsed fields are strings, not arrayrefs • Working siplog mock infrastructure in C4::SIP::Sip::MsgType Arrayref Prevention Tests: Tests verify message parsing DOESN'T create empty arrayrefs that would later cause "Odd number of elements in anonymous hash" errors when passed to Koha::Objects->find() operations. Enhanced Debug Logging Tests: Tests validate that empty critical fields (patron_id, item_id) are detected during parsing and generate appropriate debug messages to help troubleshoot downstream constraint errors. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187121|0 |1 is obsolete| | --- Comment #4 from Kyle M Hall (khall) <kyle@bywatersolutions.com> --- Created attachment 187183 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187183&action=edit Bug 40915: Add enhanced debug logging for empty critical fields in SIP message parsing This patch adds debug logging to C4::SIP::Sip::MsgType::_initialize() to help identify when critical fields (patron_id, item_id) are parsed as empty values. Empty critical fields in SIP messages can later cause DBIx::Class constraint errors when passed to Koha::Objects->find() operations, generating confusing 'Odd number of elements in anonymous hash' errors in SIP server logs. The enhanced logging helps administrators identify the root cause by logging when empty patron_id or item_id fields are detected during message parsing. To test: 1. Apply regression tests patch first 2. Run: $ ktd --shell k$ prove t/db_dependent/SIP/MsgType.t => FAIL: Debug logging tests fail (no enhanced logging present) 3. Apply this patch 4. Run: $ ktd --shell k$ prove t/db_dependent/SIP/MsgType.t => SUCCESS: All tests pass, debug logging works correctly 5. Sign off :-D Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 Kyle M Hall (khall) <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187182|0 |1 is obsolete| | --- Comment #5 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 187667 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187667&action=edit Bug 40915: Add comprehensive regression tests for SIP MsgType message parsing and debug logging These tests verify that C4::SIP::Sip::MsgType correctly parses SIP messages without creating problematic arrayref field values, and validates enhanced debug logging for empty critical fields. Test Coverage: • Message parsing with correct protocol versions (PATRON_INFO/HOLD/RENEW use v2, CHECKOUT uses v1) • Empty patron_id and item_id field detection with debug logging • Malformed delimiter handling (double delimiters, trailing empty fields) • Multiple consecutive empty fields in single message • Confusing delimiter patterns that could break parsing • Verification that parsed fields are strings, not arrayrefs • Working siplog mock infrastructure in C4::SIP::Sip::MsgType Arrayref Prevention Tests: Tests verify message parsing DOESN'T create empty arrayrefs that would later cause "Odd number of elements in anonymous hash" errors when passed to Koha::Objects->find() operations. Enhanced Debug Logging Tests: Tests validate that empty critical fields (patron_id, item_id) are detected during parsing and generate appropriate debug messages to help troubleshoot downstream constraint errors. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #187183|0 |1 is obsolete| | --- Comment #6 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 187668 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=187668&action=edit Bug 40915: Add enhanced debug logging for empty critical fields in SIP message parsing This patch adds debug logging to C4::SIP::Sip::MsgType::_initialize() to help identify when critical fields (patron_id, item_id) are parsed as empty values. Empty critical fields in SIP messages can later cause DBIx::Class constraint errors when passed to Koha::Objects->find() operations, generating confusing 'Odd number of elements in anonymous hash' errors in SIP server logs. The enhanced logging helps administrators identify the root cause by logging when empty patron_id or item_id fields are detected during message parsing. To test: 1. Apply regression tests patch first 2. Run: $ ktd --shell k$ prove t/db_dependent/SIP/MsgType.t => FAIL: Debug logging tests fail (no enhanced logging present) 3. Apply this patch 4. Run: $ ktd --shell k$ prove t/db_dependent/SIP/MsgType.t => SUCCESS: All tests pass, debug logging works correctly 5. Sign off :-D Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main Version(s)| |25.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 --- Comment #7 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 25.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to main |Pushed to stable Version(s)|25.11.00 |25.11.00,25.05.05 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 --- Comment #8 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Nice work everyone! Pushed to 25.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable CC| |fridolin.somers@biblibre.co | |m Version(s)|25.11.00,25.05.05 |25.11.00,25.05.05,24.11.11 released in| | --- Comment #9 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 24.11.x for 24.11.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 Johanna Räisä <johanna.raisa@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |johanna.raisa@koha-suomi.fi --- Comment #10 from Johanna Räisä <johanna.raisa@koha-suomi.fi> --- This fix has not resolved the actual problem; we are still seeing similar errors in the SIP logs. The main issue is that the code is calling checkouts like this: Koha::Checkouts->find({ itemnumber => $item->itemnumber }); However, the find method expects a primary key value, not a hashref for searching by other fields. A better approach is to use the search method, like this: Koha::Checkouts->search({ itemnumber => $item->itemnumber })->next; This ensures the code works correctly even if the database schema changes, making it more robust and future-proof. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 --- Comment #11 from Johanna Räisä <johanna.raisa@koha-suomi.fi> --- I made a bug about this, https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41672 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 Jan Kissig <bibliothek@th-wildau.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bibliothek@th-wildau.de See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=41707 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40915 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|25.11.00,25.05.05,24.11.11 |25.11.00,25.05.05,24.11.11, released in| |22.11.36 CC| |wainuiwitikapark@catalyst.n | |et.nz Status|Pushed to oldstable |Needs documenting --- Comment #12 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- Pushed to 22.11.x for 22.11.36 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org