Bug 28290 - Record matching rules with no subfields cause ISE
Summary: Record matching rules with no subfields cause ISE
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-04 18:28 UTC by Nick Clemens
Modified: 2022-11-20 00:22 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00, 22.05.07, 21.11.14


Attachments
Bug 28290: Don't send subfields to 'as_string' if none to send (2.81 KB, patch)
2021-05-04 18:31 UTC, Nick Clemens
Details | Diff | Splinter Review
Test record (6.56 KB, text/plain)
2021-05-04 18:32 UTC, Nick Clemens
Details
Bug 28290: Don't send subfields to 'as_string' if none to send (2.87 KB, patch)
2021-05-07 12:05 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 28290: Don't send subfields to 'as_string' if none to send (2.87 KB, patch)
2021-05-11 10:06 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28290: (follow-up) Remove debug code (639 bytes, patch)
2021-05-11 10:06 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28290: Don't send subfields to 'as_string' if none to send (2.63 KB, patch)
2022-09-02 06:40 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28290: (follow-up) Unit test (1.23 KB, patch)
2022-09-02 06:40 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 28290: Don't send subfields to 'as_string' if none to send (2.69 KB, patch)
2022-09-07 09:10 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 28290: (follow-up) Unit test (1.29 KB, patch)
2022-09-07 09:11 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2021-05-04 18:28:31 UTC
To recreate:
1 - Define a new matching rule with no subfields
    Match-threshold: 1000
    Record type: bibliographic record
    Search-index: Other-control-number
    Score: 1000
    Tag: 035
    Subfields:
    Offset:
    Length:
2 - Attempt to stage a record with an 035 using this rule (i will attach example to bug)
3 - Staging fails
4 - Stage again with no matching rule
5 - Manage the staged record, try to apply the matching rule
6 - It fails, 500 error
Comment 1 Nick Clemens 2021-05-04 18:31:49 UTC
Created attachment 120474 [details] [review]
Bug 28290: Don't send subfields to 'as_string' if none to send

This fixes an error where we send a blank string to Marc::Field->as_string

The subroutine fails as the regex is marked as incomplete as there is nothing between the brackets

Also fixes a missing hidden input for 'item_action' when no items are staged during import

 1 - Define a new matching rule with no subfields
    Match-threshold: 1000
    Record type: bibliographic record
    Search-index: Other-control-number
    Score: 1000
    Tag: 035
    Subfields:
    Offset:
    Length:
 2 - Attempt to stage a record with an 035 using this rule (i will attach example to bug)
 3 - Staging fails
 4 - Stage again with no matching rule
 5 - Manage the staged record, try to apply the matching rule
 6 - It fails, 500 error
 7 - Apply patch
 8 - Restart all the things
 9 - Apply rule again, it works!
10 - Stage the record again using the rule for matching
11 - It stages and matches!
Comment 2 Nick Clemens 2021-05-04 18:32:18 UTC
Created attachment 120475 [details]
Test record
Comment 3 Owen Leonard 2021-05-07 12:05:21 UTC
Created attachment 120686 [details] [review]
Bug 28290: Don't send subfields to 'as_string' if none to send

This fixes an error where we send a blank string to
Marc::Field->as_string

The subroutine fails as the regex is marked as incomplete as there is
nothing between the brackets

Also fixes a missing hidden input for 'item_action' when no items are
staged during import

 1 - Define a new matching rule with no subfields
    Match-threshold: 1000
    Record type: bibliographic record
    Search-index: Other-control-number
    Score: 1000
    Tag: 035
    Subfields:
    Offset:
    Length:
 2 - Attempt to stage a record with an 035 using this rule (i will
     attach example to bug)
 3 - Staging fails
 4 - Stage again with no matching rule
 5 - Manage the staged record, try to apply the matching rule
 6 - It fails, 500 error
 7 - Apply patch
 8 - Restart all the things
 9 - Apply rule again, it works!
10 - Stage the record again using the rule for matching
11 - It stages and matches!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 4 Katrin Fischer 2021-05-10 21:09:51 UTC
QA script explodes a bit - missing a dependency maybe? Can you please verify/check?

 FAIL	C4/Matcher.pm
   OK	  critic
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  pod
   FAIL	  pod coverage
		POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666
   OK	  spelling
   FAIL	  valid
		Can't locate Carp/Always.pm in @INC (you may need to install the Carp::Always module) (@INC contains: /home/vagrant/kohaclone /home/vagrant/qa-test-tools /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.24.1 /usr/local/share/perl/5.24.1 /usr/lib/x86_64-linux-gnu/perl5/5.24 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.24 /usr/share/perl/5.24 /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base /var/lib/koha/kohadev/plugins)
