@joebigelow / wix / commits / 9f360945

Try to log Burn command line even if it was invalid.

Sean Hall committed Jun 7, 2021 at 12:13 UTC 9f360945ce3703677701b12267a42334bbe7dca1
6 files changed +111 -36
src/burn/engine/core.cpp
+70 -24
@@ -32,7 +32,8 @@ static HRESULT ParseCommandLine(
32 __out_z LPWSTR* psczActiveParent,
33 __out_z LPWSTR* psczIgnoreDependencies,
34 __out_z LPWSTR* psczAncestors,
35 - __out_z LPWSTR* psczSanitizedCommandLine
35 + __out_z LPWSTR* psczSanitizedCommandLine,
36 + __inout BOOL* pfInvalidCommandLine
37 );
38 static HRESULT ParsePipeConnection(
39 __in_ecount(3) LPWSTR* rgArgs,
@@ -103,11 +104,19 @@ extern "C" HRESULT CoreInitialize(
104 ExitOnFailure(hr, "Failed to initialize containers.");
105
106 // Parse command line.
106 - hr = ParseCommandLine(pEngineState->argc, pEngineState->argv, &pEngineState->command, &pEngineState->companionConnection, &pEngineState->embeddedConnection, &pEngineState->variables, &pEngineState->mode, &pEngineState->automaticUpdates, &pEngineState->fDisableSystemRestore, &sczSourceProcessPath, &sczOriginalSource, &pEngineState->fDisableUnelevate, &pEngineState->log.dwAttributes, &pEngineState->log.sczPath, &pEngineState->registration.sczActiveParent, &pEngineState->sczIgnoreDependencies, &pEngineState->registration.sczAncestors, &sczSanitizedCommandLine);
107 - ExitOnFailure(hr, "Failed to parse command line.");
107 + hr = ParseCommandLine(pEngineState->argc, pEngineState->argv, &pEngineState->command, &pEngineState->companionConnection, &pEngineState->embeddedConnection, &pEngineState->variables, &pEngineState->mode, &pEngineState->automaticUpdates, &pEngineState->fDisableSystemRestore, &sczSourceProcessPath, &sczOriginalSource, &pEngineState->fDisableUnelevate, &pEngineState->log.dwAttributes, &pEngineState->log.sczPath, &pEngineState->registration.sczActiveParent, &pEngineState->sczIgnoreDependencies, &pEngineState->registration.sczAncestors, &sczSanitizedCommandLine, &pEngineState->fInvalidCommandLine);
108 + ExitOnFailure(hr, "Fatal error while parsing command line.");
109
110 LogId(REPORT_STANDARD, MSG_BURN_COMMAND_LINE, sczSanitizedCommandLine ? sczSanitizedCommandLine : L"");
110 -
111 +
112 + // The command line wasn't logged immediately so that hidden variables set on the command line can be obscured in the log.
113 + // This delay creates issues when troubleshooting parsing errors because the original command line is not in the log.
114 + // The code does its best to process the entire command line and keep track if the command line was invalid so that it can log the sanitized command line before erroring out.
115 + if (pEngineState->fInvalidCommandLine)
116 + {
117 + LogExitOnRootFailure(hr = E_INVALIDARG, MSG_FAILED_PARSE_COMMAND_LINE, "Failed to parse command line.");
118 + }
119 +
120 hr = CoreInitializeConstants(pEngineState);
121 ExitOnFailure(hr, "Failed to initialize contants.");
122
@@ -1188,7 +1197,8 @@ static HRESULT ParseCommandLine(
1197 __out_z LPWSTR* psczActiveParent,
1198 __out_z LPWSTR* psczIgnoreDependencies,
1199 __out_z LPWSTR* psczAncestors,
1191 - __out_z LPWSTR* psczSanitizedCommandLine
1200 + __out_z LPWSTR* psczSanitizedCommandLine,
1201 + __inout BOOL* pfInvalidCommandLine
1202 )
1203 {
1204 HRESULT hr = S_OK;
@@ -1197,6 +1207,7 @@ static HRESULT ParseCommandLine(
1207 LPWSTR sczCommandLine = NULL;
1208 LPWSTR sczSanitizedArgument = NULL;
1209 LPWSTR sczVariableName = NULL;
1210 + BOOL fInvalidCommandLine = FALSE;
1211
1212 for (int i = 0; i < argc; ++i)
1213 {
@@ -1219,6 +1230,7 @@ static HRESULT ParseCommandLine(
1230
1231 if (i + 1 >= argc)
1232 {
1233 + fInvalidCommandLine = TRUE;
1234 ExitOnRootFailure(hr = E_INVALIDARG, "Must specify a path for log.");
1235 }
1236
@@ -1331,6 +1343,7 @@ static HRESULT ParseCommandLine(
1343 {
1344 if (i + 1 >= argc)
1345 {
1346 + fInvalidCommandLine = TRUE;
1347 ExitOnRootFailure(hr = E_INVALIDARG, "Must specify a path for original source.");
1348 }
1349
@@ -1342,6 +1355,7 @@ static HRESULT ParseCommandLine(
1355 {
1356 if (i + 1 >= argc)
1357 {
1358 + fInvalidCommandLine = TRUE;
1359 ExitOnRootFailure(hr = E_INVALIDARG, "Must specify a value for parent.");
1360 }
1361
@@ -1359,6 +1373,7 @@ static HRESULT ParseCommandLine(
1373 {
1374 if (i + 1 >= argc)
1375 {
1376 + fInvalidCommandLine = TRUE;
1377 ExitOnRootFailure(hr = E_INVALIDARG, "Must specify a path for append log.");
1378 }
1379
@@ -1373,12 +1388,14 @@ static HRESULT ParseCommandLine(
1388 {
1389 if (i + 3 >= argc)
1390 {
1391 + fInvalidCommandLine = TRUE;
1392 ExitOnRootFailure(hr = E_INVALIDARG, "Must specify the elevated name, token and parent process id.");
1393 }
1394
1395 if (BURN_MODE_UNTRUSTED != *pMode)
1396 {
1381 - ExitOnRootFailure(hr = E_INVALIDARG, "Multiple mode command-line switches were provided.");
1397 + fInvalidCommandLine = TRUE;
1398 + TraceLog(E_INVALIDARG, "Multiple mode command-line switches were provided.");
1399 }
1400
1401 *pMode = BURN_MODE_ELEVATED;
@@ -1386,7 +1403,12 @@ static HRESULT ParseCommandLine(
1403 ++i;
1404
1405 hr = ParsePipeConnection(argv + i, pCompanionConnection);
1389 - ExitOnFailure(hr, "Failed to parse elevated connection.");
1406 + if (FAILED(hr))
1407 + {
1408 + fInvalidCommandLine = TRUE;
1409 + TraceLog(hr, "Failed to parse elevated connection.");
1410 + hr = S_OK;
1411 + }
1412
1413 i += 2;
1414 }
@@ -1396,23 +1418,28 @@ static HRESULT ParseCommandLine(
1418 LPCWSTR wzParam = &argv[i][1 + lstrlenW(BURN_COMMANDLINE_SWITCH_CLEAN_ROOM)];
1419 if (L'=' != wzParam[0] || L'\0' == wzParam[1])
1420 {
1399 - ExitOnRootFailure(hr = E_INVALIDARG, "Missing required parameter for switch: %ls", BURN_COMMANDLINE_SWITCH_CLEAN_ROOM);
1421 + fInvalidCommandLine = TRUE;
1422 + TraceLog(E_INVALIDARG, "Missing required parameter for switch: %ls", BURN_COMMANDLINE_SWITCH_CLEAN_ROOM);
1423 + }
1424 + else
1425 + {
1426 + *pMode = BURN_MODE_NORMAL;
1427 +
1428 + hr = StrAllocString(psczSourceProcessPath, wzParam + 1, 0);
1429 + ExitOnFailure(hr, "Failed to copy source process path.");
1430 }
1431
1432 if (BURN_MODE_UNTRUSTED != *pMode)
1433 {
1404 - ExitOnRootFailure(hr = E_INVALIDARG, "Multiple mode command-line switches were provided.");
1434 + fInvalidCommandLine = TRUE;
1435 + TraceLog(E_INVALIDARG, "Multiple mode command-line switches were provided.");
1436 }
1406 -
1407 - *pMode = BURN_MODE_NORMAL;
1408 -
1409 - hr = StrAllocString(psczSourceProcessPath, wzParam + 1, 0);
1410 - ExitOnFailure(hr, "Failed to copy source process path.");
1437 }
1438 else if (CSTR_EQUAL == ::CompareStringW(LOCALE_INVARIANT, NORM_IGNORECASE, &argv[i][1], -1, BURN_COMMANDLINE_SWITCH_EMBEDDED, -1))
1439 {
1440 if (i + 3 >= argc)
1441 {
1442 + fInvalidCommandLine = TRUE;
1443 ExitOnRootFailure(hr = E_INVALIDARG, "Must specify the embedded name, token and parent process id.");
1444 }
1445
@@ -1428,13 +1455,19 @@ static HRESULT ParseCommandLine(
1455 *pMode = BURN_MODE_EMBEDDED;
1456 break;
1457 default:
1458 + fInvalidCommandLine = TRUE;
1459 ExitOnRootFailure(hr = E_INVALIDARG, "Multiple mode command-line switches were provided.");
1460 }
1461
1462 ++i;
1463
1464 hr = ParsePipeConnection(argv + i, pEmbeddedConnection);
1437 - ExitOnFailure(hr, "Failed to parse embedded connection.");
1465 + if (FAILED(hr))
1466 + {
1467 + fInvalidCommandLine = TRUE;
1468 + TraceLog(hr, "Failed to parse embedded connection.");
1469 + hr = S_OK;
1470 + }
1471
1472 i += 2;
1473 }
@@ -1480,7 +1513,8 @@ static HRESULT ParseCommandLine(
1513 {
1514 if (BURN_MODE_UNTRUSTED != *pMode)
1515 {
1483 - ExitOnRootFailure(hr = E_INVALIDARG, "Multiple mode command-line switches were provided.");
1516 + fInvalidCommandLine = TRUE;
1517 + TraceLog(E_INVALIDARG, "Multiple mode command-line switches were provided.");
1518 }
1519
1520 *pMode = BURN_MODE_RUNONCE;
@@ -1491,11 +1525,14 @@ static HRESULT ParseCommandLine(
1525 LPCWSTR wzParam = &argv[i][1 + lstrlenW(BURN_COMMANDLINE_SWITCH_IGNOREDEPENDENCIES)];
1526 if (L'=' != wzParam[0] || L'\0' == wzParam[1])
1527 {
1494 - ExitOnRootFailure(hr = E_INVALIDARG, "Missing required parameter for switch: %ls", BURN_COMMANDLINE_SWITCH_IGNOREDEPENDENCIES);
1528 + fInvalidCommandLine = TRUE;
1529 + TraceLog(E_INVALIDARG, "Missing required parameter for switch: %ls", BURN_COMMANDLINE_SWITCH_IGNOREDEPENDENCIES);
1530 + }
1531 + else
1532 + {
1533 + hr = StrAllocString(psczIgnoreDependencies, &wzParam[1], 0);
1534 + ExitOnFailure(hr, "Failed to allocate the list of dependencies to ignore.");
1535 }
1496 -
1497 - hr = StrAllocString(psczIgnoreDependencies, &wzParam[1], 0);
1498 - ExitOnFailure(hr, "Failed to allocate the list of dependencies to ignore.");
1536 }
1537 else if (CSTR_EQUAL == ::CompareStringW(LOCALE_INVARIANT, NORM_IGNORECASE, &argv[i][1], lstrlenW(BURN_COMMANDLINE_SWITCH_ANCESTORS), BURN_COMMANDLINE_SWITCH_ANCESTORS, lstrlenW(BURN_COMMANDLINE_SWITCH_ANCESTORS)))
1538 {
@@ -1503,11 +1540,14 @@ static HRESULT ParseCommandLine(
1540 LPCWSTR wzParam = &argv[i][1 + lstrlenW(BURN_COMMANDLINE_SWITCH_ANCESTORS)];
1541 if (L'=' != wzParam[0] || L'\0' == wzParam[1])
1542 {
1506 - ExitOnRootFailure(hr = E_INVALIDARG, "Missing required parameter for switch: %ls", BURN_COMMANDLINE_SWITCH_ANCESTORS);
1543 + fInvalidCommandLine = TRUE;
1544 + TraceLog(hr = E_INVALIDARG, "Missing required parameter for switch: %ls", BURN_COMMANDLINE_SWITCH_ANCESTORS);
1545 + }
1546 + else
1547 + {
1548 + hr = StrAllocString(psczAncestors, &wzParam[1], 0);
1549 + ExitOnFailure(hr, "Failed to allocate the list of ancestors.");
1550 }
1508 -
1509 - hr = StrAllocString(psczAncestors, &wzParam[1], 0);
1510 - ExitOnFailure(hr, "Failed to allocate the list of ancestors.");
1551 }
1552 else if (CSTR_EQUAL == ::CompareStringW(LOCALE_INVARIANT, NORM_IGNORECASE, &argv[i][1], lstrlenW(BURN_COMMANDLINE_SWITCH_FILEHANDLE_ATTACHED), BURN_COMMANDLINE_SWITCH_FILEHANDLE_ATTACHED, lstrlenW(BURN_COMMANDLINE_SWITCH_FILEHANDLE_ATTACHED)))
1553 {
@@ -1591,6 +1631,12 @@ static HRESULT ParseCommandLine(
1631 }
1632
1633 LExit:
1634 + if (fInvalidCommandLine)
1635 + {
1636 + hr = S_OK;
1637 + *pfInvalidCommandLine = TRUE;
1638 + }
1639 +
1640 ReleaseStr(sczVariableName);
1641 ReleaseStr(sczSanitizedArgument);
1642 ReleaseStr(sczCommandLine);
src/burn/engine/core.h
+1
@@ -133,6 +133,7 @@ typedef struct _BURN_ENGINE_STATE
133
134 int argc;
135 LPWSTR* argv;
136 + BOOL fInvalidCommandLine;
137 } BURN_ENGINE_STATE;
138
139 typedef struct _BURN_APPLY_CONTEXT
src/burn/engine/engine.cpp
+30 -12
@@ -341,26 +341,44 @@ static HRESULT InitializeEngineState(
341 wzParam = &pEngineState->argv[i][2 + lstrlenW(BURN_COMMANDLINE_SWITCH_FILEHANDLE_ATTACHED)];
342 if (L'=' != wzParam[-1] || L'\0' == wzParam[0])
343 {
344 - ExitOnRootFailure(hr = E_INVALIDARG, "Missing required parameter for switch: %ls", BURN_COMMANDLINE_SWITCH_FILEHANDLE_ATTACHED);
344 + pEngineState->fInvalidCommandLine = TRUE;
345 + TraceLog(E_INVALIDARG, "Missing required parameter for switch: %ls", BURN_COMMANDLINE_SWITCH_FILEHANDLE_ATTACHED);
346 + }
347 + else
348 + {
349 + hr = StrStringToUInt64(wzParam, 0, &qw);
350 + if (FAILED(hr))
351 + {
352 + TraceLog(hr, "Failed to parse file handle: '%ls'", wzParam);
353 + hr = S_OK;
354 + }
355 + else
356 + {
357 + hSourceEngineFile = (HANDLE)qw;
358 + }
359 }
346 -
347 - hr = StrStringToUInt64(wzParam, 0, &qw);
348 - ExitOnFailure(hr, "Failed to parse file handle: '%ls'", (wzParam));
349 -
350 - hSourceEngineFile = (HANDLE)qw;
360 }
361 if (CSTR_EQUAL == ::CompareStringW(LOCALE_INVARIANT, NORM_IGNORECASE, &pEngineState->argv[i][1], lstrlenW(BURN_COMMANDLINE_SWITCH_FILEHANDLE_SELF), BURN_COMMANDLINE_SWITCH_FILEHANDLE_SELF, lstrlenW(BURN_COMMANDLINE_SWITCH_FILEHANDLE_SELF)))
362 {
363 wzParam = &pEngineState->argv[i][2 + lstrlenW(BURN_COMMANDLINE_SWITCH_FILEHANDLE_SELF)];
364 if (L'=' != wzParam[-1] || L'\0' == wzParam[0])
365 {
357 - ExitOnRootFailure(hr = E_INVALIDARG, "Missing required parameter for switch: %ls", BURN_COMMANDLINE_SWITCH_FILEHANDLE_SELF);
366 + pEngineState->fInvalidCommandLine = TRUE;
367 + TraceLog(E_INVALIDARG, "Missing required parameter for switch: %ls", BURN_COMMANDLINE_SWITCH_FILEHANDLE_SELF);
368 + }
369 + else
370 + {
371 + hr = StrStringToUInt64(wzParam, 0, &qw);
372 + if (FAILED(hr))
373 + {
374 + TraceLog(hr, "Failed to parse file handle: '%ls'", wzParam);
375 + hr = S_OK;
376 + }
377 + else
378 + {
379 + hSectionFile = (HANDLE)qw;
380 + }
381 }
359 -
360 - hr = StrStringToUInt64(wzParam, 0, &qw);
361 - ExitOnFailure(hr, "Failed to parse file handle: '%ls'", (wzParam));
362 -
363 - hSectionFile = (HANDLE)qw;
382 }
383 }
384 }
src/burn/engine/engine.mc
+7
@@ -128,6 +128,13 @@ Language=English
128 Bootstrapper application opted out of any engine behavior to automatically uninstall the bundle during shutdown.
129 .
130
131 +MessageId=15
132 +Severity=Error
133 +SymbolicName=MSG_FAILED_PARSE_COMMAND_LINE
134 +Language=English
135 +Failed to parse command line.
136 +.
137 +
138 MessageId=51
139 Severity=Error
140 SymbolicName=MSG_FAILED_PARSE_CONDITION
src/burn/engine/inc/burnsources.h
+1
@@ -2,3 +2,4 @@
2 // Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information.
3
4 #define DUTIL_SOURCE_DEFAULT DUTIL_SOURCE_EXTERNAL
5 +#define BURN_SOURCE_DEFAULT DUTIL_SOURCE_DEFAULT
src/burn/engine/platform.h
+2
@@ -1,6 +1,8 @@
1 #pragma once
2 // Copyright (c) .NET Foundation and contributors. All rights reserved. Licensed under the Microsoft Reciprocal License. See LICENSE.TXT file in the project root for full license information.
3
4 +#define TraceLog(x, s, ...) ExitTraceSource(BURN_SOURCE_DEFAULT, x, s, __VA_ARGS__)
5 +
6
7 #if defined(__cplusplus)
8 extern "C" {