e60c42c
From 3d0adda7560137309be8b10c63ff41e41dfb1516 Mon Sep 17 00:00:00 2001
e60c42c
From: =?UTF-8?q?Petr=20P=C3=ADsa=C5=99?= <ppisar@redhat.com>
e60c42c
Date: Tue, 28 Jan 2020 17:05:32 +0100
e60c42c
Subject: [PATCH] Parse an ampersand entity in SAX interface
e60c42c
MIME-Version: 1.0
e60c42c
Content-Type: text/plain; charset=UTF-8
e60c42c
Content-Transfer-Encoding: 8bit
e60c42c
e60c42c
After disabling parsing external entities in XML-LibXML-2.0202,
e60c42c
XML::LibXML::SAX interface stopped expanding & and & entities
e60c42c
in attribute values (often found in href XHTML attributes) and
e60c42c
returned "&" instead. This was discovered by a RDF-Trine test
e60c42c
suite failure <https://github.com/kasei/perlrdf/issues/167>.
e60c42c
e60c42c
First, I suspected XML-LibXML
e60c42c
<https://rt.cpan.org/Ticket/Display.html?id=131498>, but it turned out
e60c42c
that the unexpanded entity comes from libxml2 C library itself. And
e60c42c
that it's not just an ommitted expansion, but that it's actually an
e60c42c
escape sequence for "&" characters. Other XML metacharacters (like
e60c42c
"<") are not affeced. Also text nodes are also not affected.  My
e60c42c
finding was confirmed by an old libxml2 bug report
e60c42c
<https://bugzilla.gnome.org/show_bug.cgi?id=316487>.
e60c42c
e60c42c
This patch "fixes" this discepancy by replacing all "&"
e60c42c
subtstrings with a literal "&" in SAX interface of start_element()
e60c42c
callbacks.
e60c42c
e60c42c
Signed-off-by: Petr Písař <ppisar@redhat.com>
e60c42c
---
e60c42c
 MANIFEST          |  1 +
e60c42c
 perl-libxml-sax.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
e60c42c
 t/52_sax_intent.t | 40 ++++++++++++++++++++++++++++++++++++++++
e60c42c
 3 files changed, 83 insertions(+), 2 deletions(-)
e60c42c
 create mode 100755 t/52_sax_intent.t
e60c42c
e60c42c
diff --git a/MANIFEST b/MANIFEST
e60c42c
index 5248ea5..ccc3410 100644
e60c42c
--- a/MANIFEST
e60c42c
+++ b/MANIFEST
e60c42c
@@ -174,6 +174,7 @@ t/49callbacks_returning_undef.t
e60c42c
 t/49global_extent.t
e60c42c
 t/50devel.t
e60c42c
 t/51_parse_html_string_rt87089.t
e60c42c
+t/52_sax_intent.t
e60c42c
 t/60error_prev_chain.t
e60c42c
 t/60struct_error.t
e60c42c
 t/61error.t
e60c42c
diff --git a/perl-libxml-sax.c b/perl-libxml-sax.c
e60c42c
index b949d3c..232a879 100644
e60c42c
--- a/perl-libxml-sax.c
e60c42c
+++ b/perl-libxml-sax.c
e60c42c
@@ -20,6 +20,7 @@ extern "C" {
e60c42c
 #include "ppport.h"
e60c42c
 
e60c42c
 #include <stdlib.h>
e60c42c
+#include <string.h>
e60c42c
 #include <libxml/xmlmemory.h>
e60c42c
 #include <libxml/parser.h>
e60c42c
 #include <libxml/parserInternals.h>
e60c42c
@@ -639,6 +640,34 @@ PmmGenNsName( const xmlChar * name, const xmlChar * nsURI )
e60c42c
     return retval;
e60c42c
 }
e60c42c
 
e60c42c
+/* If a value argument does not contain "&", the value pointer is returned.
e60c42c
+ * Otherwise a new xmlChar * string is allocated, the value copied there and
e60c42c
+ * "&" occurences replaced with "&". Then the caller must free it. */
e60c42c
+static
e60c42c
+xmlChar *
e60c42c
+_expandAmp( const xmlChar *value )
e60c42c
+{
e60c42c
+    xmlChar *expanded = NULL;
e60c42c
+    const xmlChar *entity;
e60c42c
+    int length;
e60c42c
+
e60c42c
+    if (value == NULL ||
e60c42c
+            (NULL == (entity = (const xmlChar *)strstr((const char *)value, "&")))) {
e60c42c
+        return (xmlChar *)value;
e60c42c
+    }
e60c42c
+
e60c42c
+    do {
e60c42c
+        length = entity - value;
e60c42c
+        expanded = xmlStrncat(expanded, value, length);
e60c42c
+        expanded = xmlStrncat(expanded, (const xmlChar *)"&", 1);
e60c42c
+        value += length + 5; /* "&" */
e60c42c
+    } while (NULL != (entity = (const xmlChar*)strstr((const char *)value, "&")));
e60c42c
+
e60c42c
+    expanded = xmlStrcat(expanded, value);
e60c42c
+
e60c42c
+    return expanded;
e60c42c
+}
e60c42c
+
e60c42c
 HV *
