RT #58323: Started as making the parser interfaces correctly report e…

…rrors

when passed null-length strings or "0" values. Turned out that the error
return interface from XMLLibXML.pm was not consistent with the rest of the
system, so fixed that as well.

lib/RPC/XML/Parser/XMLLibXML.pm

@@ -42,7 +42,7 @@ use base 'RPC::XML::Parser';
 use Scalar::Util 'reftype';
 use XML::LibXML;
 
-$VERSION = '0.101';
+$VERSION = '0.11';
 $VERSION = eval $VERSION; ## no critic (ProhibitStringyEval)
 
 # This is to identify valid types that don't already have special handling
@@ -92,7 +92,10 @@ sub parse
 
     my $parser = XML::LibXML->new(no_network => 1);
 
-    if (! $stream)
+    # RT58323: It's not enough to just test $stream, I have to check
+    # defined-ness. A 0 or null-string should yield an error, not a push-parser
+    # instance.
+    if (! defined $stream)
     {
         # If no stream is given, initialize the DOM push-parser interface and
         # return the object ref
@@ -104,16 +107,34 @@ sub parse
 
     # Determine if the stream is a string or a filehandle, and use the apropos
     # method to parse it.
-    my $doc;
+    my ($doc, $result);
     if (ref $stream)
     {
         if (reftype($stream) eq 'GLOB')
         {
-            $doc = $parser->parse_fh($stream);
+            $result = eval {
+                $doc = $parser->parse_fh($stream);
+                1;
+            };
+            if (! $result)
+            {
+                # Certain cases cause $@ to be a XML::LibXML::Error object
+                # instead of a string. So force it to stringify with "".
+                return "$@";
+            }
         }
         elsif (reftype($stream) eq 'SCALAR')
         {
-            $doc = $parser->parse_string(${$stream});
+            $result = eval {
+                $doc = $parser->parse_string(${$stream});
+                1;
+            };
+            if (! $result)
+            {
+                # Certain cases cause $@ to be a XML::LibXML::Error object
+                # instead of a string. So force it to stringify with "".
+                return "$@";
+            }
         }
         else
         {
@@ -122,7 +143,16 @@ sub parse
     }
     else
     {
-        $doc = $parser->parse_string($stream);
+        $result = eval {
+            $doc = $parser->parse_string($stream);
+            1;
+        };
+        if (! $result)
+        {
+            # Certain cases cause $@ to be a XML::LibXML::Error object
+            # instead of a string. So force it to stringify with "".
+            return "$@";
+        }
     }
 
     return $self->dom_to_obj($doc);

lib/RPC/XML/Parser/XMLParser.pm

@@ -98,7 +98,7 @@ use XML::Parser;
 
 require RPC::XML;
 
-$VERSION = '1.20';
+$VERSION = '1.21';
 $VERSION = eval $VERSION; ## no critic (ProhibitStringyEval)
 
 ###############################################################################
@@ -175,7 +175,10 @@ sub parse
 
     # If there is no stream given, then create an incremental parser handle
     # and return it.
-    if (! $stream)
+    # RT58323: It's not enough to just test $stream, I have to check
+    # defined-ness. A 0 or null-string should yield an error, not a push-parser
+    # instance.
+    if (! defined $stream)
     {
         return $parser->parse_start();
     }

t/90_rt58323_push_parser.t

@@ -0,0 +1,52 @@
+#!/usr/bin/perl
+
+# https://rt.cpan.org/Ticket/Display.html?id=58323
+#
+# Test that the parser-factory instance classes correctly cause errors when
+# passed null strings or 0 as an argument to parse().
+
+use strict;
+use vars qw($parser $eval_result $parse_result);
+
+use Test::More tests => 4;
+
+# This factory-class-instance should always be present
+require RPC::XML::Parser::XMLParser;
+# This one may not be present
+my $can_libxml = eval {
+    require RPC::XML::Parser::XMLLibXML;
+    1;
+};
+
+# To test this, instantiate each parser then call the ->parse() method with
+# both a null string and with 0 as an argument. Each call should throw an
+# error about failed parsing. If they don't, the test has failed.
+
+# First test the class we always have, RPC::XML::Parser::XMLParser
+$parser = RPC::XML::Parser::XMLParser->new();
+
+# Empty string
+$parse_result = $parser->parse(q{});
+ok(! ref($parse_result), 'RPC::XML::Parser::XMLParser null string');
+
+# Zero
+$parse_result = $parser->parse(0);
+ok(! ref($parse_result), 'RPC::XML::Parser::XMLParser zero value');
+
+# Next, test RPC::XML::Parser::XMLLibXML (which we might not have)
+SKIP: {
+    skip 'XML::LibXML not installed', 2
+        unless $can_libxml;
+
+    $parser = RPC::XML::Parser::XMLLibXML->new();
+
+    # Empty string
+    $parse_result = $parser->parse(q{});
+    ok(! ref($parse_result), 'RPC::XML::Parser::XMLLibXML null string');
+
+    # Zero
+    $parse_result = $parser->parse(0);
+    ok(! ref($parse_result), 'RPC::XML::Parser::XMLLibXML zero value');
+}
+
+exit;