shlwapi: Fix the PathCreateFromUrlW() implementation.

dlls/shlwapi/path.c

@@ -3286,62 +3286,137 @@ HRESULT WINAPI PathCreateFromUrlW(LPCWSTR pszUrl, LPWSTR pszPath,
                                   LPDWORD pcchPath, DWORD dwReserved)
 {
     static const WCHAR file_colon[] = { 'f','i','l','e',':',0 };
-    HRESULT hr;
-    DWORD nslashes = 0;
-    WCHAR *ptr;
+    static const WCHAR localhost[] = { 'l','o','c','a','l','h','o','s','t',0 };
+    DWORD nslashes, unescape, len;
+    const WCHAR *src;
+    WCHAR *tpath, *dst;
+    HRESULT ret;
 
     TRACE("(%s,%p,%p,0x%08x)\n", debugstr_w(pszUrl), pszPath, pcchPath, dwReserved);
 
     if (!pszUrl || !pszPath || !pcchPath || !*pcchPath)
         return E_INVALIDARG;
 
-
-    if (strncmpW(pszUrl, file_colon, 5))
+    if (CompareStringW(LOCALE_INVARIANT, NORM_IGNORECASE, pszUrl, 5,
+                       file_colon, 5) != CSTR_EQUAL)
         return E_INVALIDARG;
     pszUrl += 5;
+    ret = S_OK;
 
-    while(*pszUrl == '/' || *pszUrl == '\\') {
+    src = pszUrl;
+    nslashes = 0;
+    while (*src == '/' || *src == '\\') {
         nslashes++;
-        pszUrl++;
+        src++;
     }
 
-    if(isalphaW(*pszUrl) && (pszUrl[1] == ':' || pszUrl[1] == '|') && (pszUrl[2] == '/' || pszUrl[2] == '\\'))
-        nslashes = 0;
+    /* We need a temporary buffer so we can compute what size to ask for.
+     * We know that the final string won't be longer than the current pszUrl
+     * plus at most two backslashes. All the other transformations make it
+     * shorter.
+     */
+    len = 2 + lstrlenW(pszUrl) + 1;
+    if (*pcchPath < len)
+        tpath = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR));
+    else
+        tpath = pszPath;
 
