Merge pull request #1803 from RotherOSS/issue-#88-broken_kernel_syste…

…m_xml Issue #88 broken kernel system xml
RotherOSS · Jun 1, 2022 · 8aed5c2 · 8aed5c2
2 parents 5cd0fc4 + bdbb8e0
commit 8aed5c2
Show file tree

Hide file tree

Showing 4 changed files with 178 additions and 102 deletions.
diff --git a/Kernel/System/CommunicationChannel.pm b/Kernel/System/CommunicationChannel.pm
@@ -29,7 +29,6 @@ our @ObjectDependencies = (
     'Kernel::System::Log',
     'Kernel::System::Ticket::Article',
     'Kernel::System::Valid',
-    'Kernel::System::XML',
     'Kernel::System::YAML',
 );
 

diff --git a/Kernel/System/MigrateFromOTRS/OTOBOMigrateWebServiceConfiguration.pm b/Kernel/System/MigrateFromOTRS/OTOBOMigrateWebServiceConfiguration.pm
@@ -39,7 +39,6 @@ our @ObjectDependencies = (
     'Kernel::System::GenericInterface::Webservice',
     'Kernel::System::Main',
     'Kernel::System::Package',
-    'Kernel::System::XML',
 );
 
 =head1 NAME

diff --git a/scripts/test/XML.t b/scripts/test/XML.t
@@ -14,14 +14,21 @@
 # along with this program. If not, see <https://www.gnu.org/licenses/>.
 # --
 
+use v5.24;
 use strict;
 use warnings;
 use utf8;
 
-# Set up the test driver $Self when we are running as a standalone script.
-use Kernel::System::UnitTest::RegisterDriver;
+# core modules
+use Data::Dumper;
 
-use vars (qw($Self));
+# CPAN modules
+use Test2::V0;
+
+# OTOBO modules
+use Kernel::System::UnitTest::RegisterDriver;    # Set up $Kernel::OM and the test driver $Self
+
+our $Self;
 
 my $StorableObject = $Kernel::OM->Get('Kernel::System::Storable');
 my $XMLObject      = $Kernel::OM->Get('Kernel::System::XML');
@@ -35,36 +42,38 @@ $Kernel::OM->ObjectParamAdd(
 );
 my $Helper = $Kernel::OM->Get('Kernel::System::UnitTest::Helper');
 
-# test XMLParse2XMLHash() with an iso-8859-1 encoded XML
-my $String = '<?xml version="1.0" encoding="iso-8859-1" ?>
+subtest 'XMLParse2XMLHash() with an iso-8859-1 encoded XML' => sub {
+
+    my $String = '<?xml version="1.0" encoding="iso-8859-1" ?>
     <Contact>
       <Name type="long">' . "\x{00FC}" . ' Some Test</Name>
     </Contact>
 ';
-my @XMLHash = $XMLObject->XMLParse2XMLHash( String => $String );
-$Self->True(
-    $#XMLHash == 1 && $XMLHash[1]->{Contact},
-    '#1 XMLParse2XMLHash()',
-);
-$Self->Is(
-    $XMLHash[1]->{Contact}->[1]->{Name}->[1]->{type} || '',
-    'long',
-    '#1 XMLParse2XMLHash() (Contact->Name->type)',
-);
+    my @XMLHash = $XMLObject->XMLParse2XMLHash( String => $String );
+    $Self->True(
+        $#XMLHash == 1 && $XMLHash[1]->{Contact},
+        '#1 XMLParse2XMLHash()',
+    );
+    is(
+        $XMLHash[1]->{Contact}->[1]->{Name}->[1]->{type} || '',
+        'long',
+        '#1 XMLParse2XMLHash() (Contact->Name->type)',
+    );
 
-# test charset specific situations
-$Self->Is(
-    $XMLHash[1]->{Contact}->[1]->{Name}->[1]->{Content} || '',
-    "ü Some Test",
-    '#1 XMLParse2XMLHash() (Contact->Name->Content)',
-);
-$Self->True(
-    Encode::is_utf8( $XMLHash[1]->{Contact}->[1]->{Name}->[1]->{Content} ) || '',
-    '#1 XMLParse2XMLHash() (Contact->Name->type) Encode::is_utf8',
-);
+    # test charset specific situations
+    is(
+        $XMLHash[1]->{Contact}->[1]->{Name}->[1]->{Content} || '',
+        "ü Some Test",
+        '#1 XMLParse2XMLHash() (Contact->Name->Content)',
+    );
+    ok(
+        Encode::is_utf8( $XMLHash[1]->{Contact}->[1]->{Name}->[1]->{Content} ) || '',
+        '#1 XMLParse2XMLHash() (Contact->Name->type) Encode::is_utf8',
+    );
+};
 