e60c42c
 PmmGenAttributeHashSV( pTHX_ PmmSAXVectorPtr sax,
e60c42c
                        const xmlChar **attr, SV * handler )
e60c42c
@@ -653,8 +682,8 @@ PmmGenAttributeHashSV( pTHX_ PmmSAXVectorPtr sax,
e60c42c
     const xmlChar * nsURI = NULL;
e60c42c
     const xmlChar **ta    = attr;
e60c42c
     const xmlChar * name  = NULL;
e60c42c
-    const xmlChar * value = NULL;
e60c42c
 
e60c42c
+    xmlChar * value       = NULL;
e60c42c
     xmlChar * keyname     = NULL;
e60c42c
     xmlChar * localname   = NULL;
e60c42c
     xmlChar * prefix      = NULL;
e60c42c
@@ -665,7 +694,13 @@ PmmGenAttributeHashSV( pTHX_ PmmSAXVectorPtr sax,
e60c42c
         while ( *ta != NULL ) {
e60c42c
             atV = newHV();
e60c42c
             name = *ta;  ta++;
e60c42c
-            value = *ta; ta++;
e60c42c
+            /* XXX: libxml2 SAX2 interface does not expand &
e60c42c
+             * entity in the attribute values
e60c42c
+             * <https://bugzilla.gnome.org/show_bug.cgi?id=316487>
e60c42c
+             * resulting in stray "&" sequences after disabling
e60c42c
+             * external entity expansion
e60c42c
+             * <https://rt.cpan.org/Ticket/Display.html?id=131498>. */
e60c42c
+            value = _expandAmp(*ta);
e60c42c
 
e60c42c
             if ( name != NULL && XML_STR_NOT_EMPTY( name ) ) {
e60c42c
                 localname = xmlSplitQName(NULL, name, &prefix);
e60c42c
@@ -754,6 +789,11 @@ PmmGenAttributeHashSV( pTHX_ PmmSAXVectorPtr sax,
e60c42c
                 prefix    = NULL;
e60c42c
 
e60c42c
             }
e60c42c
+
e60c42c
+            if (value != *ta) {
e60c42c
+                xmlFree(value);
e60c42c
+            }
e60c42c
+            ta++;
e60c42c
         }
e60c42c
     }
e60c42c
 
e60c42c
diff --git a/t/52_sax_intent.t b/t/52_sax_intent.t
e60c42c
new file mode 100755
e60c42c
index 0000000..a45b4d1
e60c42c
--- /dev/null
e60c42c
+++ b/t/52_sax_intent.t
e60c42c
@@ -0,0 +1,40 @@
e60c42c
+use strict;
e60c42c
+use warnings;
e60c42c
+use Test::More;
e60c42c
+
e60c42c
+my %tests = (
e60c42c
+    # attribte name     raw attrib. value   expected parsed value
e60c42c
+    predefined =>       ['"',          '"'],       # alawys worked
e60c42c
+    numeric =>          ['A',           'A'],       # always worked
e60c42c
+    numericampersand => ['&',           '&'],       # a regression
e60c42c
+    ampA =>             ['&A',          '&A'],      # a corner case
e60c42c
+    Aamp =>             ['A&',          'A&'],      # a corner case
e60c42c
+    AampBampC =>        ['A&B&C',   'A&B&C'],   # a corner case
e60c42c
+);
e60c42c
+plan tests => scalar (keys %tests);
e60c42c
+
e60c42c
+my $input = '
e60c42c
+for my $test (sort keys %tests) {
e60c42c
+    $input .= sprintf(" %s='%s'", $test, $tests{$test}->[0]);
e60c42c
+}
e60c42c
+$input .= '/>';
e60c42c
+
e60c42c
+diag("Parsing $input");
e60c42c
+use XML::LibXML::SAX;
e60c42c
+
e60c42c
+XML::LibXML::SAX->new(Handler => 'Handler')->parse_string($input);
e60c42c
+
e60c42c
+
e60c42c
+package Handler;
e60c42c
+sub start_element {
e60c42c
+    my ($self, $node) = @_;
e60c42c
+    for my $attribute (sort keys %{$node->{Attributes}}) {
e60c42c
+        my $name = $node->{Attributes}->{$attribute}->{Name};
e60c42c
+        Test::More::is(
e60c42c
+            $node->{Attributes}->{$attribute}->{Value},
e60c42c
+            $tests{$name}->[1],
e60c42c
+            sprintf("%s='%s' attribute", $name, $tests{$name}->[0])
e60c42c
+        );
e60c42c
+    }
e60c42c
+}
e60c42c
+
e60c42c
-- 
e60c42c
2.21.1
e60c42c