Re: [PATCH resend] msxml3: Remove CRs in domtext_put_data and add them in domtext_get_xml.

[Nikolay Sivov]

On 03.08.2017 18:17, Alex Henrie wrote:
> Fixes https://bugs.winehq.org/show_bug.cgi?id=42468
>
> To be clear, I am not trying to fix all whitespace processing issues. I
> am only trying to prevent "
" from being erroneously inserted into
> the output.
> Signed-off-by: Alex Henrie
>
> dlls/msxml3/tests/domdoc.c | 106 +++++++++++++++++++++++++++++++++++++--------
>  dlls/msxml3/text.c         |  42 +++++++++++++++---
>  2 files changed, 122 insertions(+), 26 deletions(-)
>
> diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c

I think it needs more tests, for xml:space and put_preserveWhiteSpace().
> diff --git a/dlls/msxml3/text.c b/dlls/msxml3/text.c
> index 3a12d4bf62..6814c1f817 100644
> --- a/dlls/msxml3/text.c
> +++ b/dlls/msxml3/text.c
> @@ -519,10 +519,15 @@ static HRESULT WINAPI domtext_get_xml(
>      BSTR* p)
>  {
>      domtext *This = impl_from_IXMLDOMText( iface );
> -
>
> HRESULT hr;
>
> TRACE("(%p)->(%p)\n", This, p);
> - return node_get_xml(&This->node, FALSE, p);
> - hr = node_get_xml(&This->node, FALSE, p);
> - if (hr == S_OK)
> -
>    *p = EnsureCorrectEOL(*p);
> -
> - return hr;

Will node_get_xml(..., TRUE, ...) work?
> }
>
> static HRESULT WINAPI domtext_transformNode(
> @@ -616,15 +621,38 @@ static HRESULT WINAPI domtext_put_data(
>      BSTR data)
>  {
>      domtext *This = impl_from_IXMLDOMText( iface );
> - static const WCHAR rnW[] = {'\r','\n',0};
> -
>
> BSTR normalized_data = NULL;
> -
>
> HRESULT hr;
> -
>
> int i, j;
>
> TRACE("(%p)->(%s)\n", This, debugstr_w(data));
> - if (data && !strcmpW(rnW, data))
> -
>    This->node.node->name = xmlStringTextNoenc;
> - else
> -
>    domtext_reset_noenc(This);
> - return node_set_content(&This->node, data);
> - if (data)
> - {
> -
>    /* normalize line endings */
> -
>    normalized_data = SysAllocStringLen(NULL, SysStringLen(data));
> -
>    if (!normalized_data) return E_OUTOFMEMORY;
> -
>    for (i = 0, j = 0; data[i]; i++)
> -
>    {
> -
>        if (data[i] == '\r')
> -
>        {
> -
>            if (data[i + 1] == '\n')
> -
>                continue; /* change \r\n to just \n */
> -
>            else
> -
>                normalized_data[j] = '\n'; /* change \r by itself to \n */
> -
>        }
> -
>        else
> -
>            normalized_data[j] = data[i];
> -
>        j++;
> -
>    }
> -
>    normalized_data[j] = 0;

The only purpose for that is to inhibit libxml2 escaping, right? Could
this be done the way it is now, but with something more elaborate than
comparing to "\r\n"?
> - }
> -
> - domtext_reset_noenc(This);
> - hr = node_set_content(&This->node, normalized_data);
> -
> - SysFreeString(normalized_data);
> - return hr;
>
> }
>
> static HRESULT WINAPI domtext_get_length(

[PATCH resend] msxml3: Remove CRs in domtext_put_data and add them in domtext_get_xml.

[Alex Henrie]

2017-08-17 13:34 GMT-06:00 Nikolay Sivov :
> On 03.08.2017 18:17, Alex Henrie wrote:
> > Fixes https://bugs.winehq.org/show_bug.cgi?id=42468
> >
> > To be clear, I am not trying to fix all whitespace processing issues. I
> > am only trying to prevent "
" from being erroneously inserted into
> > the output.
> > Signed-off-by: Alex Henrie
> >
> > dlls/msxml3/tests/domdoc.c | 106 +++++++++++++++++++++++++++++++++++++--------
> >  dlls/msxml3/text.c         |  42 +++++++++++++++---
> >  2 files changed, 122 insertions(+), 26 deletions(-)
> >
> > diff --git a/dlls/msxml3/tests/domdoc.c b/dlls/msxml3/tests/domdoc.c
>
> I think it needs more tests, for xml:space and put_preserveWhiteSpace().

Okay, I've added tests for xml:space="preserve" and preserveWhiteSpace.
> > diff --git a/dlls/msxml3/text.c b/dlls/msxml3/text.c
> > index 3a12d4bf62..6814c1f817 100644
> > --- a/dlls/msxml3/text.c
> > +++ b/dlls/msxml3/text.c
> > @@ -519,10 +519,15 @@ static HRESULT WINAPI domtext_get_xml(
> >      BSTR* p)
> >  {
> >      domtext *This = impl_from_IXMLDOMText( iface );
> > -
> >
> > HRESULT hr;
> >
> > TRACE("(%p)->(%p)\n", This, p);
> > - return node_get_xml(&This->node, FALSE, p);
> > - hr = node_get_xml(&This->node, FALSE, p);
> > - if (hr == S_OK)
> > -
> >    *p = EnsureCorrectEOL(*p);
> > -
> > - return hr;
>
> Will node_get_xml(..., TRUE, ...) work?

Yes, thanks for the suggestion!
> > }
> >
> > static HRESULT WINAPI domtext_transformNode(
> > @@ -616,15 +621,38 @@ static HRESULT WINAPI domtext_put_data(
> >      BSTR data)
> >  {
> >      domtext *This = impl_from_IXMLDOMText( iface );
> > - static const WCHAR rnW[] = {'\r','\n',0};
> > -
> >
> > BSTR normalized_data = NULL;
> > -
> >
> > HRESULT hr;
> > -
> >
> > int i, j;
> >
> > TRACE("(%p)->(%s)\n", This, debugstr_w(data));
> > - if (data && !strcmpW(rnW, data))
> > -
> >    This->node.node->name = xmlStringTextNoenc;
> > - else
> > -
> >    domtext_reset_noenc(This);
> > - return node_set_content(&This->node, data);
> > - if (data)
> > - {
> > -
> >    /* normalize line endings */
> > -
> >    normalized_data = SysAllocStringLen(NULL, SysStringLen(data));
> > -
> >    if (!normalized_data) return E_OUTOFMEMORY;
> > -
> >    for (i = 0, j = 0; data[i]; i++)
> > -
> >    {
> > -
> >        if (data[i] == '\r')
> > -
> >        {
> > -
> >            if (data[i + 1] == '\n')
> > -
> >                continue; /* change \r\n to just \n */
> > -
> >            else
> > -
> >                normalized_data[j] = '\n'; /* change \r by itself to \n */
> > -
> >        }
> > -
> >        else
> > -
> >            normalized_data[j] = data[i];
> > -
> >        j++;
> > -
> >    }
> > -
> >    normalized_data[j] = 0;
>
> The only purpose for that is to inhibit libxml2 escaping, right? Could
> this be done the way it is now, but with something more elaborate than
> comparing to "\r\n"?

No, because we only want to inhibit the escaping of "\r" to "
".
We don't want to inhibit the escaping of other characters such as "&"
and "<".

-Alex