[PATCH v3 3/3] msxml3: When saving XML use encoding specified in the processing instruction. (Resend)

[Dmitry Timoshkov]

Otherwise when a PI node with not default encoding is assigned to
a document then saving an XML leads to document being encoded as
UTF-8 while XML declaration has what PI specifies.

Signed-off-by: Dmitry Timoshkov 
---
 dlls/msxml3/domdoc.c | 70 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 68 insertions(+), 2 deletions(-)

diff --git a/dlls/msxml3/domdoc.c b/dlls/msxml3/domdoc.c
index c67e570a8ea..45fbd6f43aa 100644
--- a/dlls/msxml3/domdoc.c
+++ b/dlls/msxml3/domdoc.c
@@ -2563,6 +2563,63 @@ static int XMLCALL domdoc_stream_save_closecallback(void *ctx)
     return 0;
 }

+static char *xmldoc_encoding(IXMLDOMDocument3 *doc)
+{
+    HRESULT hr;
+    IXMLDOMNode *node;
+    char *encoding = NULL;
+
+    hr = IXMLDOMDocument3_get_firstChild(doc, &node);
+    if (hr == S_OK)
+    {
+        DOMNodeType type;
+
+        hr = IXMLDOMNode_get_nodeType(node, &type);
+        if (hr == S_OK && type == NODE_PROCESSING_INSTRUCTION)
+        {
+            IXMLDOMProcessingInstruction *pi;
+            IXMLDOMNode *item;
+            IXMLDOMNamedNodeMap *node_map;
+
+            hr = IXMLDOMNode_QueryInterface(node, &IID_IXMLDOMProcessingInstruction, (void **)&pi);
+            if (hr == S_OK)
+            {
+                hr = IXMLDOMNode_get_attributes(node, &node_map);
+                if (hr == S_OK)
+                {
+                    static const WCHAR encodingW[] = {'e','n','c','o','d','i','n','g',0};
+                    BSTR bstr;
+
+                    bstr = SysAllocString(encodingW);
+                    hr = IXMLDOMNamedNodeMap_getNamedItem(node_map, bstr, &item);
+                    SysFreeString(bstr);
+                    if (hr == S_OK)
+                    {
+                        VARIANT var;
+
+                        hr = IXMLDOMNode_get_nodeValue(item, &var);
+                        if (hr == S_OK)
+                        {
+                            if (V_VT(&var) == VT_BSTR)
+                                encoding = (char *)xmlchar_from_wchar(V_BSTR(&var));
+
+                            VariantClear(&var);
+                        }
+                    }
+
+                    IXMLDOMNamedNodeMap_Release(node_map);
+                }
+
+                IXMLDOMProcessingInstruction_Release(pi);
+            }
+        }
+
+        IXMLDOMNode_Release(node);
+    }
+
+    return encoding;
+}
+
 static HRESULT WINAPI domdoc_save(
     IXMLDOMDocument3 *iface,
     VARIANT destination )
@@ -2601,8 +2658,12 @@ static HRESULT WINAPI domdoc_save(
             ret = IUnknown_QueryInterface(pUnk, &IID_IStream, (void**)&stream);
             if(ret == S_OK)
             {
+                char *encoding = xmldoc_encoding(iface);
+
+                TRACE("using encoding %s\n", encoding ? debugstr_a(encoding) : "default");
                 ctx = xmlSaveToIO(domdoc_stream_save_writecallback,
-                    domdoc_stream_save_closecallback, stream, NULL, XML_SAVE_NO_DECL);
+                    domdoc_stream_save_closecallback, stream, encoding, XML_SAVE_NO_DECL);
+                heap_free(encoding);

if(!ctx)
                 {
@@ -2616,6 +2677,7 @@ static HRESULT WINAPI domdoc_save(
     case VT_BSTR:
     case VT_BSTR | VT_BYREF:
         {
+            char *encoding;
             /* save with file path */
             HANDLE handle = CreateFileW( (V_VT(&destination) & VT_BYREF)? *V_BSTRREF(&destination) : V_BSTR(&destination),
                                          GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL );
@@ -2625,8 +2687,12 @@ static HRESULT WINAPI domdoc_save(
                 return E_FAIL;
             }

+            encoding = xmldoc_encoding(iface);
+            TRACE("using encoding %s\n", encoding ? debugstr_a(encoding) : "default");
             ctx = xmlSaveToIO(domdoc_save_writecallback, domdoc_save_closecallback,
-                              handle, NULL, XML_SAVE_NO_DECL);
+                              handle, encoding, XML_SAVE_NO_DECL);
+            heap_free(encoding);
+
             if (!ctx)
             {
                 CloseHandle(handle);

[Nikolay Sivov]

Is save() part testable? Something close to what your application needs
seems useful to have in tests. Does it load with one encoding, then
changes it and saves?

[Dmitry Timoshkov]

Nikolay Sivov  wrote:
> Is save() part testable? Something close to what your application needs
> seems useful to have in tests. Does it load with one encoding, then
> changes it and saves?

The application creates XML in run-time, saves it and then loads from the file.
There's a test for this already in test_createProcessingInstruction(), however
testing encoding of the contents requires using some cyrillic instead of "test",
so I decided to avoid that.

[Dmitry Timoshkov]

Dmitry Timoshkov  wrote:
> Nikolay Sivov  wrote:
> > Is save() part testable? Something close to what your application needs
> > seems useful to have in tests. Does it load with one encoding, then
> > changes it and saves?
>
> The application creates XML in run-time, saves it and then loads from the file.
> There's a test for this already in test_createProcessingInstruction(), however
> testing encoding of the contents requires using some cyrillic instead of "test",
> so I decided to avoid that.

Do you need any other clarifications, or the patches might be accepted as is?

[Nikolay Sivov]

On 5/31/21 1:10 PM, Dmitry Timoshkov wrote:
> Dmitry Timoshkov  wrote:
> > Nikolay Sivov  wrote:
> > > Is save() part testable? Something close to what your application needs
> > > seems useful to have in tests. Does it load with one encoding, then
> > > changes it and saves?
> >
> > The application creates XML in run-time, saves it and then loads from the file.
> > There's a test for this already in test_createProcessingInstruction(), however
> > testing encoding of the contents requires using some cyrillic instead of "test",
> > so I decided to avoid that.
>
> Do you need any other clarifications, or the patches might be accepted as is?

Let's have them as is. Would still be nice to have that saving test that
shows that correct encoding is used, as a follow up patch.

[Nikolay Sivov]

Signed-off-by: Nikolay Sivov