-# test XMLParse2XMLHash() with utf-8 encoded xml
-$String = '<?xml version="1.0" encoding="utf-8" ?>
+subtest 'XMLParse2XMLHash() with utf-8 encoded xml' => sub {
+    my $String = '<?xml version="1.0" encoding="utf-8" ?>
     <Contact role="admin" type="organization">
       <GermanText>German Umlaute öäü ÄÜÖ ß</GermanText>
       <JapanText>Japan カスタ</JapanText>
@@ -73,56 +82,123 @@ $String = '<?xml version="1.0" encoding="utf-8" ?>
     </Contact>
 ';
 
-@XMLHash = $XMLObject->XMLParse2XMLHash( String => $String );
-$Self->True(
-    $#XMLHash == 1 && $XMLHash[1]->{Contact},
-    '#2 XMLParse2XMLHash()',
-);
-$Self->Is(
-    $XMLHash[1]->{Contact}->[1]->{role} || '',
-    'admin',
-    '#2 XMLParse2XMLHash() (Contact->role)',
-);
+    my @XMLHash = $XMLObject->XMLParse2XMLHash( String => $String );
 
-# test charset specific situations
-$Self->Is(
-    $XMLHash[1]->{Contact}->[1]->{GermanText}->[1]->{Content} || '',
-    'German Umlaute öäü ÄÜÖ ß',
-    '#2 XMLParse2XMLHash() (Contact->GermanText)',
-);
-$Self->True(
-    Encode::is_utf8( $XMLHash[1]->{Contact}->[1]->{GermanText}->[1]->{Content} ) || '',
-    '#2 XMLParse2XMLHash() (Contact->GermanText) Encode::is_utf8',
-);
-$Self->Is(
-    $XMLHash[1]->{Contact}->[1]->{JapanText}->[1]->{Content} || '',
-    'Japan カスタ',
-    '#2 XMLParse2XMLHash() (Contact->JapanText)',
-);
-$Self->True(
-    Encode::is_utf8( $XMLHash[1]->{Contact}->[1]->{JapanText}->[1]->{Content} ) || '',
-    '#2 XMLParse2XMLHash() (Contact->JapanText) Encode::is_utf8',
-);
-$Self->Is(
-    $XMLHash[1]->{Contact}->[1]->{ChineseText}->[1]->{Content} || '',
-    'Chinese 用迎使用',
-    '#2 XMLParse2XMLHash() (Contact->ChineseText)',
-);
-$Self->True(
-    Encode::is_utf8( $XMLHash[1]->{Contact}->[1]->{ChineseText}->[1]->{Content} ) || '',
-    '#2 XMLParse2XMLHash() (Contact->ChineseText) Encode::is_utf8',
-);
-$Self->Is(
-    $XMLHash[1]->{Contact}->[1]->{BulgarianText}->[1]->{Content} || '',
-    'Bulgarian Език',
-    '#2 XMLParse2XMLHash() (Contact->BulgarianText)',
-);
-$Self->True(
-    Encode::is_utf8( $XMLHash[1]->{Contact}->[1]->{BulgarianText}->[1]->{Content} ) || '',
-    '#2 XMLParse2XMLHash() (Contact->BulgarianText) Encode::is_utf8',
-);
+    #diag Dumper($String, \@XMLHash);
+    ok(
+        $#XMLHash == 1 && $XMLHash[1]->{Contact},
+        '#2 XMLParse2XMLHash()',
+    );
+    is(
+        $XMLHash[1]->{Contact}->[1]->{role} || '',
+        'admin',
+        '#2 XMLParse2XMLHash() (Contact->role)',
+    );
 
-$String = '<?xml version="1.0" encoding="utf-8" ?>
+    # test charset specific situations
+    is(
+        $XMLHash[1]->{Contact}->[1]->{GermanText}->[1]->{Content} || '',
+        'German Umlaute öäü ÄÜÖ ß',
+        '#2 XMLParse2XMLHash() (Contact->GermanText)',
+    );
+    ok(
+        Encode::is_utf8( $XMLHash[1]->{Contact}->[1]->{GermanText}->[1]->{Content} ) || '',
+        '#2 XMLParse2XMLHash() (Contact->GermanText) Encode::is_utf8',
+    );
+    is(
+        $XMLHash[1]->{Contact}->[1]->{JapanText}->[1]->{Content} || '',
+        'Japan カスタ',
+        '#2 XMLParse2XMLHash() (Contact->JapanText)',
+    );
+    ok(
+        Encode::is_utf8( $XMLHash[1]->{Contact}->[1]->{JapanText}->[1]->{Content} ) || '',
+        '#2 XMLParse2XMLHash() (Contact->JapanText) Encode::is_utf8',
+    );
+    is(
+        $XMLHash[1]->{Contact}->[1]->{ChineseText}->[1]->{Content} || '',
+        'Chinese 用迎使用',
+        '#2 XMLParse2XMLHash() (Contact->ChineseText)',
+    );
+    ok(
+        Encode::is_utf8( $XMLHash[1]->{Contact}->[1]->{ChineseText}->[1]->{Content} ) || '',
+        '#2 XMLParse2XMLHash() (Contact->ChineseText) Encode::is_utf8',
+    );
+    is(
+        $XMLHash[1]->{Contact}->[1]->{BulgarianText}->[1]->{Content} || '',
+        'Bulgarian Език',
+        '#2 XMLParse2XMLHash() (Contact->BulgarianText)',
+    );
+    ok(
+        Encode::is_utf8( $XMLHash[1]->{Contact}->[1]->{BulgarianText}->[1]->{Content} ) || '',
+        '#2 XMLParse2XMLHash() (Contact->BulgarianText) Encode::is_utf8',
+    );
+};
+
+subtest 'XMLParse2XMLHash() with mixed content' => sub {
+    my $String = <<'END_XML';
+<?xml version="1.0" encoding="utf-8" ?>
+<MixedContent>
+      text A
+      <Tag>Element 1</Tag>
+      text B
+      text C
+      <Tag count="2">Element 2</Tag>
+      <Tag>Element 3</Tag>
+      text D
+      <Tag>Element 4</Tag>
+      text E
+</MixedContent>
+END_XML
+
+    my @XMLHash         = $XMLObject->XMLParse2XMLHash( String => $String );
+    my @ExpectedXMLHash = (
+        undef,
+        {
+            'MixedContent' => [
+                undef,
+                {
+                    'Content' => '
+      text A
+      ',
+                    'Tag' => [
+                        undef,
+                        {
+                            'Content' => 'Element 1',
+                        },
+                        {
+                            'Content' => 'Element 2',
+                            count     => '2',
+                        },
+                        {
+                            'Content' => 'Element 3'
+                        },
+                        {
+                            'Content' => 'Element 4'
+                        }
+                    ]
+                }
+            ]
+        }
+    );
+
+    is(
+        \@XMLHash,
+        \@ExpectedXMLHash,
+        'mixed content is not handled'
+    );
+};
+
+subtest 'XMLParse2XMLHash() with Tag "Content" ' => sub {
+    my $String = <<'END_XML';
+<?xml version="1.0" encoding="utf-8" ?>
+<Foo>the real content<Content>the imposter content</Content></Foo>
+END_XML
+    my $Exception = dies { $XMLObject->XMLParse2XMLHash( String => $String ) };
+    ok( $Exception, 'XML can\'t be parsed' );
+    like( $Exception, qr/Can't use string/, 'some mixup with the tag Content' );
+};
+
+my $String = '<?xml version="1.0" encoding="utf-8" ?>
     <Contact role="admin" type="organization">
       <Name type="long">Example Inc.</Name>
       <Email type="primary">[email protected]<Domain>1234.com</Domain></Email>
@@ -139,7 +215,7 @@ $String = '<?xml version="1.0" encoding="utf-8" ?>
     </Contact>
 ';
 
-@XMLHash = $XMLObject->XMLParse2XMLHash( String => $String );
+my @XMLHash = $XMLObject->XMLParse2XMLHash( String => $String );
 $Self->True(
     $#XMLHash == 1 && $XMLHash[1]->{Contact},
     '#3 XMLParse2XMLHash()',
@@ -681,34 +757,35 @@ else {
 
 # test bug#[12761]
 # (https://bugs.otrs.org/show_bug.cgi?id=12761) - Cache values can be modified from the outside in function XMLParse().
-#
-$XML = '<Test Name="test123" />';
-my @XMLARRAY = $XMLObject->XMLParse( String => $XML );
+{
+    my $XML      = '<Test Name="test123" />';
+    my @XMLARRAY = $XMLObject->XMLParse( String => $XML );
 
-# make a copy of the XMLArray (deep clone it),
-# it will be needed for a later comparison
-my @XMLARRAYCopy = @{ $StorableObject->Clone( Data => \@XMLARRAY ) };
+    #diag Dumper($XML, \@XMLARRAY);
 
-# check that the copy is the same as the original
-$Self->IsDeeply(
-    \@XMLARRAY,
-    \@XMLARRAYCopy,
-    '@XMLARRAY equals @XMLARRAYCopy',
-);
+    # make a copy of the XMLArray (deep clone it),
+    # it will be needed for a later comparison
+    my @XMLARRAYCopy = $StorableObject->Clone( Data => \@XMLARRAY )->@*;
 
-# modify the original, this should not influence the cache of XMLParse()
-$XMLARRAY[0]->{Hello} = 'World';
+    # check that the copy is the same as the original
+    is(
+        \@XMLARRAY,
+        \@XMLARRAYCopy,
+        'Clone: @XMLARRAY equals @XMLARRAYCopy',
+    );
 
-# create a new xml array from the same xml string than the first
-my @XMLARRAY2 = $XMLObject->XMLParse( String => $XML );
+    # modify the original, this should not influence the cache of XMLParse()
+    $XMLARRAY[0]->{Hello} = 'World';
 
-# check that the new array is the same as the original copy
-$Self->IsDeeply(
-    \@XMLARRAY2,
-    \@XMLARRAYCopy,
-    '@XMLARRAY2 equals @XMLARRAYCopy',
-);
+    # create a new xml array from the same xml string than the first
+    my @XMLARRAY2 = $XMLObject->XMLParse( String => $XML );
 
-# cleanup is done by RestoreDatabase
+    # check that the new array is the same as the original copy
+    is(
+        \@XMLARRAY2,
+        \@XMLARRAYCopy,
+        '@XMLARRAY2 equals @XMLARRAYCopy',
+    );
+}
 
-$Self->DoneTesting();
+done_testing();
diff --git a/scripts/test/XML/XXE.t b/scripts/test/XML/XXE.t
@@ -27,6 +27,7 @@ my $ConfigObject = $Kernel::OM->Get('Kernel::Config');
 my $XXEFilename  = $ConfigObject->Get('Home') . '/var/tmp/XXE.t.txt';
 
 # Write XXE payload.
+# XXE stands for XML External Entitiy. Providing XML with external entities is a method of security attacks
 unlink $XXEFilename;
 my $FileCreated = $Kernel::OM->Get('Kernel::System::Main')->FileWrite(
     Location => $XXEFilename,