Bugzilla – Attachment 194221 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: Strip non-XML characters gracefully with per-field error tracking
243a70f.patch (text/plain), 11.73 KB, created by
Martin Renvoize (ashimema)
on 2026-03-01 16:23:47 UTC
(
hide
)
Description:
Bug 35104: Strip non-XML characters gracefully with per-field error tracking
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2026-03-01 16:23:47 UTC
Size:
11.73 KB
patch
obsolete
>From 243a70fa384cfc14d9f97864263aeead85673c97 Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@openfifth.co.uk> >Date: Sat, 28 Feb 2026 17:06:53 +0000 >Subject: [PATCH] Bug 35104: Strip non-XML characters gracefully with per-field > error tracking > >Rewrite Metadata::store to strip non-XML characters automatically rather >than throwing a Koha::Exceptions::Metadata::Invalid exception. When >stripping is needed: > >* Every bad character is located individually via _find_nonxml_chars(), > which scans the raw MARCXML and records the field reference (e.g. > 336$a), character ordinal, 1-based position within the subfield value, > and a two-line context snippet generated by _context_snippet(). > >* Context snippets show up to 30 characters either side of the bad > character, replacing it with the appropriate Unicode Control Picture > (U+2400-U+241F for C0 controls, U+2421 for DEL, U+FFFD otherwise) so > the location is visible even in plain text. A second line carries a > caret (^) aligned beneath the replacement glyph. > >* Each occurrence is stored as a separate row in biblio_metadata_errors > with error_type 'nonxml_stripped'. On a clean re-save the existing > error rows are deliberately left in place: they are review flags > requiring explicit human resolution and must not be silently cleared. > Only a save that triggers fresh stripping replaces the existing set. > >* A new stripped_on_last_store() method lets callers distinguish between > "stripping just happened" and "pre-existing errors are present", so > the UI can avoid spuriously re-displaying a save-time warning when the > record is simply re-saved without changes. > >If the MARCXML cannot be recovered at all even after stripping, the >existing Koha::Exceptions::Metadata::Invalid exception is still thrown. > >Sponsored-by: OpenFifth >--- > Koha/Biblio/Metadata.pm | 251 ++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 242 insertions(+), 9 deletions(-) > >diff --git a/Koha/Biblio/Metadata.pm b/Koha/Biblio/Metadata.pm >index 926f5d85bc5..b702171cfd2 100644 >--- a/Koha/Biblio/Metadata.pm >+++ b/Koha/Biblio/Metadata.pm >@@ -24,14 +24,19 @@ use C4::Biblio qw( GetMarcFromKohaField ); > use C4::Charset qw( StripNonXmlChars ); > use C4::Items qw( GetMarcItem ); > >+use Koha::Biblio::Metadata::Error; >+use Koha::Biblio::Metadata::Errors; > use Koha::Database; > use Koha::Exceptions::Metadata; >+use Koha::Logger; > use Koha::RecordSources; > > use base qw(Koha::Object); > > =head1 NAME > >+=encoding utf-8 >+ > Koha::Metadata - Koha Metadata Object class > > =head1 API >@@ -45,11 +50,22 @@ Koha::Metadata - Koha Metadata Object class > Metadata specific store method to catch errant characters prior > to committing to the database. > >+If the MARCXML cannot be parsed but can be recovered by stripping non-XML >+characters (C<StripNonXmlChars>), the stripped version is saved and a >+C<nonxml_stripped> error row is written to C<biblio_metadata_errors> so >+callers and the UI can notify the user. Any pre-existing C<nonxml_stripped> >+error is removed when the record parses cleanly. >+ >+If the MARCXML cannot be recovered at all, a >+I<Koha::Exceptions::Metadata::Invalid> exception is thrown. >+ > =cut > > sub store { > my $self = shift; > >+ my $nonxml_error_message; >+ > # Check marcxml will roundtrip > if ( $self->format eq 'marcxml' ) { > >@@ -61,19 +77,116 @@ sub store { > }; > my $marcxml_error = $@; > chomp $marcxml_error; >+ > unless ($marcxml) { >- warn $marcxml_error; >- Koha::Exceptions::Metadata::Invalid->throw( >- id => $self->id, >- biblionumber => $self->biblionumber, >- format => $self->format, >- schema => $self->schema, >- decoding_error => $marcxml_error, >- ); >+ >+ # Attempt recovery by stripping non-XML characters >+ my $stripped_metadata = StripNonXmlChars( $self->metadata ); >+ my $stripped_marcxml = eval { MARC::Record::new_from_xml( $stripped_metadata, 'UTF-8', $self->schema ); }; >+ >+ if ($stripped_marcxml) { >+ >+ # Stripping fixed it â enumerate every bad character for the error log >+ my @nonxml_chars = _find_nonxml_chars( $self->metadata ); >+ my $field_context = do { >+ my %seen; >+ join( ', ', grep { !$seen{$_}++ } map { $_->{field} } @nonxml_chars ) >+ || 'unknown location'; >+ }; >+ >+ Koha::Logger->get->warn( >+ sprintf( >+ "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 = \@nonxml_chars || $marcxml_error; >+ } else { >+ >+ # Truly unrecoverable >+ Koha::Logger->get->warn($marcxml_error); >+ Koha::Exceptions::Metadata::Invalid->throw( >+ id => $self->id, >+ biblionumber => $self->biblionumber, >+ format => $self->format, >+ schema => $self->schema, >+ decoding_error => $marcxml_error, >+ ); >+ } >+ } >+ } >+ >+ $self->SUPER::store; >+ >+ # If this save triggered fresh stripping, add to any existing nonxml_stripped >+ # errors with the new set. If the save was clean (no stripping needed), leave >+ # any existing errors alone â they are review flags requiring explicit human >+ # resolution and should not be silently cleared by a routine re-save. >+ if ($nonxml_error_message) { >+ my @occurrences = >+ ref $nonxml_error_message eq 'ARRAY' >+ ? @{$nonxml_error_message} >+ : (); >+ >+ if (@occurrences) { >+ for my $occ (@occurrences) { >+ Koha::Biblio::Metadata::Error->new( >+ { >+ metadata_id => $self->id, >+ error_type => 'nonxml_stripped', >+ message => sprintf( >+ "%s: invalid char value %d (U+%04X) at position %d\n%s", >+ $occ->{field}, $occ->{char_ord}, $occ->{char_ord}, $occ->{position}, >+ $occ->{snippet} >+ ), >+ } >+ )->store; >+ } >+ } else { >+ >+ # Fallback: couldn't pinpoint individual chars â store the parser error >+ Koha::Biblio::Metadata::Error->new( >+ { >+ metadata_id => $self->id, >+ error_type => 'nonxml_stripped', >+ message => $nonxml_error_message, >+ } >+ )->store; > } > } > >- return $self->SUPER::store; >+ $self->{_stripped_on_last_store} = $nonxml_error_message ? 1 : 0; >+ >+ return $self; >+} >+ >+=head3 stripped_on_last_store >+ >+ if ( $metadata->stripped_on_last_store ) { ... } >+ >+Returns true if the most recent call to C<store> triggered non-XML character >+stripping. Returns false (including before C<store> has ever been called). >+ >+=cut >+ >+sub stripped_on_last_store { >+ my $self = shift; >+ return $self->{_stripped_on_last_store} // 0; >+} >+ >+=head3 metadata_errors >+ >+ my $errors = $metadata->metadata_errors; >+ >+Returns a I<Koha::Biblio::Metadata::Errors> resultset for errors associated >+with this metadata record. >+ >+=cut >+ >+sub metadata_errors { >+ my ($self) = @_; >+ return Koha::Biblio::Metadata::Errors->_new_from_dbic( scalar $self->_result->biblio_metadata_errors ); > } > > =head3 record >@@ -291,6 +404,126 @@ sub _embed_items { > return $record; > } > >+=head3 _context_snippet >+ >+ my $snippet = _context_snippet( \@chars, $pos_0, $char_ord ); >+ >+Given an array-ref of characters C<\@chars>, a 0-based position C<$pos_0> of >+a bad character, and its ordinal C<$char_ord>, returns a two-line string: >+ >+=over 4 >+ >+=item * Line 1 â up to 30 characters of context either side of the bad >+character, with the bad character replaced by a visible glyph (Unicode >+Control Pictures U+2400âU+241F for C0 controls, C<â¡> for DEL, C<?> >+otherwise) and ellipses when the window is truncated. >+ >+=item * Line 2 â a caret (C<^>) aligned beneath the visible glyph. >+ >+=back >+ >+Example output (4-space indent): >+ >+ t $xtac arrâ\ $btxt V $2rdac ontent >+ ^ >+ >+=cut >+ >+sub _context_snippet { >+ my ( $chars_ref, $pos_0, $char_ord ) = @_; >+ >+ # Visible stand-in for the removed character: >+ # C0 controls (U+0000âU+001F) â Unicode Control Pictures (U+2400âU+241F) >+ # DEL (U+007F) â U+2421 SYMBOL FOR DELETE (â¡) >+ # anything else â U+FFFD REPLACEMENT CHARACTER (?) >+ my $visible = >+ $char_ord <= 0x1F ? chr( $char_ord + 0x2400 ) >+ : $char_ord == 0x7F ? "\x{2421}" >+ : "\x{FFFD}"; >+ >+ my $last = $#{$chars_ref}; >+ my $win = 30; >+ >+ my $pre_start = ( $pos_0 > $win ) ? $pos_0 - $win : 0; >+ my $post_end = ( $pos_0 + $win < $last ) ? $pos_0 + $win : $last; >+ >+ my $pre = $pos_0 > 0 ? join( '', @{$chars_ref}[ $pre_start .. $pos_0 - 1 ] ) : ''; >+ my $post = $pos_0 < $last ? join( '', @{$chars_ref}[ $pos_0 + 1 .. $post_end ] ) : ''; >+ >+ my $prefix = ( $pos_0 > $win ) ? '...' : ''; >+ my $suffix = ( $pos_0 + $win < $last ) ? '...' : ''; >+ >+ my $indent = ' '; >+ my $line = "$indent$prefix$pre$visible$post$suffix"; >+ my $caret_col = length($indent) + length($prefix) + length($pre); >+ >+ return "$line\n" . ( ' ' x $caret_col ) . '^'; >+} >+ >+=head3 _find_nonxml_chars >+ >+ my @occurrences = _find_nonxml_chars( $marcxml_string ); >+ >+Scans a raw MARC XML string for every individual character that is illegal >+in XML 1.0. Returns a list of hashrefs (one per character occurrence) with >+the following keys: >+ >+=over 4 >+ >+=item * C<field> - human-readable field reference, e.g. C<245$a> or C<001> >+ >+=item * C<char_ord> - ordinal (decimal) value of the bad character >+ >+=item * C<position> - 1-based character offset within the field value >+ >+=item * C<snippet> - two-line string with context window and caret pointer (see C<_context_snippet>) >+ >+=back >+ >+The same character class is used as L<C4::Charset/StripNonXmlChars>, so >+every occurrence returned here is exactly one that would be stripped. >+ >+=cut >+ >+sub _find_nonxml_chars { >+ 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 @occurrences; >+ >+ my $scan = sub { >+ my ( $field_ref, $val ) = @_; >+ my @chars = split //, $val; >+ for my $i ( 0 .. $#chars ) { >+ if ( $chars[$i] =~ $non_xml_re ) { >+ push @occurrences, { >+ field => $field_ref, >+ char_ord => ord( $chars[$i] ), >+ position => $i + 1, >+ snippet => _context_snippet( \@chars, $i, ord( $chars[$i] ) ), >+ }; >+ } >+ } >+ }; >+ >+ # 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 ) { >+ $scan->( "$tag\$$1", $2 ); >+ } >+ } >+ >+ # Scan control fields >+ while ( $marcxml =~ m{<controlfield\b[^>]*\btag="(\w+)"[^>]*>(.*?)</controlfield>}gs ) { >+ $scan->( $1, $2 ); >+ } >+ >+ return @occurrences; >+} >+ > =head3 _type > > =cut >-- >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
|
194213
|
194214
|
194215
|
194216
|
194217
|
194218
|
194219
|
194220
| 194221 |
194222
|
194223
|
194224
|
194225
|
194226
|
194227