From 243a70fa384cfc14d9f97864263aeead85673c97 Mon Sep 17 00:00:00 2001 From: Martin Renvoize 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), the stripped version is saved and a +C error row is written to C so +callers and the UI can notify the user. Any pre-existing C +error is removed when the record parses cleanly. + +If the MARCXML cannot be recovered at all, a +I 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 triggered non-XML character +stripping. Returns false (including before C 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 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 - human-readable field reference, e.g. C<245$a> or C<001> + +=item * C - ordinal (decimal) value of the bad character + +=item * C - 1-based character offset within the field value + +=item * C - two-line string with context window and caret pointer (see C<_context_snippet>) + +=back + +The same character class is used as L, 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{]*\btag="(\w+)"[^>]*>(.*?)}gs ) { + my ( $tag, $content ) = ( $1, $2 ); + while ( $content =~ m{]*\bcode="(\w)"[^>]*>(.*?)}gs ) { + $scan->( "$tag\$$1", $2 ); + } + } + + # Scan control fields + while ( $marcxml =~ m{]*\btag="(\w+)"[^>]*>(.*?)}gs ) { + $scan->( $1, $2 ); + } + + return @occurrences; +} + =head3 _type =cut -- 2.53.0