[PATCH] PR19433 - Escape filepaths in abixml

[PATCH] PR19433 - Escape filepaths in abixml

[Ondrej Oprala]

[-- Attachment #1: Type: text/plain, Size: 89 bytes --]

Filepaths can contain '<' and '>', which makes XML parsers unhappy :(

Cheers,
   Ondrej

[-- Attachment #2: 0001-Escape-the-value-of-the-filepath-attribute.patch --]
[-- Type: text/x-patch, Size: 1179 bytes --]

From 6c23fa261bff7cf4eb49237e4d795e3feb8b4e0b Mon Sep 17 00:00:00 2001
From: Ondrej Oprala <ooprala@redhat.com>
Date: Mon, 18 Jan 2016 08:51:21 +0100
Subject: [PATCH] Escape the value of the filepath attribute.

	* src/abg-writer.cc (write_location): Sanitize the filepath with
	xml::escape_xml_string().

Signed-off-by: Ondrej Oprala <ooprala@redhat.com>
---
 src/abg-writer.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/abg-writer.cc b/src/abg-writer.cc
index 62b5748..cfcb61a 100644
--- a/src/abg-writer.cc
+++ b/src/abg-writer.cc
@@ -592,7 +592,7 @@ write_location(const location& loc, ostream& o)
 
   loc.expand(filepath, line, column);
 
-  o << " filepath='" << filepath << "'"
+  o << " filepath='" << xml::escape_xml_string(filepath) << "'"
     << " line='"     << line     << "'"
     << " column='"   << column   << "'";
 }
@@ -620,7 +620,7 @@ write_location(const decl_base_sptr&	decl,
 
   loc.expand(filepath, line, column);
 
-  o << " filepath='" << filepath << "'"
+  o << " filepath='" << xml::escape_xml_string(filepath) << "'"
     << " line='"     << line     << "'"
     << " column='"   << column   << "'";
 }
-- 
2.5.0

Re: [PATCH] PR19433 - Escape filepaths in abixml

[Dodji Seketeli]

Hello,

Ondrej Oprala <ooprala@redhat.com> a écrit:

> Filepaths can contain '<' and '>', which makes XML parsers unhappy :(

Right.

>
> 	* src/abg-writer.cc (write_location): Sanitize the filepath with
> 	xml::escape_xml_string().

I first thought that the reader side should be updated too, to un-escape
the stuff that got escaped, but then I realized that the read_location()
function from abg-reader.cc uses the xmlGetProp() function from libxml2
which actually does "entity substitution" a.k.a un-escaping.  So the
patch is good.

I am wondering, would it be possible to add a small test case that
exhibits the issue?  There are cases where it's too complicated to
devise a small test case, but I am thinking that in this case, it should
be possible fairly easily.  Or am I missing something?

If we agree, you could add it to the test-types-stability.cc test
harness.

OK to commit with that change.

Cheers,

-- 
		Dodji