From fad0bea304ee1908ad021df22f1483800cd55eca Mon Sep 17 00:00:00 2001 From: Francois Gouget Date: Wed, 26 Sep 2012 04:22:00 +0200 Subject: [PATCH] shlwapi: Fix the PathCreateFromUrlW() implementation. --- dlls/shlwapi/path.c | 139 +++++++++++++++++++++++++++++++++++----------- dlls/shlwapi/tests/path.c | 66 +++++++++++----------- 2 files changed, 140 insertions(+), 65 deletions(-) diff --git a/dlls/shlwapi/path.c b/dlls/shlwapi/path.c index de891967281..dc7bc4b7ad6 100644 --- a/dlls/shlwapi/path.c +++ b/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; } /************************************************************************* diff --git a/dlls/shlwapi/tests/path.c b/dlls/shlwapi/tests/path.c index 0dd4e00d588..64179884e6a 100644 --- a/dlls/shlwapi/tests/path.c +++ b/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); } -- 2.11.4.GIT