@joebigelow / wix-1 / commits / 0c2b4cf3

Integrate fixes that make dutil a little more robust to failure

Rob Mensching committed Apr 10, 2021 at 16:05 UTC 0c2b4cf3a439eda3e19d20fadfc65ddc7d0394c0
6 files changed +265 -14
src/dutil/cabutil.cpp
+43 -9
@@ -102,6 +102,29 @@ LExit:
102 }
103
104
105 +static HANDLE OpenFileWithRetry(
106 + __in LPCWSTR wzPath,
107 + __in DWORD dwDesiredAccess,
108 + __in DWORD dwCreationDisposition
109 +)
110 +{
111 + HANDLE hFile = INVALID_HANDLE_VALUE;
112 +
113 + for (DWORD i = 0; i < 30; ++i)
114 + {
115 + hFile = ::CreateFileW(wzPath, dwDesiredAccess, FILE_SHARE_READ, NULL, dwCreationDisposition, FILE_ATTRIBUTE_NORMAL, NULL);
116 + if (INVALID_HANDLE_VALUE != hFile)
117 + {
118 + break;
119 + }
120 +
121 + ::Sleep(100);
122 + }
123 +
124 + return hFile;
125 +}
126 +
127 +
128 /********************************************************************
129 CabInitialize - initializes internal static variables
130
@@ -340,6 +363,7 @@ static __callback void DIAMONDAPI CabExtractFree(__in LPVOID pvData)
363 static __callback INT_PTR FAR DIAMONDAPI CabExtractOpen(__in_z PSTR pszFile, __in int oflag, __in int pmode)
364 {
365 HRESULT hr = S_OK;
366 + HANDLE hFile = INVALID_HANDLE_VALUE;
367 INT_PTR pFile = -1;
368 LPWSTR sczCabFile = NULL;
369
@@ -353,19 +377,24 @@ static __callback INT_PTR FAR DIAMONDAPI CabExtractOpen(__in_z PSTR pszFile, __i
377 hr = StrAllocStringAnsi(&sczCabFile, pszFile, 0, CP_UTF8);
378 CabExitOnFailure(hr, "Failed to convert UTF8 cab file name to wide character string");
379
356 - pFile = reinterpret_cast<INT_PTR>(::CreateFileW(sczCabFile, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL));
357 - if (INVALID_HANDLE_VALUE == reinterpret_cast<HANDLE>(pFile))
380 + hFile = OpenFileWithRetry(sczCabFile, GENERIC_READ, OPEN_EXISTING);
381 + if (INVALID_HANDLE_VALUE == hFile)
382 {
383 CabExitWithLastError(hr, "failed to open file: %ls", sczCabFile);
384 }
385
386 + pFile = reinterpret_cast<INT_PTR>(hFile);
387 +
388 if (vdw64EmbeddedOffset)
389 {
390 hr = CabExtractSeek(pFile, 0, 0);
391 CabExitOnFailure(hr, "Failed to seek to embedded offset %I64d", vdw64EmbeddedOffset);
392 }
393
394 + hFile = INVALID_HANDLE_VALUE;
395 +
396 LExit:
397 + ReleaseFileHandle(hFile);
398 ReleaseStr(sczCabFile);
399
400 return FAILED(hr) ? -1 : pFile;
@@ -460,6 +489,7 @@ static __callback INT_PTR DIAMONDAPI CabExtractCallback(__in FDINOTIFICATIONTYPE
489 Assert(pFDINotify->pv);
490
491 HRESULT hr = S_OK;
492 + HANDLE hFile = INVALID_HANDLE_VALUE;
493 INT_PTR ipResult = 0; // result to return on success
494
495 CAB_CALLBACK_STRUCT* pccs = static_cast<CAB_CALLBACK_STRUCT*>(pFDINotify->pv);
@@ -503,7 +533,6 @@ static __callback INT_PTR DIAMONDAPI CabExtractCallback(__in FDINOTIFICATIONTYPE
533 CabExitWithLastError(hr, "failed to get time for resource: %ls", wz);
534 }
535 ::LocalFileTimeToFileTime(&ftLocal, &ft);
506 -
536
537 WCHAR wzPath[MAX_PATH];
538 hr = ::StringCchCopyW(wzPath, countof(wzPath), pccs->pwzExtractDir);
@@ -511,21 +540,24 @@ static __callback INT_PTR DIAMONDAPI CabExtractCallback(__in FDINOTIFICATIONTYPE
540 hr = ::StringCchCatW(wzPath, countof(wzPath), wz);
541 CabExitOnFailure(hr, "failed to concat onto path: %ls file: %ls", wzPath, wz);
542
514 - ipResult = reinterpret_cast<INT_PTR>(::CreateFileW(wzPath, GENERIC_WRITE, FILE_SHARE_READ, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL));
515 - if (INVALID_HANDLE_VALUE == reinterpret_cast<HANDLE>(ipResult))
543 + hFile = OpenFileWithRetry(wzPath, GENERIC_WRITE, CREATE_ALWAYS);
544 + if (INVALID_HANDLE_VALUE == hFile)
545 {
546 CabExitWithLastError(hr, "failed to create file: %ls", wzPath);
547 }
548
520 - ::SetFileTime(reinterpret_cast<HANDLE>(ipResult), &ft, &ft, &ft); // try to set the file time (who cares if it fails)
549 + ::SetFileTime(hFile, &ft, &ft, &ft); // try to set the file time (who cares if it fails)
550
522 - if (::SetFilePointer(reinterpret_cast<HANDLE>(ipResult), pFDINotify->cb, NULL, FILE_BEGIN)) // try to set the end of the file (don't worry if this fails)
551 + if (::SetFilePointer(hFile, pFDINotify->cb, NULL, FILE_BEGIN)) // try to set the end of the file (don't worry if this fails)
552 {
524 - if (::SetEndOfFile(reinterpret_cast<HANDLE>(ipResult)))
553 + if (::SetEndOfFile(hFile))
554 {
526 - ::SetFilePointer(reinterpret_cast<HANDLE>(ipResult), 0, NULL, FILE_BEGIN); // reset the file pointer
555 + ::SetFilePointer(hFile, 0, NULL, FILE_BEGIN); // reset the file pointer
556 }
557 }
558 +
559 + ipResult = reinterpret_cast<INT_PTR>(hFile);
560 + hFile = INVALID_HANDLE_VALUE;
561 }
562 else // resource wasn't requested, skip it
563 {
@@ -579,5 +611,7 @@ static __callback INT_PTR DIAMONDAPI CabExtractCallback(__in FDINOTIFICATIONTYPE
611 };
612
613 LExit:
614 + ReleaseFileHandle(hFile);
615 +
616 return (S_OK == hr) ? ipResult : -1;
617 }
src/dutil/fileutil.cpp
+128
@@ -1105,6 +1105,134 @@ LExit:
1105 }
1106
1107
1108 +/*******************************************************************
1109 + FileCopyUsingHandlesWithProgress
1110 +
1111 +*******************************************************************/
1112 +extern "C" HRESULT DAPI FileCopyUsingHandlesWithProgress(
1113 + __in HANDLE hSource,
1114 + __in HANDLE hTarget,
1115 + __in DWORD64 cbCopy,
1116 + __in_opt LPPROGRESS_ROUTINE lpProgressRoutine,
1117 + __in_opt LPVOID lpData,
1118 + __in_opt LPBOOL pbCancel,
1119 + __out_opt DWORD64* pcbCopied
1120 +)
1121 +{
1122 + HRESULT hr = S_OK;
1123 + BOOL fStop = FALSE;
1124 + BOOL fCanceled = FALSE;
1125 + DWORD64 cbTotalCopied = 0;
1126 + BYTE rgbData[64 * 1024];
1127 + DWORD cbRead = 0;
1128 +
1129 + LARGE_INTEGER liSourceSize = { };
1130 + LARGE_INTEGER liTotalCopied = { };
1131 + LARGE_INTEGER liZero = { };
1132 + DWORD dwResult = 0;
1133 +
1134 + hr = FileSizeByHandle(hSource, &liSourceSize.QuadPart);
1135 + FileExitOnFailure(hr, "Failed to get size of source.");
1136 +
1137 + if (0 < cbCopy && cbCopy < (DWORD64)liSourceSize.QuadPart)
1138 + {
1139 + liSourceSize.QuadPart = cbCopy;
1140 + }
1141 +
1142 + dwResult = lpProgressRoutine(liSourceSize, liTotalCopied, liZero, liZero, 0, CALLBACK_STREAM_SWITCH, hSource, hTarget, lpData);
1143 + switch (dwResult)
1144 + {
1145 + case PROGRESS_CONTINUE:
1146 + break;
1147 +
1148 + case PROGRESS_CANCEL:
1149 + fCanceled = TRUE;
1150 + fStop = TRUE;
1151 + break;
1152 +
1153 + case PROGRESS_STOP:
1154 + fStop = TRUE;
1155 + break;
1156 +
1157 + case PROGRESS_QUIET:
1158 + lpProgressRoutine = NULL;
1159 + break;
1160 + }
1161 +
1162 + // Set size of the target file.
1163 + ::SetFilePointerEx(hTarget, liSourceSize, NULL, FILE_BEGIN);
1164 +
1165 + if (!::SetEndOfFile(hTarget))
1166 + {
1167 + FileExitWithLastError(hr, "Failed to set end of target file.");
1168 + }
1169 +
1170 + if (!::SetFilePointerEx(hTarget, liZero, NULL, FILE_BEGIN))
1171 + {
1172 + FileExitWithLastError(hr, "Failed to reset target file pointer.");
1173 + }
1174 +
1175 + // Copy with progress.
1176 + while (!fStop && (0 == cbCopy || cbTotalCopied < cbCopy))
1177 + {
1178 + cbRead = static_cast<DWORD>((0 == cbCopy) ? countof(rgbData) : min(countof(rgbData), cbCopy - cbTotalCopied));
1179 + if (!::ReadFile(hSource, rgbData, cbRead, &cbRead, NULL))
1180 + {
1181 + FileExitWithLastError(hr, "Failed to read from source.");
1182 + }
1183 +
1184 + if (cbRead)
1185 + {
1186 + hr = FileWriteHandle(hTarget, rgbData, cbRead);
1187 + FileExitOnFailure(hr, "Failed to write to target.");
1188 +
1189 + cbTotalCopied += cbRead;
1190 +
1191 + if (lpProgressRoutine)
1192 + {
1193 + liTotalCopied.QuadPart = cbTotalCopied;
1194 + dwResult = lpProgressRoutine(liSourceSize, liTotalCopied, liZero, liZero, 0, CALLBACK_CHUNK_FINISHED, hSource, hTarget, lpData);
1195 + switch (dwResult)
1196 + {
1197 + case PROGRESS_CONTINUE:
1198 + break;
1199 +
1200 + case PROGRESS_CANCEL:
1201 + fCanceled = TRUE;
1202 + fStop = TRUE;
1203 + break;
1204 +
1205 + case PROGRESS_STOP:
1206 + fStop = TRUE;
1207 + break;
1208 +
1209 + case PROGRESS_QUIET:
1210 + lpProgressRoutine = NULL;
1211 + break;
1212 + }
1213 + }
1214 + }
1215 + else
1216 + {
1217 + fStop = TRUE;
1218 + }
1219 + }
1220 +
1221 +LExit:
1222 + if (pbCancel)
1223 + {
1224 + *pbCancel = fCanceled;
1225 + }
1226 +
1227 + if (pcbCopied)
1228 + {
1229 + *pcbCopied = cbTotalCopied;
1230 + }
1231 +
1232 + return hr;
1233 +}
1234 +
1235 +
1236 /*******************************************************************
1237 FileEnsureCopy
1238
src/dutil/inc/fileutil.h
+9
@@ -148,6 +148,15 @@ HRESULT DAPI FileCopyUsingHandles(
148 __in DWORD64 cbCopy,
149 __out_opt DWORD64* pcbCopied
150 );
151 +HRESULT DAPI FileCopyUsingHandlesWithProgress(
152 + __in HANDLE hSource,
153 + __in HANDLE hTarget,
154 + __in DWORD64 cbCopy,
155 + __in_opt LPPROGRESS_ROUTINE lpProgressRoutine,
156 + __in_opt LPVOID lpData,
157 + __in_opt LPBOOL pbCancel,
158 + __out_opt DWORD64* pcbCopied
159 + );
160 HRESULT DAPI FileEnsureCopy(
161 __in_z LPCWSTR wzSource,
162 __in_z LPCWSTR wzTarget,
src/dutil/inc/regutil.h
+13
@@ -226,6 +226,19 @@ HRESULT DAPI RegQueryKey(
226 __out_opt DWORD* pcSubKeys,
227 __out_opt DWORD* pcValues
228 );
229 +HRESULT DAPI RegKeyReadNumber(
230 + __in HKEY hk,
231 + __in_z LPCWSTR wzSubKey,
232 + __in_z_opt LPCWSTR wzName,
233 + __in BOOL f64Bit,
234 + __out DWORD* pdwValue
235 + );
236 +BOOL DAPI RegValueExists(
237 + __in HKEY hk,
238 + __in_z LPCWSTR wzSubKey,
239 + __in_z_opt LPCWSTR wzName,
240 + __in BOOL f64Bit
241 + );
242
243 #ifdef __cplusplus
244 }
src/dutil/regutil.cpp
+57 -2
@@ -283,7 +283,7 @@ LExit:
283
284
285 /********************************************************************
286 - RegKeyEnum - enumerates a registry key.
286 + RegKeyEnum - enumerates child registry keys.
287
288 *********************************************************************/
289 extern "C" HRESULT DAPI RegKeyEnum(
@@ -340,7 +340,7 @@ LExit:
340
341
342 /********************************************************************
343 - RegValueEnum - enumerates a registry value.
343 + RegValueEnum - enumerates registry values.
344
345 *********************************************************************/
346 HRESULT DAPI RegValueEnum(
@@ -939,6 +939,61 @@ LExit:
939 return hr;
940 }
941
942 +/********************************************************************
943 +RegKeyReadNumber - reads a DWORD registry key value as a number from
944 +a specified subkey.
945 +
946 +*********************************************************************/
947 +extern "C" HRESULT DAPI RegKeyReadNumber(
948 + __in HKEY hk,
949 + __in_z LPCWSTR wzSubKey,
950 + __in_z_opt LPCWSTR wzName,
951 + __in BOOL f64Bit,
952 + __out DWORD* pdwValue
953 + )
954 +{
955 + HRESULT hr = S_OK;
956 + HKEY hkKey = NULL;
957 +
958 + hr = RegOpen(hk, wzSubKey, KEY_READ | f64Bit ? KEY_WOW64_64KEY : 0, &hkKey);
959 + RegExitOnFailure(hr, "Failed to open key: %ls", wzSubKey);
960 +
961 + hr = RegReadNumber(hkKey, wzName, pdwValue);
962 + RegExitOnFailure(hr, "Failed to read value: %ls/@%ls", wzSubKey, wzName);
963 +
964 +LExit:
965 + ReleaseRegKey(hkKey);
966 +
967 + return hr;
968 +}
969 +
970 +/********************************************************************
971 +RegValueExists - determines whether a named value exists in a
972 +specified subkey.
973 +
974 +*********************************************************************/
975 +extern "C" BOOL DAPI RegValueExists(
976 + __in HKEY hk,
977 + __in_z LPCWSTR wzSubKey,
978 + __in_z_opt LPCWSTR wzName,
979 + __in BOOL f64Bit
980 + )
981 +{
982 + HRESULT hr = S_OK;
983 + HKEY hkKey = NULL;
984 + DWORD dwType = 0;
985 +
986 + hr = RegOpen(hk, wzSubKey, KEY_READ | f64Bit ? KEY_WOW64_64KEY : 0, &hkKey);
987 + RegExitOnFailure(hr, "Failed to open key: %ls", wzSubKey);
988 +
989 + hr = RegGetType(hkKey, wzName, &dwType);
990 + RegExitOnFailure(hr, "Failed to read value type: %ls/@%ls", wzSubKey, wzName);
991 +
992 +LExit:
993 + ReleaseRegKey(hkKey);
994 +
995 + return SUCCEEDED(hr);
996 +}
997
998 static HRESULT WriteStringToRegistry(
999 __in HKEY hk,
src/dutil/wiutil.cpp
+15 -3
@@ -4,6 +4,7 @@
4
5
6 // Exit macros
7 +#define WiuExitTrace(x, s, ...) ExitTraceSource(DUTIL_SOURCE_WIUTIL, x, s, __VA_ARGS__)
8 #define WiuExitOnLastError(x, s, ...) ExitOnLastErrorSource(DUTIL_SOURCE_WIUTIL, x, s, __VA_ARGS__)
9 #define WiuExitOnLastErrorDebugTrace(x, s, ...) ExitOnLastErrorDebugTraceSource(DUTIL_SOURCE_WIUTIL, x, s, __VA_ARGS__)
10 #define WiuExitWithLastError(x, s, ...) ExitWithLastErrorSource(DUTIL_SOURCE_WIUTIL, x, s, __VA_ARGS__)
@@ -692,12 +693,23 @@ extern "C" HRESULT DAPI WiuEnumRelatedProductCodes(
693
694 if (fReturnHighestVersionOnly)
695 {
695 - // get the version
696 + // try to get the version but if the product registration is broken
697 + // (for whatever reason), skip this product
698 hr = WiuGetProductInfo(wzCurrentProductCode, L"VersionString", &sczInstalledVersion);
697 - WiuExitOnFailure(hr, "Failed to get version for product code: %ls", wzCurrentProductCode);
699 + if (FAILED(hr))
700 + {
701 + WiuExitTrace(hr, "Could not get product version for product code: %ls, skipping...", wzCurrentProductCode);
702 + continue;
703 + }
704
705 + // try to parse the product version but if it is corrupt (for whatever
706 + // reason), skip it
707 hr = FileVersionFromStringEx(sczInstalledVersion, 0, &qwCurrentVersion);
700 - WiuExitOnFailure(hr, "Failed to convert version: %ls to DWORD64 for product code: %ls", sczInstalledVersion, wzCurrentProductCode);
708 + if (FAILED(hr))
709 + {
710 + WiuExitTrace(hr, "Could not convert version: %ls to DWORD64 for product code: %ls, skipping...", sczInstalledVersion, wzCurrentProductCode);
711 + continue;
712 + }
713
714 // if this is the first product found then it is the highest version (for now)
715 if (0 == *pcRelatedProducts)