Bug 23873 - Allow marc modification templates to use capturing groups in substitutions
Summary: Allow marc modification templates to use capturing groups in substitutions
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords: Manual, release-notes-needed
: 29014 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-10-22 12:51 UTC by Andrew Fuerste-Henry
Modified: 2022-12-12 21:24 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00


Attachments
Bug 23873: Enable capture groups in MarcModificationTemplates (2.29 KB, patch)
2021-12-13 10:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23873: Enable capture groups in MarcModificationTemplates (2.32 KB, patch)
2021-12-13 10:12 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23873: Unit tests (3.14 KB, patch)
2021-12-13 10:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23873: Enable capture groups in MarcModificationTemplates (2.32 KB, patch)
2021-12-13 10:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 23873: Unit tests (3.21 KB, patch)
2022-01-03 19:19 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 23873: Enable capture groups in MarcModificationTemplates (2.39 KB, patch)
2022-01-03 19:19 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 23873: Unit tests (3.27 KB, patch)
2022-02-17 15:25 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23873: Enable capture groups in MarcModificationTemplates (2.45 KB, patch)
2022-02-17 15:25 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Fuerste-Henry 2019-10-22 12:51:55 UTC
To test:
- Find/create a bib record with a date formatted as YYYYMMDD
- Create a marc modification template to Copy and Replace your field with this regex:
 s/(\d{4})(\d{2})(\d{2})/\1\-\2\-\3/ -- this should reformat your date as "YYYY-MM-DD"
- Apply your template to your bib
- Observe that the match worked, but the substitution failed -- the value "\1\-\2\-\3" is treated literally

Apparently this fails due to some aspect of how regex works within perl? It'd be hugely useful if we could get it working.
Comment 1 Nick Clemens 2019-10-23 11:19:08 UTC
The issue here is that the two sides of the replacement are each passed in a variable.

$value =~ s/$regex->{search}/$regex->{replace}/


On the right side, we don't evaluate the variable, so anything there is treated as text. This was encountered during the callnumber split development and is why the code there stores the full regex in a single field.

https://blog.james.rcpt.to/2010/10/25/perl-search-and-replace-using-variables/

It seems maybe we can safely do this if we allow for /ee (double encoding) in the regex switch, then it will prevent malicious code, but allow desired code.

In the case of callnumber splitting I think we decided too that we should trust the end user. Someone who already has access to the staff client is hopefully someone trustworthy
Comment 2 AspenCat Team 2021-12-01 03:05:45 UTC
I would also like to see substitution work in marc modification templates - or really anywhere we can use regex (such as batch item modification).  This would really power-up the regex capabilities. Agree with Andrew that it would be hugely useful not just for date modification.
Comment 3 Martin Renvoize 2021-12-06 09:13:12 UTC
Are you thinking of working on this Nick.. it sounds like you had a hold of the issues it may present?
Comment 4 Martin Renvoize 2021-12-13 10:02:26 UTC
Created attachment 128430 [details] [review]
Bug 23873: Enable capture groups in MarcModificationTemplates

This patch adds support for regular expression capture groups in
MarcModificationTemplates.

We escape double quotes in the replacement string, then quote the whole
string before applying the `ee` modifier to the final regex application.

See
https://blog.james.rcpt.to/2010/10/25/perl-search-and-replace-using-variables/
for further details.

Test plan
1) Attempt to use capture groups in your regular expression based
   MarcModificationTemplate and note it does not work.
2) Apply the patch and try again, this time the capture group should
   yield the expected results.
Comment 5 Martin Renvoize 2021-12-13 10:12:03 UTC
Created attachment 128431 [details] [review]
Bug 23873: Enable capture groups in MarcModificationTemplates

This patch adds support for regular expression capture groups in
MarcModificationTemplates.

We escape double quotes in the replacement string, then quote the whole
string before applying the `ee` modifier to the final regex application.

See
https://blog.james.rcpt.to/2010/10/25/perl-search-and-replace-using-variables/
for further details.

Test plan
1) Attempt to use capture groups in your regular expression based
   MarcModificationTemplate and note it does not work.
2) Apply the patch and try again, this time the capture group should
   yield the expected results.
3) Run t/SimpleMARC.t and confirm the tests still pass.
Comment 6 Martin Renvoize 2021-12-13 10:22:31 UTC
Created attachment 128432 [details] [review]
Bug 23873: Unit tests

This patch adds a unit test for capture groups support in
Koha::SimpleMARC.

Test plan
1) Run the test prior to applying the capture groups fix, note it fails.
2) Apply teh capture groups fix patch
3) Run the test again, it should now pass
Comment 7 Martin Renvoize 2021-12-13 10:22:35 UTC
Created attachment 128433 [details] [review]
Bug 23873: Enable capture groups in MarcModificationTemplates

