From 926649a9967d28b9f323d44b87849aa0ca621a42 Mon Sep 17 00:00:00 2001
From: =?utf8?q?V=C3=A1clav=20Slav=C3=ADk?= <vslavik@fastmail.fm>
Date: Wed, 4 Aug 2010 14:57:30 +0000
Subject: [PATCH 1/1] Fix attributes escaping when writing XML.

wxXmlDocument didn't correctly escape some characters that the spec says
must be escaped. Behaves correctly now.

Fixes #12275.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@65192 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775
---
 src/xml/xml.cpp       | 109 +++++++++++++++++++++++-------------------
 tests/xml/xmltest.cpp |  25 ++++++++++
 2 files changed, 84 insertions(+), 50 deletions(-)

diff --git a/src/xml/xml.cpp b/src/xml/xml.cpp
index 6c3a9e5d40..5507a4fd77 100644
--- a/src/xml/xml.cpp
+++ b/src/xml/xml.cpp
@@ -821,62 +821,69 @@ bool OutputString(wxOutputStream& stream,
     return stream.IsOk();
 }
 
-// flags for OutputStringEnt()
-enum
+enum EscapingMode
 {
-    XML_ESCAPE_QUOTES = 1
+    Escape_Text,
+    Escape_Attribute
 };
 
 // Same as above, but create entities first.
-// Translates '<' to "&lt;", '>' to "&gt;" and '&' to "&amp;"
-bool OutputStringEnt(wxOutputStream& stream,
-                     const wxString& str,
-                     wxMBConv *convMem,
-                     wxMBConv *convFile,
-                     int flags = 0)
-{
-    const size_t len = str.length();
-    size_t i,
-           last = 0;
-    for (i = 0; i < len; i++)
-    {
-        wxChar c = str.GetChar(i);
-        if (c == wxS('<') || c == wxS('>') ||
-            (c == wxS('&') && str.substr(i+1, 4) != wxS("amp;")) ||
-            ((flags & XML_ESCAPE_QUOTES) && c == wxS('"')))
-        {
-            if ( !OutputString(stream, str.substr(last, i - last),
-                               convMem, convFile) )
-                return false;
+// Translates '<' to "&lt;", '>' to "&gt;" and so on, according to the spec:
+// http://www.w3.org/TR/2000/WD-xml-c14n-20000119.html#charescaping
+bool OutputEscapedString(wxOutputStream& stream,
+                         const wxString& str,
+                         wxMBConv *convMem,
+                         wxMBConv *convFile,
+                         EscapingMode mode)
+{
+    wxString escaped;
+    escaped.reserve(str.length());
 
-            const char *escaped;
-            switch ( c )
-            {
-                case wxS('<'):
-                    escaped = "&lt;";
-                    break;
-                case wxS('>'):
-                    escaped = "&gt;";
-                    break;
-                case wxS('&'):
-                    escaped = "&amp;";
-                    break;
-                case wxS('"'):
-                    escaped = "&quot;";
-                    break;
-                default:
-                    wxFAIL_MSG( "logic error in the code" );
-                    return false;
-            }
+    for ( wxString::const_iterator i = str.begin(); i != str.end(); ++i )
+    {
+        const wxChar c = *i;
 
-            if ( !OutputString(stream, escaped, convMem, convFile) )
-                return false;
+        switch ( c )
+        {
+            case wxS('<'):
+                escaped.append(wxS("&lt;"));
+                break;
+            case wxS('>'):
+                escaped.append(wxS("&gt;"));
+                break;
+            case wxS('&'):
+                escaped.append(wxS("&amp;"));
+                break;
+            case wxS('\r'):
+                escaped.append(wxS("&#xD;"));
+                break;
+            default:
+                if ( mode == Escape_Attribute )
+                {
+                    switch ( c )
+                    {
+                        case wxS('"'):
+                            escaped.append(wxS("&quot;"));
+                            break;
+                        case wxS('\t'):
+                            escaped.append(wxS("&#x9;"));
+                            break;
+                        case wxS('\n'):
+                            escaped.append(wxS("&#xA;"));
+                            break;
+                        default:
+                            escaped.append(c);
+                    }
 
-            last = i + 1;
+                }
+                else
+                {
+                    escaped.append(c);
+                }
         }
     }
 
-    return OutputString(stream, str.substr(last, i - last), convMem, convFile);
+    return OutputString(stream, escaped, convMem, convFile);
 }
 
 bool OutputIndentation(wxOutputStream& stream,
@@ -906,7 +913,9 @@ bool OutputNode(wxOutputStream& stream,
             break;
 
         case wxXML_TEXT_NODE:
-            rc = OutputStringEnt(stream, node->GetContent(), convMem, convFile);
+            rc = OutputEscapedString(stream, node->GetContent(),
+                                     convMem, convFile,
+                                     Escape_Text);
             break;
 
         case wxXML_ELEMENT_NODE:
@@ -922,9 +931,9 @@ bool OutputNode(wxOutputStream& stream,
                     rc = OutputString(stream,
                                       wxS(" ") + attr->GetName() +  wxS("=\""),
                                       convMem, convFile) &&
-                         OutputStringEnt(stream, attr->GetValue(),
-                                         convMem, convFile,
-                                         XML_ESCAPE_QUOTES) &&
+                         OutputEscapedString(stream, attr->GetValue(),
+                                             convMem, convFile,
+                                             Escape_Attribute) &&
                          OutputString(stream, wxS("\""), convMem, convFile);
                 }
             }
diff --git a/tests/xml/xmltest.cpp b/tests/xml/xmltest.cpp
index 9f3d6056a5..79ca9ee3c6 100644
--- a/tests/xml/xmltest.cpp
+++ b/tests/xml/xmltest.cpp
@@ -76,12 +76,14 @@ private:
         CPPUNIT_TEST( InsertChildAfter );
         CPPUNIT_TEST( LoadSave );
         CPPUNIT_TEST( CDATA );
+        CPPUNIT_TEST( Escaping );
     CPPUNIT_TEST_SUITE_END();
 
     void InsertChild();
     void InsertChildAfter();
     void LoadSave();
     void CDATA();
+    void Escaping();
 
     DECLARE_NO_COPY_CLASS(XmlTestCase)
 };
@@ -215,3 +217,26 @@ void XmlTestCase::CDATA()
     // is not
     CPPUNIT_ASSERT_EQUAL( "Giovanni Mittone", n->GetContent() );
 }
+
+void XmlTestCase::Escaping()
+{
+    // Verify that attribute values are escaped correctly, see
+    // http://trac.wxwidgets.org/ticket/12275
+
+    const char *xmlText =
+"<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+"<root text=\"hello&#xD;&#xA;this is a new line\">\n"
+"  <x/>\n"
+"</root>\n"
+    ;
+
+    wxStringInputStream sis(xmlText);
+
+    wxXmlDocument doc;
+    CPPUNIT_ASSERT( doc.Load(sis) );
+
+    wxStringOutputStream sos;
+    CPPUNIT_ASSERT( doc.Save(sos) );
+
+    CPPUNIT_ASSERT_EQUAL( xmlText, sos.GetString() );
+}
-- 
2.45.2