Allow UrlCombine to calculate size of required buffer.

dlls/shlwapi/tests/path.c

@@ -42,6 +42,7 @@ typedef struct _TEST_URL_CANONICALIZE {
 } TEST_URL_CANONICALIZE;
 
 const TEST_URL_CANONICALIZE TEST_CANONICALIZE[] = {
+    /*FIXME {"http://www.winehq.org/tests/../tests/../..", 0, S_OK, "http://www.winehq.org/"},*/
     {"http://www.winehq.org/tests/../tests", 0, S_OK, "http://www.winehq.org/tests"},
     {"http://www.winehq.org/tests\n", URL_WININET_COMPATIBILITY|URL_ESCAPE_SPACES_ONLY|URL_ESCAPE_UNSAFE, S_OK, "http://www.winehq.org/tests"},
     {"http://www.winehq.org/tests\r", URL_WININET_COMPATIBILITY|URL_ESCAPE_SPACES_ONLY|URL_ESCAPE_UNSAFE, S_OK, "http://www.winehq.org/tests"},
@@ -77,6 +78,28 @@ const TEST_URL_ESCAPE TEST_ESCAPE[] = {
     {"/direct/swhelp/series6/6.2i_latestservicepack.dat\r", URL_ESCAPE_SPACES_ONLY, 0, S_OK, "/direct/swhelp/series6/6.2i_latestservicepack.dat\r"},
 };
 
+typedef struct _TEST_URL_COMBINE {
+    char *url1;
+    char *url2;
+    DWORD flags;
+    HRESULT expectret;
+    char *expecturl;
+} TEST_URL_COMBINE;
+
+const TEST_URL_COMBINE TEST_COMBINE[] = {
+    {"http://www.winehq.org/tests", "tests1", 0, S_OK, "http://www.winehq.org/tests1"},
+    /*FIXME {"http://www.winehq.org/tests", "../tests2", 0, S_OK, "http://www.winehq.org/tests2"},*/
+    {"http://www.winehq.org/tests/", "../tests3", 0, S_OK, "http://www.winehq.org/tests3"},
+    {"http://www.winehq.org/tests/../tests", "tests4", 0, S_OK, "http://www.winehq.org/tests4"},
+    {"http://www.winehq.org/tests/../tests/", "tests5", 0, S_OK, "http://www.winehq.org/tests/tests5"},
+    {"http://www.winehq.org/tests/../tests/", "/tests6/..", 0, S_OK, "http://www.winehq.org/"},
+    {"http://www.winehq.org/tests/../tests/..", "tests7/..", 0, S_OK, "http://www.winehq.org/"},
+    {"http://www.winehq.org/tests/?query=x&return=y", "tests8", 0, S_OK, "http://www.winehq.org/tests/tests8"},
+    {"http://www.winehq.org/tests/#example", "tests9", 0, S_OK, "http://www.winehq.org/tests/tests9"},
+    {"http://www.winehq.org/tests/../tests/", "/tests10/..", URL_DONT_SIMPLIFY, S_OK, "http://www.winehq.org/tests10/.."},
+    {"http://www.winehq.org/tests/../", "tests11", URL_DONT_SIMPLIFY, S_OK, "http://www.winehq.org/tests/../tests11"},
+};
+
 static LPWSTR GetWideString(const char* szString)
 {
   LPWSTR wszString = (LPWSTR) HeapAlloc(GetProcessHeap(), 0,
@@ -204,10 +227,77 @@ static void test_UrlCanonicalize(void)
     }
 }
 
+static void test_url_combine(const char *szUrl1, const char *szUrl2, DWORD dwFlags, HRESULT dwExpectReturn, const char *szExpectUrl)
+{
+    HRESULT hr;
+    CHAR szReturnUrl[INTERNET_MAX_URL_LENGTH];
+    WCHAR wszReturnUrl[INTERNET_MAX_URL_LENGTH];
+    LPWSTR wszUrl1 = GetWideString(szUrl1);
+    LPWSTR wszUrl2 = GetWideString(szUrl2);
+    LPWSTR wszExpectUrl = GetWideString(szExpectUrl);
+    LPWSTR wszConvertedUrl;
+
+    DWORD dwSize;
+    DWORD dwExpectLen = lstrlen(szExpectUrl);
+
+    hr = UrlCombineA(szUrl1, szUrl2, NULL, NULL, dwFlags);
+    ok(hr == E_INVALIDARG, "UrlCombineA returned 0x%08lx, expected 0x%08lx\n", hr, E_INVALIDARG);
+    
+    dwSize = 0;
+    hr = UrlCombineA(szUrl1, szUrl2, NULL, &dwSize, dwFlags);
+    ok(hr == E_POINTER, "Checking length of string, return was 0x%08lx, expected 0x%08lx\n", hr, E_POINTER);
+    ok(dwSize == dwExpectLen+1, "Got length %ld, expected %ld\n", dwSize, dwExpectLen+1);
+
+    dwSize--;
+    hr = UrlCombineA(szUrl1, szUrl2, szReturnUrl, &dwSize, dwFlags);
+    ok(hr == E_POINTER, "UrlCombineA returned 0x%08lx, expected 0x%08lx\n", hr, E_POINTER);
+    ok(dwSize == dwExpectLen+1, "Got length %ld, expected %ld\n", dwSize, dwExpectLen+1);
+    
+    hr = UrlCombineA(szUrl1, szUrl2, szReturnUrl, &dwSize, dwFlags);
+    ok(hr == dwExpectReturn, "UrlCombineA returned 0x%08lx, expected 0x%08lx\n", hr, dwExpectReturn);
+    ok(dwSize == dwExpectLen, "Got length %ld, expected %ld\n", dwSize, dwExpectLen);
+    if(SUCCEEDED(hr)) {
+        ok(strcmp(szReturnUrl,szExpectUrl)==0, "Expected %s, but got %s\n", szExpectUrl, szReturnUrl);
+    }
+
+    dwSize = 0;
+    hr = UrlCombineW(wszUrl1, wszUrl2, NULL, &dwSize, dwFlags);
+    ok(hr == E_POINTER, "Checking length of string, return was 0x%08lx, expected 0x%08lx\n", hr, E_POINTER);
+    ok(dwSize == dwExpectLen+1, "Got length %ld, expected %ld\n", dwSize, dwExpectLen+1);
+
+    dwSize--;
+    hr = UrlCombineW(wszUrl1, wszUrl2, wszReturnUrl, &dwSize, dwFlags);
+    ok(hr == E_POINTER, "UrlCombineA returned 0x%08lx, expected 0x%08lx\n", hr, E_POINTER);
+    ok(dwSize == dwExpectLen+1, "Got length %ld, expected %ld\n", dwSize, dwExpectLen+1);
+    
+    hr = UrlCombineW(wszUrl1, wszUrl2, wszReturnUrl, &dwSize, dwFlags);
+    ok(hr == dwExpectReturn, "UrlCombineW returned 0x%08lx, expected 0x%08lx\n", hr, dwExpectReturn);
+    ok(dwSize == dwExpectLen, "Got length %ld, expected %ld\n", dwSize, dwExpectLen);
+    if(SUCCEEDED(hr)) {
+        wszConvertedUrl = GetWideString(szReturnUrl);
+        ok(strcmpW(wszReturnUrl, wszConvertedUrl)==0, "Strings didn't match between ascii and unicode UrlCombine!\n");
+        FreeWideString(wszConvertedUrl);
+    }
+
+    FreeWideString(wszUrl1);
+    FreeWideString(wszUrl2);
+    FreeWideString(wszExpectUrl);
+}
+
+static void test_UrlCombine(void)
+{
+    int i;
+    for(i=0; i<sizeof(TEST_COMBINE)/sizeof(TEST_COMBINE[0]); i++) {
+        test_url_combine(TEST_COMBINE[i].url1, TEST_COMBINE[i].url2, TEST_COMBINE[i].flags,
+                         TEST_COMBINE[i].expectret, TEST_COMBINE[i].expecturl);
+    }
+}
+
 START_TEST(path)
 {
   test_UrlHash();
   test_UrlGetPart();
   test_UrlCanonicalize();
   test_UrlEscape();
+  test_UrlCombine();
 }

dlls/shlwapi/url.c

@@ -643,7 +643,7 @@ HRESULT WINAPI UrlCombineA(LPCSTR pszBase, LPCSTR pszRelative,
 	  debugstr_a(pszBase),debugstr_a(pszRelative),
 	  pcchCombined?*pcchCombined:0,dwFlags);
 
-    if(!pszBase || !pszRelative || !pszCombined || !pcchCombined)
+    if(!pszBase || !pszRelative || !pcchCombined)
 	return E_INVALIDARG;
 
     base = (LPWSTR) HeapAlloc(GetProcessHeap(), 0,
@@ -653,10 +653,11 @@ HRESULT WINAPI UrlCombineA(LPCSTR pszBase, LPCSTR pszRelative,
 
     MultiByteToWideChar(0, 0, pszBase, -1, base, INTERNET_MAX_URL_LENGTH);
     MultiByteToWideChar(0, 0, pszRelative, -1, relative, INTERNET_MAX_URL_LENGTH);
-    len = INTERNET_MAX_URL_LENGTH;
+    len = *pcchCombined;
 
-    ret = UrlCombineW(base, relative, combined, &len, dwFlags);
+    ret = UrlCombineW(base, relative, pszCombined?combined:NULL, &len, dwFlags);
     if (ret != S_OK) {
+	*pcchCombined = len;
 	HeapFree(GetProcessHeap(), 0, base);
 	return ret;
     }
@@ -667,7 +668,7 @@ HRESULT WINAPI UrlCombineA(LPCSTR pszBase, LPCSTR pszRelative,
 	HeapFree(GetProcessHeap(), 0, base);
 	return E_POINTER;
     }
-    WideCharToMultiByte(0, 0, combined, len+1, pszCombined, *pcchCombined,
+    WideCharToMultiByte(0, 0, combined, len+1, pszCombined, (*pcchCombined)+1,
 			0, 0);
     *pcchCombined = len2;
     HeapFree(GetProcessHeap(), 0, base);
@@ -695,7 +696,7 @@ HRESULT WINAPI UrlCombineW(LPCWSTR pszBase, LPCWSTR pszRelative,
 	  debugstr_w(pszBase),debugstr_w(pszRelative),
 	  pcchCombined?*pcchCombined:0,dwFlags);
 
-    if(!pszBase || !pszRelative || !pszCombined || !pcchCombined)
+    if(!pszBase || !pszRelative || !pcchCombined)
 	return E_INVALIDARG;
 
     base.size = 24;
@@ -829,12 +830,6 @@ HRESULT WINAPI UrlCombineW(LPCWSTR pszBase, LPCWSTR pszRelative,
 	      * Return pszRelative appended to what ever is in pszCombined,
 	      * (which may the string "file:///"
 	      */
-	len = strlenW(mrelative) + strlenW(preliminary);
-	if (len+1 > *pcchCombined) {
-	    *pcchCombined = len;
-	    ret = E_POINTER;
-	    break;
-	}
 	strcatW(preliminary, mrelative);
 	break;
 
@@ -842,12 +837,6 @@ HRESULT WINAPI UrlCombineW(LPCWSTR pszBase, LPCWSTR pszRelative,
 	      * Same as case 1, but if URL_PLUGGABLE_PROTOCOL was specified
 	      * and pszRelative starts with "//", then append a "/"
 	      */
-	len = strlenW(mrelative) + 1;
-	if (len+1 > *pcchCombined) {
-	    *pcchCombined = len;
-	    ret = E_POINTER;
-	    break;
-	}
 	strcpyW(preliminary, mrelative);
 	if (!(dwFlags & URL_PLUGGABLE_PROTOCOL) &&
 	    URL_JustLocation(relative.ap2))
@@ -858,12 +847,6 @@ HRESULT WINAPI UrlCombineW(LPCWSTR pszBase, LPCWSTR pszRelative,
 	      * Return the pszBase scheme with pszRelative. Basically
 	      * keeps the scheme and replaces the domain and following.
 	      */
-	len = base.sizep1 + 1 + relative.sizep2 + 1;
-	if (len+1 > *pcchCombined) {
-	    *pcchCombined = len;
-	    ret = E_POINTER;
-	    break;
-	}
 	strncpyW(preliminary, base.ap1, base.sizep1 + 1);
 	work = preliminary + base.sizep1 + 1;
 	strcpyW(work, relative.ap2);
@@ -877,12 +860,6 @@ HRESULT WINAPI UrlCombineW(LPCWSTR pszBase, LPCWSTR pszRelative,
 	      * after the location is pszRelative. (Replace document
 	      * from root on.)
 	      */
-	len = base.sizep1 + 1 + sizeloc + relative.sizep2 + 1;
-	if (len+1 > *pcchCombined) {
-	    *pcchCombined = len;
-	    ret = E_POINTER;
-	    break;
-	}
 	strncpyW(preliminary, base.ap1, base.sizep1+1+sizeloc);
 	work = preliminary + base.sizep1 + 1 + sizeloc;
 	if (dwFlags & URL_PLUGGABLE_PROTOCOL)
@@ -894,12 +871,6 @@ HRESULT WINAPI UrlCombineW(LPCWSTR pszBase, LPCWSTR pszRelative,
 	      * Return the pszBase without its document (if any) and
 	      * append pszRelative after its scheme.
 	      */
-	len = base.sizep1 + 1 + base.sizep2 + relative.sizep2;
-	if (len+1 > *pcchCombined) {
-	    *pcchCombined = len;
-	    ret = E_POINTER;
-	    break;
-	}
 	strncpyW(preliminary, base.ap1, base.sizep1+1+base.sizep2);
 	work = preliminary + base.sizep1+1+base.sizep2 - 1;
 	if (*work++ != L'/')
@@ -913,21 +884,10 @@ HRESULT WINAPI UrlCombineW(LPCWSTR pszBase, LPCWSTR pszRelative,
     }
 
     if (ret == S_OK) {
-	/*
-	 * Now that the combining is done, process the escape options if
-	 * necessary, otherwise just copy the string.
-	 */
-	myflags = dwFlags & (URL_ESCAPE_PERCENT |
-			     URL_ESCAPE_SPACES_ONLY |
-                             URL_DONT_ESCAPE_EXTRA_INFO |
-			     URL_ESCAPE_SEGMENT_ONLY);
-	if (myflags)
-	    ret = UrlEscapeW(preliminary, pszCombined,
-			     pcchCombined, myflags);
-	else {
-	    len = (strlenW(preliminary) + 1) * sizeof(WCHAR);
-	    memcpy(pszCombined, preliminary, len);
-	    *pcchCombined = strlenW(preliminary);
+	/* Reuse mrelative as temp storage as its already allocated and not needed anymore */
+	ret = UrlCanonicalizeW(preliminary, mrelative, pcchCombined, dwFlags);
+	if(SUCCEEDED(ret) && pszCombined) {
+	    lstrcpyW(pszCombined, mrelative);
 	}
 	TRACE("return-%ld len=%ld, %s\n",
 	      process_case, *pcchCombined, debugstr_w(pszCombined));