Bug 40079: (follow-up) Improve test coverage and fix undef handling

This patch modernizes the Scrubber.t unit test with comprehensive
coverage following Koha best practices:

- Uses subtest structure for logical grouping
- Tests all scrubber types (default, comment, note)
- Includes security testing for XSS prevention
- Covers edge cases and error handling
- Tests new list elements added in previous commits

Also fixes C4::Scrubber to handle undef parameters cleanly by
treating them as 'default' type instead of generating warnings.

Test plan:
1. Run prove t/Scrubber.t
2. Verify all tests pass without warnings
3. Confirm comprehensive coverage of module functionality

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
(cherry picked from commit 21f3340442)
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This commit is contained in:
Martin Renvoize 2025-06-26 15:36:08 +01:00 committed by Paul Derscheid
parent ceb1f9f230
commit 1ab7ccde18
No known key found for this signature in database
GPG Key ID: C25F9FD44CED595B
2 changed files with 140 additions and 75 deletions

View File

@ -35,6 +35,7 @@ my %scrubbertypes = (
sub new {
shift; # ignore our class we are wrapper
my $type = (@_) ? shift : 'default';
$type = 'default' if !defined $type;
if ( !exists $scrubbertypes{$type} ) {
croak "New called with unrecognized type '$type'";
}

View File

@ -1,94 +1,158 @@
#!/usr/bin/perl
# Copyright 2025 Koha Development team
#
# This file is part of Koha
#
# Koha is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# Koha is distributed in the hope that it will be useful, but
# WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with Koha; if not, see <http://www.gnu.org/licenses>
use Modern::Perl;
$| = 1;
use Test::More tests => 7;
use Test::NoWarnings;
use Test::More tests => 27;
use Test::Exception;
use Test::Warn;
BEGIN {
use FindBin;
use lib $FindBin::Bin;
use_ok('C4::Scrubber');
}
sub pretty_line {
my $max = 54;
(@_) or return "#" x $max . "\n";
my $phrase = " " . shift() . " ";
my $half = "#" x ( ( $max - length($phrase) ) / 2 );
return $half . $phrase . $half . "\n";
}
subtest 'new() constructor tests' => sub {
plan tests => 8;
my ( $scrubber, $html, $result, @types, $collapse );
$collapse = 1;
@types = qw(default comment note);
$html = q|
<![CDATA[selfdestruct]]&#x5d;>
<?php echo(" EVIL EVIL EVIL "); ?> <!-- COMMENT -->
<hr> <!-- TMPL_VAR NAME="password" -->
<style type="text/css">body{display:none;}</style>
<link media="screen" type="text/css" rev="stylesheet" rel="stylesheet" href="css.css">
<I FAKE="attribute" > I am ITALICS with fake="attribute" </I><br />
<em FAKE="attribute" > I am em with fake="attribute" </em><br />
<B> I am BOLD </B><br />
<span style="background-image: url(http://hackersite.cn/porno.jpg);"> I am a span w/ style. Bad style.</span>
<span> I am a span trying to inject a link: &lt;a href="badlink.html"&gt; link &lt;/a&gt;</span>
<br>
<A NAME="evil">
<A HREF="javascript:alert('OMG YOO R HACKED');">I am a link firing javascript.</A>
<br />
<A HREF="image/bigone.jpg" ONMOUSEOVER="alert('OMG YOO R HACKED');">
<IMG SRC="image/smallone.jpg" ALT="ONMOUSEOVER JAVASCRIPT">
</A>
</A> <br>
At the end here, I actually have some regular text.
|;
my $scrubber;
lives_ok { $scrubber = C4::Scrubber->new() } 'Constructor with no parameters succeeds';
isa_ok( $scrubber, 'HTML::Scrubber', 'Constructor returns HTML::Scrubber object' );
ok( $scrubber = C4::Scrubber->new(), "Constructor: C4::Scrubber->new()" );
lives_ok { $scrubber = C4::Scrubber->new('default') } 'Constructor with default type succeeds';
isa_ok( $scrubber, 'HTML::Scrubber', 'Constructor with default type returns HTML::Scrubber object' );
isa_ok( $scrubber, 'HTML::Scrubber', 'Constructor returns HTML::Scrubber object' );
lives_ok { $scrubber = C4::Scrubber->new('comment') } 'Constructor with comment type succeeds';
isa_ok( $scrubber, 'HTML::Scrubber', 'Constructor with comment type returns HTML::Scrubber object' );
warning_like { $scrubber->default() } '', "\$scrubber->default ran without fault.";
warning_like { $scrubber->comment() } '', "\$scrubber->comment ran without fault.";
warning_like { $scrubber->process() } '', "\$scrubber->process ran without fault.";
ok( $result = $scrubber->scrub($html), "Getting scrubbed text (type: [default])" );
foreach (@types) {
ok( $scrubber = C4::Scrubber->new($_), "testing Constructor: C4::Scrubber->new($_)" );
warning_like { $scrubber->default() } '', "\$scrubber->default ran without fault.";
warning_like { $scrubber->comment() } '', "\$scrubber->comment ran without fault.";
warning_like { $scrubber->process() } '', "\$scrubber->process ran without fault.";
ok( $result = $scrubber->scrub($html), "Getting scrubbed text (type: $_)" );
}
#Test for invalid new entry
eval {
C4::Scrubber->new("");
fail("test should fail on entry of ''");
lives_ok { $scrubber = C4::Scrubber->new('note') } 'Constructor with note type succeeds';
isa_ok( $scrubber, 'HTML::Scrubber', 'Constructor with note type returns HTML::Scrubber object' );
};
if ($@) {
pass("Test should have failed on entry of '' (empty string) and it did. YAY!");
}
eval {
C4::Scrubber->new("Client");
fail("test should fail on entry of 'Client'");
subtest 'constructor error handling' => sub {
plan tests => 5;
my $scrubber;
lives_ok { $scrubber = C4::Scrubber->new(undef) } 'Constructor with undef type succeeds (treated as default)';
isa_ok( $scrubber, 'HTML::Scrubber', 'Constructor with undef type returns HTML::Scrubber object' );
throws_ok {
C4::Scrubber->new('');
}
qr/New called with unrecognized type/, 'Constructor throws exception for empty string type';
throws_ok {
C4::Scrubber->new('invalid_type');
}
qr/New called with unrecognized type/, 'Constructor throws exception for invalid type';
throws_ok {
C4::Scrubber->new('Client');
}
qr/New called with unrecognized type/, 'Constructor throws exception for Client type';
};
if ($@) {
pass("Test should have failed on entry of 'Client' and it did. YAY!");
}
my $scrub_text =
'<div><span><p><b>bold</b><i>ital</i><em>emphatic</em><big>embiggen</big><small>shrink</small><strong>strongbad</strong><br><u>under</u><hr><ol><li>ordered item</li></ol><ul><li>unordered item</li></ul><dl><dt>term</dt><dd>definition</dd></dl></p></span></div>';
my $scrub_comment =
'<b>bold</b><i>ital</i><em>emphatic</em><big>embiggen</big><small>shrink</small><strong>strongbad</strong><br>underordered itemunordered itemtermdefinition';
is( C4::Scrubber->new('comment')->scrub($scrub_text), $scrub_comment, "Comment scrubber removes expected elements" );
is(
C4::Scrubber->new('note')->scrub($scrub_text), $scrub_text,
"Note scrubber removes (additional) expected elements"
);
subtest 'default scrubber functionality' => sub {
plan tests => 5;
my $scrubber = C4::Scrubber->new('default');
my $malicious_html = q|
<![CDATA[selfdestruct]]&#x5d;>
<?php echo("EVIL EVIL EVIL"); ?>
<script>alert('XSS Attack!');</script>
<style type="text/css">body{display:none;}</style>
<link href="evil.css" rel="stylesheet">
<img src="x" onerror="alert('XSS')">
<a href="javascript:alert('XSS')">Click me</a>
<p onclick="alert('XSS')">Paragraph</p>
<div style="background:url(javascript:alert('XSS'))">Content</div>
Plain text content
|;
my $result = $scrubber->scrub($malicious_html);
unlike( $result, qr/<script/i, 'Script tags are removed' );
unlike( $result, qr/<style/i, 'Style tags are removed' );
unlike( $result, qr/<link/i, 'Link tags are removed' );
unlike( $result, qr/javascript:/i, 'JavaScript URLs are removed' );
like( $result, qr/Plain text content/, 'Plain text content is preserved' );
};
subtest 'comment scrubber functionality' => sub {
plan tests => 10;
my $scrubber = C4::Scrubber->new('comment');
my $test_html =
'<p>Paragraph</p><b>Bold</b><i>Italic</i><em>Emphasis</em><big>Big</big><small>Small</small><strong>Strong</strong><br><u>Underline</u><hr><span>Span</span><div>Div</div><script>Evil</script>';
my $result = $scrubber->scrub($test_html);
like( $result, qr/<b>Bold<\/b>/, 'Bold tags are preserved' );
like( $result, qr/<i>Italic<\/i>/, 'Italic tags are preserved' );
like( $result, qr/<em>Emphasis<\/em>/, 'Em tags are preserved' );
like( $result, qr/<big>Big<\/big>/, 'Big tags are preserved' );
like( $result, qr/<small>Small<\/small>/, 'Small tags are preserved' );
like( $result, qr/<strong>Strong<\/strong>/, 'Strong tags are preserved' );
like( $result, qr/<br>/, 'Break tags are preserved' );
unlike( $result, qr/<p>/, 'Paragraph tags are removed' );
unlike( $result, qr/<span>/, 'Span tags are removed' );
unlike( $result, qr/<script>/, 'Script tags are removed' );
};
subtest 'note scrubber functionality' => sub {
plan tests => 22;
my $scrubber = C4::Scrubber->new('note');
my $comprehensive_html =
'<div><span><p><b>Bold</b><i>Italic</i><em>Emphasis</em><big>Big</big><small>Small</small><strong>Strong</strong><br><u>Underline</u><hr><ol><li>Ordered item 1</li><li>Ordered item 2</li></ol><ul><li>Unordered item 1</li><li>Unordered item 2</li></ul><dl><dt>Term</dt><dd>Definition</dd></dl></p></span></div>';
my $result = $scrubber->scrub($comprehensive_html);
like( $result, qr/<div>/, 'Div tags are preserved' );
like( $result, qr/<span>/, 'Span tags are preserved' );
like( $result, qr/<p>/, 'Paragraph tags are preserved' );
like( $result, qr/<b>Bold<\/b>/, 'Bold tags are preserved' );
like( $result, qr/<i>Italic<\/i>/, 'Italic tags are preserved' );
like( $result, qr/<em>Emphasis<\/em>/, 'Em tags are preserved' );
like( $result, qr/<big>Big<\/big>/, 'Big tags are preserved' );
like( $result, qr/<small>Small<\/small>/, 'Small tags are preserved' );
like( $result, qr/<strong>Strong<\/strong>/, 'Strong tags are preserved' );
like( $result, qr/<br>/, 'Break tags are preserved' );
like( $result, qr/<u>Underline<\/u>/, 'Underline tags are preserved' );
like( $result, qr/<hr>/, 'HR tags are preserved' );
like( $result, qr/<ol>/, 'Ordered list tags are preserved' );
like( $result, qr/<ul>/, 'Unordered list tags are preserved' );
like( $result, qr/<li>/, 'List item tags are preserved' );
like( $result, qr/<dl>/, 'Description list tags are preserved' );
like( $result, qr/<dt>Term<\/dt>/, 'Description term tags are preserved' );
like( $result, qr/<dd>Definition<\/dd>/, 'Description definition tags are preserved' );
is( $result, $comprehensive_html, 'All allowed tags in note scrubber are preserved exactly' );
my $malicious_note = '<p>Safe content</p><script>alert("XSS")</script><iframe src="evil.html"></iframe>';
my $safe_result = $scrubber->scrub($malicious_note);
like( $safe_result, qr/<p>Safe content<\/p>/, 'Safe content is preserved' );
unlike( $safe_result, qr/<script>/, 'Script tags are removed from notes' );
unlike( $safe_result, qr/<iframe>/, 'Iframe tags are removed from notes' );
};