-    switch(nslashes) {
-    case 2:
-        pszUrl -= 2;
-        break;
+    len = 0;
+    dst = tpath;
+    unescape = 1;
+    switch (nslashes)
+    {
     case 0:
+        /* 'file:' + escaped DOS path */
         break;
-    default:
-        pszUrl -= 1;
+    case 1:
+        /* 'file:/' + escaped DOS path */
+        /* fall through */
+    case 3:
+        /* 'file:///' (implied localhost) + escaped DOS path */
+        if (!isalphaW(*src) || (src[1] != ':' && src[1] != '|'))
+            src -= 1;
+        break;
+    case 2:
+        if (CompareStringW(LOCALE_INVARIANT, NORM_IGNORECASE, src, 9,
+                           localhost, 9) == CSTR_EQUAL &&
+            (src[9] == '/' || src[9] == '\\'))
+        {
+            /* 'file://localhost/' + escaped DOS path */
+            src += 10;
+        }
+        else if (isalphaW(*src) && (src[1] == ':' || src[1] == '|'))
+        {
+            /* 'file://' + unescaped DOS path */
+            unescape = 0;
+        }
+        else
+        {
+            /*    'file://hostname:port/path' (where path is escaped)
+             * or 'file:' + escaped UNC path (\\server\share\path)
+             * The second form is clearly specific to Windows and it might
+             * even be doing a network lookup to try to figure it out.
+             */
+            while (*src && *src != '/' && *src != '\\')
+                src++;
+            len = src - pszUrl;
+            StrCpyNW(dst, pszUrl, len + 1);
+            dst += len;
+            if (isalphaW(src[1]) && (src[2] == ':' || src[2] == '|'))
+            {
+                /* 'Forget' to add a trailing '/', just like Windows */
+                src++;
+            }
+        }
         break;
+    case 4:
+        /* 'file://' + unescaped UNC path (\\server\share\path) */
+        unescape = 0;
+        if (isalphaW(*src) && (src[1] == ':' || src[1] == '|'))
+            break;
+        /* fall through */
+    default:
+        /* 'file:/...' + escaped UNC path (\\server\share\path) */
+        src -= 2;
     }
 
-    hr = UrlUnescapeW((LPWSTR)pszUrl, pszPath, pcchPath, 0);
-    if(hr != S_OK) return hr;
-
-    for(ptr = pszPath; *ptr; ptr++)
-        if(*ptr == '/') *ptr = '\\';
+    /* Copy the remainder of the path */
+    len += lstrlenW(src);
+    StrCpyW(dst, src);
 
-    while(*pszPath == '\\')
-        pszPath++;
- 
-    if(isalphaW(*pszPath) && pszPath[1] == '|' && pszPath[2] == '\\') /* c|\ -> c:\ */
-        pszPath[1] = ':';
+     /* First do the Windows-specific path conversions */
+    for (dst = tpath; *dst; dst++)
+        if (*dst == '/') *dst = '\\';
+    if (isalphaW(*tpath) && tpath[1] == '|')
+        tpath[1] = ':'; /* c| -> c: */
 
-    if(nslashes == 2 && (ptr = strchrW(pszPath, '\\'))) { /* \\host\c:\ -> \\hostc:\ */
-        ptr++;
-        if(isalphaW(*ptr) && (ptr[1] == ':' || ptr[1] == '|') && ptr[2] == '\\') {
-            memmove(ptr - 1, ptr, (strlenW(ptr) + 1) * sizeof(WCHAR));
-            (*pcchPath)--;
+    /* And only then unescape the path (i.e. escaped slashes are left as is) */
+    if (unescape)
+    {
+        ret = UrlUnescapeW(tpath, NULL, &len, URL_UNESCAPE_INPLACE);
+        if (ret == S_OK)
+        {
+            /* When working in-place UrlUnescapeW() does not set len */
+            len = lstrlenW(tpath);
         }
     }
 
-    TRACE("Returning %s\n",debugstr_w(pszPath));
+    if (*pcchPath < len + 1)
+    {
+        ret = E_POINTER;
+        *pcchPath = len + 1;
+    }
+    else
+    {
+        *pcchPath = len;
+        if (tpath != pszPath)
+            StrCpyW(pszPath, tpath);
+    }
+    if (tpath != pszPath)
+      HeapFree(GetProcessHeap(), 0, tpath);
 
-    return hr;
+    TRACE("Returning (%u) %s\n", *pcchPath, debugstr_w(pszPath));
+    return ret;
 }
 
 /*************************************************************************

dlls/shlwapi/tests/path.c

@@ -46,9 +46,9 @@ static const struct {
     {"file:c|/foo/bar", "c:\\foo\\bar", S_OK, 0},
     {"file:cx|/foo/bar", "cx|\\foo\\bar", S_OK, 0},
     {"file:c:foo/bar", "c:foo\\bar", S_OK, 0},
-    {"file:c|foo/bar", "c:foo\\bar", S_OK, 0x2},
-    {"file:c:/foo%20ba%2fr", "c:\\foo ba/r", S_OK, 0x2},
-    {"file:foo%20ba%2fr", "foo ba/r", S_OK, 0x2},
+    {"file:c|foo/bar", "c:foo\\bar", S_OK, 0},
+    {"file:c:/foo%20ba%2fr", "c:\\foo ba/r", S_OK, 0},
+    {"file:foo%20ba%2fr", "foo ba/r", S_OK, 0},
     {"file:foo/bar/", "foo\\bar\\", S_OK, 0},
 
     /* 1 leading (back)slash */
@@ -56,10 +56,10 @@ static const struct {
     {"file:\\c:/foo/bar", "c:\\foo\\bar", S_OK, 0},
     {"file:/c|/foo/bar", "c:\\foo\\bar", S_OK, 0},
     {"file:/cx|/foo/bar", "\\cx|\\foo\\bar", S_OK, 0},
-    {"file:/c:foo/bar", "c:foo\\bar", S_OK, 0x2},
-    {"file:/c|foo/bar", "c:foo\\bar", S_OK, 0x2},
-    {"file:/c:/foo%20ba%2fr", "c:\\foo ba/r", S_OK, 0x2},
-    {"file:/foo%20ba%2fr", "\\foo ba/r", S_OK, 0x2},
+    {"file:/c:foo/bar", "c:foo\\bar", S_OK, 0},
+    {"file:/c|foo/bar", "c:foo\\bar", S_OK, 0},
+    {"file:/c:/foo%20ba%2fr", "c:\\foo ba/r", S_OK, 0},
+    {"file:/foo%20ba%2fr", "\\foo ba/r", S_OK, 0},
     {"file:/foo/bar/", "\\foo\\bar\\", S_OK, 0},
 
     /* 2 leading (back)slashes */
@@ -67,52 +67,52 @@ static const struct {
     {"file://c:/d:/foo/bar", "c:\\d:\\foo\\bar", S_OK, 0},
     {"file://c|/d|/foo/bar", "c:\\d|\\foo\\bar", S_OK, 0},
     {"file://cx|/foo/bar", "\\\\cx|\\foo\\bar", S_OK, 0},
-    {"file://c:foo/bar", "c:foo\\bar", S_OK, 0x2},
-    {"file://c|foo/bar", "c:foo\\bar", S_OK, 0x2},
-    {"file://c:/foo%20ba%2fr", "c:\\foo%20ba%2fr", S_OK, 0x2},
+    {"file://c:foo/bar", "c:foo\\bar", S_OK, 0},
+    {"file://c|foo/bar", "c:foo\\bar", S_OK, 0},
+    {"file://c:/foo%20ba%2fr", "c:\\foo%20ba%2fr", S_OK, 0},
     {"file://c%3a/foo/../bar", "\\\\c:\\foo\\..\\bar", S_OK, 0},
-    {"file://c%7c/foo/../bar", "\\\\c|\\foo\\..\\bar", S_OK, 0x2},
-    {"file://foo%20ba%2fr", "\\\\foo ba/r", S_OK, 0x2},
-    {"file://localhost/c:/foo/bar", "c:\\foo\\bar", S_OK, 0x6},
-    {"file://localhost/c:/foo%20ba%5Cr", "c:\\foo ba\\r", S_OK, 0x6},
-    {"file://LocalHost/c:/foo/bar", "c:\\foo\\bar", S_OK, 0x6},
-    {"file:\\\\localhost\\c:\\foo\\bar", "c:\\foo\\bar", S_OK, 0x6},
+    {"file://c%7c/foo/../bar", "\\\\c|\\foo\\..\\bar", S_OK, 0},
+    {"file://foo%20ba%2fr", "\\\\foo ba/r", S_OK, 0},
+    {"file://localhost/c:/foo/bar", "c:\\foo\\bar", S_OK, 0},
+    {"file://localhost/c:/foo%20ba%5Cr", "c:\\foo ba\\r", S_OK, 0},
+    {"file://LocalHost/c:/foo/bar", "c:\\foo\\bar", S_OK, 0},
+    {"file:\\\\localhost\\c:\\foo\\bar", "c:\\foo\\bar", S_OK, 0},
     {"file://incomplete", "\\\\incomplete", S_OK, 0},
 
     /* 3 leading (back)slashes (omitting hostname) */
     {"file:///c:/foo/bar", "c:\\foo\\bar", S_OK, 0},
-    {"File:///c:/foo/bar", "c:\\foo\\bar", S_OK, 0x1},
-    {"file:///c:/foo%20ba%2fr", "c:\\foo ba/r", S_OK, 0x2},
-    {"file:///foo%20ba%2fr", "\\foo ba/r", S_OK, 0x2},
+    {"File:///c:/foo/bar", "c:\\foo\\bar", S_OK, 0},
+    {"file:///c:/foo%20ba%2fr", "c:\\foo ba/r", S_OK, 0},
+    {"file:///foo%20ba%2fr", "\\foo ba/r", S_OK, 0},
     {"file:///foo/bar/", "\\foo\\bar\\", S_OK, 0},
     {"file:///localhost/c:/foo/bar", "\\localhost\\c:\\foo\\bar", S_OK, 0},
 
     /* 4 leading (back)slashes */
     {"file:////c:/foo/bar", "c:\\foo\\bar", S_OK, 0},
-    {"file:////c:/foo%20ba%2fr", "c:\\foo%20ba%2fr", S_OK, 0x2},
-    {"file:////foo%20ba%2fr", "\\\\foo%20ba%2fr", S_OK, 0x2},
+    {"file:////c:/foo%20ba%2fr", "c:\\foo%20ba%2fr", S_OK, 0},
+    {"file:////foo%20ba%2fr", "\\\\foo%20ba%2fr", S_OK, 0},
 
     /* 5 and more leading (back)slashes */
-    {"file://///c:/foo/bar", "\\\\c:\\foo\\bar", S_OK, 0x2},
-    {"file://///c:/foo%20ba%2fr", "\\\\c:\\foo ba/r", S_OK, 0x2},
-    {"file://///foo%20ba%2fr", "\\\\foo ba/r", S_OK, 0x2},
-    {"file://////c:/foo/bar", "\\\\c:\\foo\\bar", S_OK, 0x2},
+    {"file://///c:/foo/bar", "\\\\c:\\foo\\bar", S_OK, 0},
+    {"file://///c:/foo%20ba%2fr", "\\\\c:\\foo ba/r", S_OK, 0},
+    {"file://///foo%20ba%2fr", "\\\\foo ba/r", S_OK, 0},
+    {"file://////c:/foo/bar", "\\\\c:\\foo\\bar", S_OK, 0},
 
     /* Leading (back)slashes cannot be escaped */
-    {"file:%2f%2flocalhost%2fc:/foo/bar", "//localhost/c:\\foo\\bar", S_OK, 0x2},
+    {"file:%2f%2flocalhost%2fc:/foo/bar", "//localhost/c:\\foo\\bar", S_OK, 0},
     {"file:%5C%5Clocalhost%5Cc:/foo/bar", "\\\\localhost\\c:\\foo\\bar", S_OK, 0},
 
     /* Hostname handling */
-    {"file://l%6fcalhost/c:/foo/bar", "\\\\localhostc:\\foo\\bar", S_OK, 0x4},
-    {"file://localhost:80/c:/foo/bar", "\\\\localhost:80c:\\foo\\bar", S_OK, 0x4},
-    {"file://host/c:/foo/bar", "\\\\hostc:\\foo\\bar", S_OK, 0x4},
+    {"file://l%6fcalhost/c:/foo/bar", "\\\\localhostc:\\foo\\bar", S_OK, 0},
+    {"file://localhost:80/c:/foo/bar", "\\\\localhost:80c:\\foo\\bar", S_OK, 0},
+    {"file://host/c:/foo/bar", "\\\\hostc:\\foo\\bar", S_OK, 0},
     {"file://host//c:/foo/bar", "\\\\host\\\\c:\\foo\\bar", S_OK, 0},
     {"file://host/\\c:/foo/bar", "\\\\host\\\\c:\\foo\\bar", S_OK, 0},
-    {"file://host/c:foo/bar", "\\\\hostc:foo\\bar", S_OK, 0x2},
+    {"file://host/c:foo/bar", "\\\\hostc:foo\\bar", S_OK, 0},
     {"file://host/foo/bar", "\\\\host\\foo\\bar", S_OK, 0},
-    {"file:\\\\host\\c:\\foo\\bar", "\\\\hostc:\\foo\\bar", S_OK, 0x4},
+    {"file:\\\\host\\c:\\foo\\bar", "\\\\hostc:\\foo\\bar", S_OK, 0},
     {"file:\\\\host\\ca\\foo\\bar", "\\\\host\\ca\\foo\\bar", S_OK, 0},
-    {"file:\\\\host\\c|\\foo\\bar", "\\\\hostc|\\foo\\bar", S_OK, 0x4},
+    {"file:\\\\host\\c|\\foo\\bar", "\\\\hostc|\\foo\\bar", S_OK, 0},
     {"file:\\%5Chost\\c:\\foo\\bar", "\\\\host\\c:\\foo\\bar", S_OK, 0},
     {"file:\\\\host\\cx:\\foo\\bar", "\\\\host\\cx:\\foo\\bar", S_OK, 0},
     {"file:///host/c:/foo/bar", "\\host\\c:\\foo\\bar", S_OK, 0},
@@ -305,7 +305,7 @@ static void test_PathCreateFromUrl(void)
                        ret_path, TEST_PATHFROMURL[i].path, TEST_PATHFROMURL[i].url);
                     ok(len == lstrlenW(ret_pathW), "ret len %d from url L\"%s\"\n", len, TEST_PATHFROMURL[i].url);
                 } else todo_wine
-                /* Wrong string, don't bother checking the length */
+                    /* Wrong string, don't bother checking the length */
                     ok(!lstrcmpiW(ret_pathW, pathW), "got %s expected %s from url L\"%s\"\n",
                        ret_path, TEST_PATHFROMURL[i].path, TEST_PATHFROMURL[i].url);
             }