Bug 23006 - Can't use inventory tool with barcodes that contain regex relevant characters ($,...)
Summary: Can't use inventory tool with barcodes that contain regex relevant characters...
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-05-29 09:36 UTC by Katrin Fischer
Modified: 2019-06-11 16:23 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.01


Attachments
Bug 23006: Cannot use inventory with barcodes containing special chars (1.37 KB, patch)
2019-05-29 14:12 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 23006: Cannot use inventory with barcodes containing special chars (1.38 KB, patch)
2019-05-31 07:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 23006: Cannot use inventory with barcodes containing special chars (1.46 KB, patch)
2019-05-31 17:00 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2019-05-29 09:36:45 UTC
If your barcodes contain characters like $ (and possible others), it's not possible to use the inventory tool with a barcode file as they will report as 'not scanned', while the datelastseen is still updated.

We have a library system that uses barcodes with the following schema:
LOCATION_CODE$DIGITS

When uploading a file with such barcodes in the inventory tool, they all report as missing. Exceptions are items that are on the wrong place on the shelf, they report correctly. The datelastseen is updated for all items anyway.

To test:
1) Create an item with the following barcode: LOC$00001
2) Create a file with the same barcode on the first line.
3) Go to the inventory tool
4) Upload barcode file
   Check "Compare barcodes list to results"
   Check "Do not check in items scanned during inventory"
   Check "Skip items on loan"
   You might want to pick additional limits for a smaller result set depending
   on your sample db size, make sure it matches the item catalogued
5) Verify the scanned item is in the list, but the status reads "Missing (not scanned)".

I found this regex in the perl code:

         if( !$barcode ) {
             $item->{problems}->{no_barcode} = 1;
         } elsif ( grep /^$barcode$/, @scanned_barcodes ) {
             next;
         } else {
             $item->{problems}->{not_scanned} = 1;
         }

I assume that grep /^$barcode$/ fails because of the unescaped $. 

Is there a good way to make sure all problematic characters are escaped? 
Is the grep really necessary here? 

Later on we make the comparisons differently which is why the "wrongplace" still appears correctly and probably also why the datelastseen is updated as well.
Comment 1 Katrin Fischer 2019-05-29 09:53:12 UTC
Suggested fix (thx mtj!)

} elsif ( grep /^\Q$barcode\E$/, @scanned_barcodes ) {
Comment 2 Katrin Fischer 2019-05-29 09:53:24 UTC
(In reply to Katrin Fischer from comment #1)
> Suggested fix (thx mtj!)
> 
> } elsif ( grep /^\Q$barcode\E$/, @scanned_barcodes ) {

https://perldoc.perl.org/functions/quotemeta.html
Comment 3 Marcel de Rooy 2019-05-29 12:19:39 UTC
Also working on bug 22908 (slightly) related in using regexs for separating barcodes.
Comment 4 Marcel de Rooy 2019-05-29 12:33:07 UTC
(In reply to Marcel de Rooy from comment #3)
> Also working on bug 22908 (slightly) related in using regexs for separating
> barcodes.

Oops, bug 22996
Comment 5 Jonathan Druart 2019-05-29 12:55:00 UTC
A more readable grep would be:

} elsif ( grep { $_ eq $barcode } @scanned_barcodes ) {
Comment 6 Marcel de Rooy 2019-05-29 13:46:36 UTC
Will submit a patch.
Comment 7 Marcel de Rooy 2019-05-29 14:12:25 UTC
Created attachment 90188 [details] [review]
Bug 23006: Cannot use inventory with barcodes containing special chars

If a barcode contains special characters like $, the regex in inventory
does no longer work.
This fix (as suggested by Jonathan) replaces the regex by a eq compare.

Test plan:
Pick one barcode and add a $ somewhere in the middle, say 123$456.
Include this barcode in a barcode file for inventory.
Run inventory on that file and verify that barcode was read and datelastseen
was updated. The barcode should not be reported as missing (not scanned).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Katrin Fischer 2019-05-31 07:51:06 UTC
Created attachment 90217 [details] [review]
Bug 23006: Cannot use inventory with barcodes containing special chars

If a barcode contains special characters like $, the regex in inventory
does no longer work.
This fix (as suggested by Jonathan) replaces the regex by a eq compare.

Test plan:
Pick one barcode and add a $ somewhere in the middle, say 123$456.
Include this barcode in a barcode file for inventory.
Run inventory on that file and verify that barcode was read and datelastseen
was updated. The barcode should not be reported as missing (not scanned).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 9 Jonathan Druart 2019-05-31 17:00:03 UTC
Created attachment 90237 [details] [review]
Bug 23006: Cannot use inventory with barcodes containing special chars

If a barcode contains special characters like $, the regex in inventory
does no longer work.
This fix (as suggested by Jonathan) replaces the regex by a eq compare.

Test plan:
Pick one barcode and add a $ somewhere in the middle, say 123$456.
Include this barcode in a barcode file for inventory.
Run inventory on that file and verify that barcode was read and datelastseen
was updated. The barcode should not be reported as missing (not scanned).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 10 Katrin Fischer 2019-05-31 17:55:17 UTC
Thx all!
Comment 11 Martin Renvoize 2019-06-04 09:08:08 UTC
Nice work!

Pushed to master for 19.11.00
Comment 12 Fridolin SOMERS 2019-06-10 14:49:21 UTC
Pushed to 19.05.x for 19.05.01
Comment 13 Lucas Gass 2019-06-11 16:23:22 UTC
backported to 18.11.x for 18.11.07