Bugzilla – Attachment 194165 Details for
Bug 35104
We should warn when attempting to save MARC records that contain characters invalid in XML
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 35104: Improve nonxml_stripped error context to cover all affected fields
10a9579.patch (text/plain), 6.73 KB, created by
Martin Renvoize (ashimema)
on 2026-02-27 19:24:10 UTC
(
hide
)
Description:
Bug 35104: Improve nonxml_stripped error context to cover all affected fields
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2026-02-27 19:24:10 UTC
Size:
6.73 KB
patch
obsolete
>From 10a9579dbb2a842b97091324d70fcb44a0e7d1c7 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Fri, 27 Feb 2026 13:25:09 +0000 >Subject: [PATCH] Bug 35104: Improve nonxml_stripped error context to cover all > affected fields > >The previous implementation stored only the first XML parser error message >(e.g. "PCDATA invalid Char value 31 at line X, column Y"), which had two >problems: > - The XML parser stops at the first bad character, so records with multiple > affected fields only reported one > - The message gave no indication of which MARC field/subfield to fix > >Add _nonxml_affected_fields(), which scans the raw MARC XML string for every >datafield subfield and control field that contains at least one character >illegal in XML 1.0 (the same set stripped by StripNonXmlChars), and returns >a deduplicated list of field references (e.g. ["245$a", "520$b"]). > >The stored error message now reads "Non-XML characters stripped from: 245$a" >so cataloguers can immediately see which fields need attention, while the >logger warn includes the field context alongside the original XML error for >technical debugging. > >Sponsored-by: OpenFifth >--- > Koha/Biblio/Metadata.pm | 60 +++++++++++++++++++++++++-- > t/db_dependent/Biblio.t | 6 +-- > t/db_dependent/Koha/Biblio/Metadata.t | 11 +++-- > 3 files changed, 66 insertions(+), 11 deletions(-) > >diff --git a/Koha/Biblio/Metadata.pm b/Koha/Biblio/Metadata.pm >index bce132d00c5..f72c8f47c42 100644 >--- a/Koha/Biblio/Metadata.pm >+++ b/Koha/Biblio/Metadata.pm >@@ -84,15 +84,21 @@ sub store { > > if ($stripped_marcxml) { > >- # Stripping fixed it â save the clean version >+ # Stripping fixed it â save the clean version and record all affected fields >+ my @affected = _nonxml_affected_fields( $self->metadata ); >+ my $field_context = >+ @affected >+ ? join( ', ', @affected ) >+ : 'unknown location'; >+ > Koha::Logger->get->warn( > sprintf( >- "Non-XML characters stripped from bibliographic record (biblionumber=%s): %s", >- $self->biblionumber // 'N/A', $marcxml_error >+ "Non-XML characters stripped from bibliographic record (biblionumber=%s) in %s: %s", >+ $self->biblionumber // 'N/A', $field_context, $marcxml_error > ) > ); > $self->metadata($stripped_metadata); >- $nonxml_error_message = $marcxml_error; >+ $nonxml_error_message = "Non-XML characters stripped from: $field_context"; > } else { > > # Truly unrecoverable >@@ -354,6 +360,52 @@ sub _embed_items { > return $record; > } > >+=head3 _nonxml_affected_fields >+ >+ my @fields = _nonxml_affected_fields( $marcxml_string ); >+ >+Scans a raw MARC XML string for every field and subfield that contains at >+least one character illegal in XML 1.0. Returns a deduplicated list of >+human-readable references such as C<245$a> (for subfields) or C<001> (for >+control fields), in document order. >+ >+The same character class is used as L<C4::Charset/StripNonXmlChars>, so the >+set of affected fields reported here exactly matches what would be stripped. >+ >+=cut >+ >+sub _nonxml_affected_fields { >+ my ($marcxml) = @_; >+ >+ # Characters illegal in XML 1.0 â identical to the set stripped by StripNonXmlChars >+ my $non_xml_re = qr/[^\x09\x0A\x0D\x{0020}-\x{D7FF}\x{E000}-\x{FFFD}\x{10000}-\x{10FFFF}]/; >+ >+ my @affected; >+ my %seen; >+ >+ # Scan every subfield inside every datafield >+ while ( $marcxml =~ m{<datafield\b[^>]*\btag="(\w+)"[^>]*>(.*?)</datafield>}gs ) { >+ my ( $tag, $content ) = ( $1, $2 ); >+ while ( $content =~ m{<subfield\b[^>]*\bcode="(\w)"[^>]*>(.*?)</subfield>}gs ) { >+ my ( $code, $val ) = ( $1, $2 ); >+ my $ref = "$tag\$$code"; >+ if ( $val =~ $non_xml_re && !$seen{$ref}++ ) { >+ push @affected, $ref; >+ } >+ } >+ } >+ >+ # Scan control fields >+ while ( $marcxml =~ m{<controlfield\b[^>]*\btag="(\w+)"[^>]*>(.*?)</controlfield>}gs ) { >+ my ( $tag, $val ) = ( $1, $2 ); >+ if ( $val =~ $non_xml_re && !$seen{$tag}++ ) { >+ push @affected, $tag; >+ } >+ } >+ >+ return @affected; >+} >+ > =head3 _type > > =cut >diff --git a/t/db_dependent/Biblio.t b/t/db_dependent/Biblio.t >index 3af45df7475..5fc533b172e 100755 >--- a/t/db_dependent/Biblio.t >+++ b/t/db_dependent/Biblio.t >@@ -1221,15 +1221,15 @@ subtest 'ModBiblio on record with strippable non-XML characters' => sub { > my @warnings; > C4::Biblio::ModBiblio( $record, $biblionumber, '', { warnings => \@warnings } ); > like( >- $warnings[0]{message}, qr/parser error : PCDATA invalid Char value 31/, >+ $warnings[0]{message}, qr/Non-XML characters stripped from: /, > 'Non-XML character stripping reported via warnings arrayref' > ); > > my $metadata = Koha::Biblios->find($biblionumber)->metadata; > my $nonxml_error = $metadata->metadata_errors->search( { error_type => 'nonxml_stripped' } )->next; > like( >- $nonxml_error->message, qr/parser error : PCDATA invalid Char value 31/, >- 'nonxml_stripped error persisted in biblio_metadata_errors after ModBiblio' >+ $nonxml_error->message, qr/Non-XML characters stripped from: /, >+ 'nonxml_stripped error message persisted in biblio_metadata_errors after ModBiblio' > ); > > my $action_logs = >diff --git a/t/db_dependent/Koha/Biblio/Metadata.t b/t/db_dependent/Koha/Biblio/Metadata.t >index 7acba793a9e..96b26fe02ab 100755 >--- a/t/db_dependent/Koha/Biblio/Metadata.t >+++ b/t/db_dependent/Koha/Biblio/Metadata.t >@@ -84,7 +84,7 @@ EOX > }; > > subtest 'MARCXML with strippable non-XML characters' => sub { >- plan tests => 3; >+ plan tests => 4; > $schema->storage->txn_begin; > > # Build MARCXML containing a non-XML control character (chr(31) = Unit Separator) >@@ -107,9 +107,12 @@ EOX > > lives_ok { $record->store } 'MARCXML with strippable characters stores successfully'; > >- ok( >- $record->metadata_errors->search( { error_type => 'nonxml_stripped' } )->count, >- 'nonxml_stripped error recorded after stripping' >+ my $error = $record->metadata_errors->search( { error_type => 'nonxml_stripped' } )->next; >+ ok( $error, 'nonxml_stripped error recorded after stripping' ); >+ like( >+ $error->message, >+ qr/245\$a/, >+ 'error message identifies the affected MARC field and subfield' > ); > > my $stored = Koha::Biblio::Metadatas->find( { biblionumber => $biblio->{biblionumber}, format => 'marcxml' } ); >-- >2.53.0 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 35104
:
157625
|
157627
|
157628
|
157642
|
159615
|
159620
|
159621
|
159630
|
159810
|
159811
|
159812
|
159813
|
166431
|
173145
|
173146
|
176820
|
176821
|
176850
|
176851
|
176852
|
176860
|
193985
|
193986
|
193987
|
193988
|
193989
|
193990
|
193991
|
193992
|
193993
|
193994
|
193995
|
193996
|
193997
|
193998
|
193999
|
194000
|
194001
|
194004
|
194005
|
194006
|
194007
|
194008
|
194009
|
194010
|
194011
|
194012
|
194013
|
194152
|
194153
|
194154
|
194155
|
194156
|
194157
|
194158
|
194159
|
194160
|
194161
|
194162
|
194163
|
194164
| 194165 |
194166
|
194167
|
194168
|
194169
|
194170
|
194171
|
194172