[PATCH] msxml3: all string passed to IXMLDOMDocument_load() need to be URL-unescaped

Damjan Jovanovic damjan.jov at gmail.com
Thu Dec 5 12:08:20 CST 2019


On Thu, Dec 5, 2019 at 1:45 PM Nikolay Sivov <nsivov at codeweavers.com> wrote:

> Do we need to unescape if it's not a file:// url? I was hoping we could
> find some IUri/path API flags to do what we need for us.
>
>
The CreateUri() function called at the end of create_uri() does its own
unescaping, which is why I didn't pass it the unescaped URL, but left it
with the original URL. My unescaping is done for the benefit of the earlier
function, PathIsURLW(), which doesn't unescape internally, and for
PathSearchAndQualifyW() and UrlCreateFromPathW(), which require paths not
URLs, and paths are always unescaped.



> Another concern is path/url max lengths are used seemingly randomly, and
> it's not a problem with your change, but with existing code. I think
> asking for required length and allocating it is better.
>
>
Sure.


> +    /* Regular local path with some URL encoded characters. */
> +    strcpy(path2, path);
> +    n = strlen(path2);
> +    path2[n-1] = '%';
> +    path2[n] = '6';
> +    path2[n+1] = 'C';
> +    path2[n+2] = '\0';   /* C:\path\to\winetest.xm%6C */
> +    test_doc_load_from_path(doc, path2);
>
> Could you make this more readable? Maybe strcat-ing escaped file name as
> string literal instead.
>

Will do.


>
> +    /* Regular local path with all URL encoded characters. */
> +    percent_path = HeapAlloc(GetProcessHeap(), 0, 3*n + 1);
> +    for (i = 0; i < n; i++)
> +    {
> +        static char hex_tab[] = "0123456789ABCDEF";
> +        percent_path[3*i] = '%';
> +        percent_path[3*i + 1] = hex_tab[path[i] >> 4];
> +        percent_path[3*i + 2] = hex_tab[path[i] & 0xF];
> +    }
> +    percent_path[3*n] = '\0';
> +    test_doc_load_from_path(doc, percent_path);
> +    HeapFree(GetProcessHeap(), 0, percent_path);
>
> Couple of cases would be enough, like you did earlier for "l" -> %6c, and
> another one for " " -> %20. Space is actually not tested by this patch, and
> that's what application is using. If you still want to encode it entirely,
> please add a helper function.
>

Agreed. It would actually be good to test with both a space and %20.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20191205/bfa09e16/attachment.htm>


More information about the wine-devel mailing list