Comment 5 Nick Clemens 2021-05-11 10:06:38 UTC
Created attachment 120827 [details] [review]
Bug 28290: Don't send subfields to 'as_string' if none to send

This fixes an error where we send a blank string to
Marc::Field->as_string

The subroutine fails as the regex is marked as incomplete as there is
nothing between the brackets

Also fixes a missing hidden input for 'item_action' when no items are
staged during import

 1 - Define a new matching rule with no subfields
    Match-threshold: 1000
    Record type: bibliographic record
    Search-index: Other-control-number
    Score: 1000
    Tag: 035
    Subfields:
    Offset:
    Length:
 2 - Attempt to stage a record with an 035 using this rule (i will
     attach example to bug)
 3 - Staging fails
 4 - Stage again with no matching rule
 5 - Manage the staged record, try to apply the matching rule
 6 - It fails, 500 error
 7 - Apply patch
 8 - Restart all the things
 9 - Apply rule again, it works!
10 - Stage the record again using the rule for matching
11 - It stages and matches!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 6 Nick Clemens 2021-05-11 10:06:41 UTC
Created attachment 120828 [details] [review]
Bug 28290: (follow-up) Remove debug code
Comment 7 Martin Renvoize 2021-05-17 13:31:23 UTC
Looks good.. but, it looks like C4::Matcher is fairly well tested.. could you add a test for this change too?
Comment 8 Fridolin Somers 2022-08-31 18:26:14 UTC
Still valid ?
Comment 9 Nick Clemens 2022-09-02 06:40:23 UTC
Created attachment 140066 [details] [review]
Bug 28290: Don't send subfields to 'as_string' if none to send

This fixes an error where we send a blank string to
Marc::Field->as_string

The subroutine fails as the regex is marked as incomplete as there is
nothing between the brackets

Also fixes a missing hidden input for 'item_action' when no items are
staged during import

 1 - Define a new matching rule with no subfields
    Match-threshold: 1000
    Record type: bibliographic record
    Search-index: Other-control-number
    Score: 1000
    Tag: 035
    Subfields:
    Offset:
    Length:
 2 - Attempt to stage a record with an 035 using this rule (i will
     attach example to bug)
 3 - Staging fails
 4 - Stage again with no matching rule
 5 - Manage the staged record, try to apply the matching rule
 6 - It fails, 500 error
 7 - Apply patch
 8 - Restart all the things
 9 - Apply rule again, it works!
10 - Stage the record again using the rule for matching
11 - It stages and matches!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 10 Nick Clemens 2022-09-02 06:40:28 UTC
Created attachment 140067 [details] [review]
Bug 28290: (follow-up) Unit test
Comment 11 Katrin Fischer 2022-09-07 09:10:59 UTC
Created attachment 140263 [details] [review]
Bug 28290: Don't send subfields to 'as_string' if none to send

This fixes an error where we send a blank string to
Marc::Field->as_string

The subroutine fails as the regex is marked as incomplete as there is
nothing between the brackets

Also fixes a missing hidden input for 'item_action' when no items are
staged during import

 1 - Define a new matching rule with no subfields
    Match-threshold: 1000
    Record type: bibliographic record
    Search-index: Other-control-number
    Score: 1000
    Tag: 035
    Subfields:
    Offset:
    Length:
 2 - Attempt to stage a record with an 035 using this rule (i will
     attach example to bug)
 3 - Staging fails
 4 - Stage again with no matching rule
 5 - Manage the staged record, try to apply the matching rule
 6 - It fails, 500 error
 7 - Apply patch
 8 - Restart all the things
 9 - Apply rule again, it works!
10 - Stage the record again using the rule for matching
11 - It stages and matches!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Katrin Fischer 2022-09-07 09:11:03 UTC
Created attachment 140264 [details] [review]
Bug 28290: (follow-up) Unit test

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Tomás Cohen Arazi 2022-09-07 20:57:59 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 14 Lucas Gass 2022-10-31 20:02:58 UTC
Backported to 22.05.x for upcoming 22.05.07 release
Comment 15 Arthur Suzuki 2022-11-14 13:33:35 UTC
applied to 21.11.x for 21.11.14
Comment 16 Victor Grousset/tuxayo 2022-11-20 00:22:06 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document it seems, marking resolved.