This patch adds support for regular expression capture groups in
MarcModificationTemplates.

We escape double quotes in the replacement string, then quote the whole
string before applying the `ee` modifier to the final regex application.

See
https://blog.james.rcpt.to/2010/10/25/perl-search-and-replace-using-variables/
for further details.

Test plan
1) Attempt to use capture groups in your regular expression based
   MarcModificationTemplate and note it does not work.
2) Apply the patch and try again, this time the capture group should
   yield the expected results.
3) Run t/SimpleMARC.t and confirm the tests still pass.
Comment 8 Andrew Fuerste-Henry 2022-01-03 19:10:06 UTC
Adding some details to the test plan, to spare others having to think about it :)

1 - have a record with 245a "the best book ever"
2 - create a modification template to copy and replace the 245a to the 245a, using regex
3 - in your regex, match "(.+\b)best(.+)" and replace with "$1worst$2"
4 - perform a batch mod on your record, end up with "$1worst$2" as your literal title
5) Apply the patch and try again, this time the capture group should
   yield the expected results.
6) Run t/SimpleMARC.t and confirm the tests still pass.
Comment 9 Andrew Fuerste-Henry 2022-01-03 19:19:55 UTC
Created attachment 128980 [details] [review]
Bug 23873: Unit tests

This patch adds a unit test for capture groups support in
Koha::SimpleMARC.

Test plan
1) Run the test prior to applying the capture groups fix, note it fails.
2) Apply teh capture groups fix patch
3) Run the test again, it should now pass

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 10 Andrew Fuerste-Henry 2022-01-03 19:19:59 UTC
Created attachment 128981 [details] [review]
Bug 23873: Enable capture groups in MarcModificationTemplates

This patch adds support for regular expression capture groups in
MarcModificationTemplates.

We escape double quotes in the replacement string, then quote the whole
string before applying the `ee` modifier to the final regex application.

See
https://blog.james.rcpt.to/2010/10/25/perl-search-and-replace-using-variables/
for further details.

Test plan
1) Attempt to use capture groups in your regular expression based
   MarcModificationTemplate and note it does not work.
2) Apply the patch and try again, this time the capture group should
   yield the expected results.
3) Run t/SimpleMARC.t and confirm the tests still pass.

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 11 AspenCat Team 2022-01-11 23:17:55 UTC
Since this works - YAY, could this be applied to batch item modification (such as changing a captured group in the call number and changing it to upper case)? I tried in sandbox after patch applied and did not work, should it have?
Comment 12 Katrin Fischer 2022-01-15 13:05:47 UTC
(In reply to AspenCat Team from comment #11)
> Since this works - YAY, could this be applied to batch item modification
> (such as changing a captured group in the call number and changing it to
> upper case)? I tried in sandbox after patch applied and did not work, should
> it have?

Best to file a separate bug for this if there isn't one yet. They can be linked using 'see also'.
Comment 13 Andrew Fuerste-Henry 2022-01-18 14:06:33 UTC
(In reply to AspenCat Team from comment #11)
> Since this works - YAY, could this be applied to batch item modification
> (such as changing a captured group in the call number and changing it to
> upper case)? I tried in sandbox after patch applied and did not work, should
> it have?

Good news! There'a already a separate bug for this. I just signed it off: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29781
Comment 14 Nick Clemens 2022-02-17 15:25:30 UTC
Created attachment 130759 [details] [review]
Bug 23873: Unit tests

This patch adds a unit test for capture groups support in
Koha::SimpleMARC.

Test plan
1) Run the test prior to applying the capture groups fix, note it fails.
2) Apply teh capture groups fix patch
3) Run the test again, it should now pass

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 15 Nick Clemens 2022-02-17 15:25:34 UTC
Created attachment 130760 [details] [review]
Bug 23873: Enable capture groups in MarcModificationTemplates

This patch adds support for regular expression capture groups in
MarcModificationTemplates.

We escape double quotes in the replacement string, then quote the whole
string before applying the `ee` modifier to the final regex application.

See
https://blog.james.rcpt.to/2010/10/25/perl-search-and-replace-using-variables/
for further details.

Test plan
1) Attempt to use capture groups in your regular expression based
   MarcModificationTemplate and note it does not work.
2) Apply the patch and try again, this time the capture group should
   yield the expected results.
3) Run t/SimpleMARC.t and confirm the tests still pass.

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 16 Katrin Fischer 2022-02-19 18:38:20 UTC
*** Bug 29014 has been marked as a duplicate of this bug. ***
Comment 17 Fridolin Somers 2022-02-22 01:13:56 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