@joebigelow / wix-1 / commits / 07fbc356

Allow quoted values in foreach

Fixes 7039

Rob Mensching committed Dec 6, 2022 at 14:11 UTC 07fbc3561fb66dba1502305ba4aff1db905c84e4

2 files changed +287 -226

src/wix/WixToolset.Core/Preprocessor.cs

+237 -226

@@ -242,26 +242,26 @@ namespace WixToolset.Core

242 expression = expression.ToUpperInvariant();

243 switch (operation)

244 {

245 - case PreprocessorOperation.Not:

246 - if (expression.StartsWith("NOT ", StringComparison.Ordinal) || expression.StartsWith("NOT(", StringComparison.Ordinal))

247 - {

248 - return true;

249 - }

250 - break;

251 - case PreprocessorOperation.And:

252 - if (expression.StartsWith("AND ", StringComparison.Ordinal) || expression.StartsWith("AND(", StringComparison.Ordinal))

253 - {

254 - return true;

255 - }

256 - break;

257 - case PreprocessorOperation.Or:

258 - if (expression.StartsWith("OR ", StringComparison.Ordinal) || expression.StartsWith("OR(", StringComparison.Ordinal))

259 - {

260 - return true;

261 - }

262 - break;

263 - default:

264 - break;

245 + case PreprocessorOperation.Not:

246 + if (expression.StartsWith("NOT ", StringComparison.Ordinal) || expression.StartsWith("NOT(", StringComparison.Ordinal))

247 + {

248 + return true;

249 + }

250 + break;

251 + case PreprocessorOperation.And:

252 + if (expression.StartsWith("AND ", StringComparison.Ordinal) || expression.StartsWith("AND(", StringComparison.Ordinal))

253 + {

254 + return true;

255 + }

256 + break;

257 + case PreprocessorOperation.Or:

258 + if (expression.StartsWith("OR ", StringComparison.Ordinal) || expression.StartsWith("OR(", StringComparison.Ordinal))

259 + {

260 + return true;

261 + }

262 + break;

263 + default:

264 + break;

265 }

266 return false;

267 }

@@ -298,97 +298,97 @@ namespace WixToolset.Core

298

299 switch (reader.LocalName)

300 {

301 - case "if":

302 - ifStack.Push(ifContext);

303 - if (ifContext.IsTrue)

304 - {

305 - ifContext = new IfContext(ifContext.IsTrue & ifContext.Active, this.EvaluateExpression(state, reader.Value), IfState.If);

306 - }

307 - else // Use a default IfContext object so we don't try to evaluate the expression if the context isn't true

308 - {

309 - ifContext = new IfContext();

310 - }

311 - ignore = true;

312 - break;

301 + case "if":

302 + ifStack.Push(ifContext);

303 + if (ifContext.IsTrue)

304 + {

305 + ifContext = new IfContext(ifContext.IsTrue & ifContext.Active, this.EvaluateExpression(state, reader.Value), IfState.If);

306 + }

307 + else // Use a default IfContext object so we don't try to evaluate the expression if the context isn't true

308 + {

309 + ifContext = new IfContext();

310 + }

311 + ignore = true;

312 + break;

313

314 - case "ifdef":

315 - ifStack.Push(ifContext);

316 - name = reader.Value.Trim();

317 - if (ifContext.IsTrue)

318 - {

319 - ifContext = new IfContext(ifContext.IsTrue & ifContext.Active, (null != state.Helper.GetVariableValue(state.Context, name, true)), IfState.If);

320 - }

321 - else // Use a default IfContext object so we don't try to evaluate the expression if the context isn't true

322 - {

323 - ifContext = new IfContext();

324 - }

325 - ignore = true;

326 - this.IfDef?.Invoke(this, new IfDefEventArgs(sourceLineNumbers, true, ifContext.IsTrue, name));

327 - break;

314 + case "ifdef":

315 + ifStack.Push(ifContext);

316 + name = reader.Value.Trim();

317 + if (ifContext.IsTrue)

318 + {

319 + ifContext = new IfContext(ifContext.IsTrue & ifContext.Active, (null != state.Helper.GetVariableValue(state.Context, name, true)), IfState.If);

320 + }

321 + else // Use a default IfContext object so we don't try to evaluate the expression if the context isn't true

322 + {

323 + ifContext = new IfContext();

324 + }

325 + ignore = true;

326 + this.IfDef?.Invoke(this, new IfDefEventArgs(sourceLineNumbers, true, ifContext.IsTrue, name));

327 + break;

328

329 - case "ifndef":

330 - ifStack.Push(ifContext);

331 - name = reader.Value.Trim();

332 - if (ifContext.IsTrue)

333 - {

334 - ifContext = new IfContext(ifContext.IsTrue & ifContext.Active, (null == state.Helper.GetVariableValue(state.Context, name, true)), IfState.If);

335 - }

336 - else // Use a default IfContext object so we don't try to evaluate the expression if the context isn't true

337 - {

338 - ifContext = new IfContext();

339 - }

340 - ignore = true;

341 - this.IfDef?.Invoke(this, new IfDefEventArgs(sourceLineNumbers, false, !ifContext.IsTrue, name));

342 - break;

329 + case "ifndef":

330 + ifStack.Push(ifContext);

331 + name = reader.Value.Trim();

332 + if (ifContext.IsTrue)

333 + {

334 + ifContext = new IfContext(ifContext.IsTrue & ifContext.Active, (null == state.Helper.GetVariableValue(state.Context, name, true)), IfState.If);

335 + }

336 + else // Use a default IfContext object so we don't try to evaluate the expression if the context isn't true

337 + {

338 + ifContext = new IfContext();

339 + }

340 + ignore = true;

341 + this.IfDef?.Invoke(this, new IfDefEventArgs(sourceLineNumbers, false, !ifContext.IsTrue, name));

342 + break;

343

344 - case "elseif":

345 - if (0 == ifStack.Count)

346 - {

347 - throw new WixException(ErrorMessages.UnmatchedPreprocessorInstruction(sourceLineNumbers, "if", "elseif"));

348 - }

344 + case "elseif":

345 + if (0 == ifStack.Count)

346 + {

347 + throw new WixException(ErrorMessages.UnmatchedPreprocessorInstruction(sourceLineNumbers, "if", "elseif"));

348 + }

349

350 - if (IfState.If != ifContext.IfState && IfState.ElseIf != ifContext.IfState)

351 - {

352 - throw new WixException(ErrorMessages.UnmatchedPreprocessorInstruction(sourceLineNumbers, "if", "elseif"));

353 - }

350 + if (IfState.If != ifContext.IfState && IfState.ElseIf != ifContext.IfState)

351 + {

352 + throw new WixException(ErrorMessages.UnmatchedPreprocessorInstruction(sourceLineNumbers, "if", "elseif"));

353 + }

354

355 - ifContext.IfState = IfState.ElseIf; // we're now in an elseif

356 - if (!ifContext.WasEverTrue) // if we've never evaluated the if context to true, then we can try this test

357 - {

358 - ifContext.IsTrue = this.EvaluateExpression(state, reader.Value);

359 - }

360 - else if (ifContext.IsTrue)

361 - {

362 - ifContext.IsTrue = false;

363 - }

364 - ignore = true;

365 - break;

355 + ifContext.IfState = IfState.ElseIf; // we're now in an elseif

356 + if (!ifContext.WasEverTrue) // if we've never evaluated the if context to true, then we can try this test

357 + {

358 + ifContext.IsTrue = this.EvaluateExpression(state, reader.Value);

359 + }

360 + else if (ifContext.IsTrue)

361 + {

362 + ifContext.IsTrue = false;

363 + }

364 + ignore = true;

365 + break;

366

367 - case "else":

368 - if (0 == ifStack.Count)

369 - {

370 - throw new WixException(ErrorMessages.UnmatchedPreprocessorInstruction(sourceLineNumbers, "if", "else"));

371 - }

367 + case "else":

368 + if (0 == ifStack.Count)

369 + {

370 + throw new WixException(ErrorMessages.UnmatchedPreprocessorInstruction(sourceLineNumbers, "if", "else"));

371 + }

372

373 - if (IfState.If != ifContext.IfState && IfState.ElseIf != ifContext.IfState)

374 - {

375 - throw new WixException(ErrorMessages.UnmatchedPreprocessorInstruction(sourceLineNumbers, "if", "else"));

376 - }

373 + if (IfState.If != ifContext.IfState && IfState.ElseIf != ifContext.IfState)

374 + {

375 + throw new WixException(ErrorMessages.UnmatchedPreprocessorInstruction(sourceLineNumbers, "if", "else"));

376 + }

377

378 - ifContext.IfState = IfState.Else; // we're now in an else

379 - ifContext.IsTrue = !ifContext.WasEverTrue; // if we were never true, we can be true now

380 - ignore = true;

381 - break;

378 + ifContext.IfState = IfState.Else; // we're now in an else

379 + ifContext.IsTrue = !ifContext.WasEverTrue; // if we were never true, we can be true now

380 + ignore = true;

381 + break;

382

383 - case "endif":

384 - if (0 == ifStack.Count)

385 - {

386 - throw new WixException(ErrorMessages.UnmatchedPreprocessorInstruction(sourceLineNumbers, "if", "endif"));

387 - }

383 + case "endif":

384 + if (0 == ifStack.Count)

385 + {

386 + throw new WixException(ErrorMessages.UnmatchedPreprocessorInstruction(sourceLineNumbers, "if", "endif"));

387 + }

388

389 - ifContext = ifStack.Pop();

390 - ignore = true;

391 - break;

389 + ifContext = ifStack.Pop();

390 + ignore = true;

391 + break;

392 }

393

394 if (ignore) // ignore this node since we just handled it above

@@ -404,132 +404,132 @@ namespace WixToolset.Core

404

405 switch (reader.NodeType)

406 {

407 - case XmlNodeType.XmlDeclaration:

408 - if (currentContainer is XDocument document)

409 - {

410 - document.Declaration = new XDeclaration(null, null, null);

411 - while (reader.MoveToNextAttribute())

407 + case XmlNodeType.XmlDeclaration:

408 + if (currentContainer is XDocument document)

409 {

413 - switch (reader.LocalName)

410 + document.Declaration = new XDeclaration(null, null, null);

411 + while (reader.MoveToNextAttribute())

412 {

415 - case "version":

416 - document.Declaration.Version = reader.Value;

417 - break;

418 -

419 - case "encoding":

420 - document.Declaration.Encoding = reader.Value;

421 - break;

422 -

423 - case "standalone":

424 - document.Declaration.Standalone = reader.Value;

425 - break;

413 + switch (reader.LocalName)

414 + {

415 + case "version":

416 + document.Declaration.Version = reader.Value;

417 + break;

418 +

419 + case "encoding":

420 + document.Declaration.Encoding = reader.Value;

421 + break;

422 +

423 + case "standalone":

424 + document.Declaration.Standalone = reader.Value;

425 + break;

426 + }

427 }

428 }

428 - }

429 - //else

430 - //{

431 - // display an error? Can this happen?

432 - //}

433 - break;

434 -

435 - case XmlNodeType.ProcessingInstruction:

436 - switch (reader.LocalName)

437 - {

438 - case "define":

439 - this.PreprocessDefine(state, reader.Value);

429 + //else

430 + //{

431 + // display an error? Can this happen?

432 + //}

433 break;

434

442 - case "error":

443 - this.PreprocessError(state, reader.Value);

444 - break;

435 + case XmlNodeType.ProcessingInstruction:

436 + switch (reader.LocalName)

437 + {

438 + case "define":

439 + this.PreprocessDefine(state, reader.Value);

440 + break;

441

446 - case "warning":

447 - this.PreprocessWarning(state, reader.Value);

448 - break;

442 + case "error":

443 + this.PreprocessError(state, reader.Value);

444 + break;

445

450 - case "undef":

451 - this.PreprocessUndef(state, reader.Value);

452 - break;

446 + case "warning":

447 + this.PreprocessWarning(state, reader.Value);

448 + break;

449

454 - case "include":

455 - this.UpdateCurrentLineNumber(state, reader, offset);

456 - this.PreprocessInclude(state, reader.Value, currentContainer);

457 - break;

450 + case "undef":

451 + this.PreprocessUndef(state, reader.Value);

452 + break;

453

459 - case "foreach":

460 - this.PreprocessForeach(state, reader, currentContainer, offset);

461 - break;

454 + case "include":

455 + this.UpdateCurrentLineNumber(state, reader, offset);

456 + this.PreprocessInclude(state, reader.Value, currentContainer);

457 + break;

458

463 - case "endforeach": // endforeach is handled in PreprocessForeach, so seeing it here is an error

464 - throw new WixException(ErrorMessages.UnmatchedPreprocessorInstruction(sourceLineNumbers, "foreach", "endforeach"));

459 + case "foreach":

460 + this.PreprocessForeach(state, reader, currentContainer, offset);

461 + break;

462

466 - case "pragma":

467 - this.PreprocessPragma(state, reader.Value, currentContainer);

468 - break;

463 + case "endforeach": // endforeach is handled in PreprocessForeach, so seeing it here is an error

464 + throw new WixException(ErrorMessages.UnmatchedPreprocessorInstruction(sourceLineNumbers, "foreach", "endforeach"));

465

470 - default:

471 - // unknown processing instructions are currently ignored

466 + case "pragma":

467 + this.PreprocessPragma(state, reader.Value, currentContainer);

468 + break;

469 +

470 + default:

471 + // unknown processing instructions are currently ignored

472 + break;

473 + }

474 break;

473 - }

474 - break;

475

476 - case XmlNodeType.Element:

477 - if (0 < state.IncludeNextStack.Count && state.IncludeNextStack.Peek())

478 - {

479 - if ("Include" != reader.LocalName)

476 + case XmlNodeType.Element:

477 + if (0 < state.IncludeNextStack.Count && state.IncludeNextStack.Peek())

478 {

481 - this.Messaging.Write(ErrorMessages.InvalidDocumentElement(sourceLineNumbers, reader.Name, "include", "Include"));

482 - }

479 + if ("Include" != reader.LocalName)

480 + {

481 + this.Messaging.Write(ErrorMessages.InvalidDocumentElement(sourceLineNumbers, reader.Name, "include", "Include"));

482 + }

483

484 - state.IncludeNextStack.Pop();

485 - state.IncludeNextStack.Push(false);

486 - break;

487 - }

484 + state.IncludeNextStack.Pop();

485 + state.IncludeNextStack.Push(false);

486 + break;

487 + }

488

489 - var empty = reader.IsEmptyElement;

490 - var ns = XNamespace.Get(reader.NamespaceURI);

491 - var element = new XElement(ns + reader.LocalName);

492 - currentContainer.Add(element);

489 + var empty = reader.IsEmptyElement;

490 + var ns = XNamespace.Get(reader.NamespaceURI);

491 + var element = new XElement(ns + reader.LocalName);

492 + currentContainer.Add(element);

493

494 - this.UpdateCurrentLineNumber(state, reader, offset);

495 - element.AddAnnotation(sourceLineNumbers);

494 + this.UpdateCurrentLineNumber(state, reader, offset);

495 + element.AddAnnotation(sourceLineNumbers);

496

497 - while (reader.MoveToNextAttribute())

498 - {

499 - var value = state.Helper.PreprocessString(state.Context, reader.Value);

497 + while (reader.MoveToNextAttribute())

498 + {

499 + var value = state.Helper.PreprocessString(state.Context, reader.Value);

500

501 - var attribNamespace = XNamespace.Get(reader.NamespaceURI);

502 - attribNamespace = XNamespace.Xmlns == attribNamespace && reader.LocalName.Equals("xmlns") ? XNamespace.None : attribNamespace;

501 + var attribNamespace = XNamespace.Get(reader.NamespaceURI);

502 + attribNamespace = XNamespace.Xmlns == attribNamespace && reader.LocalName.Equals("xmlns") ? XNamespace.None : attribNamespace;

503

504 - element.Add(new XAttribute(attribNamespace + reader.LocalName, value));

505 - }

504 + element.Add(new XAttribute(attribNamespace + reader.LocalName, value));

505 + }

506

507 - if (!empty)

508 - {

509 - containerStack.Push(currentContainer);

510 - currentContainer = element;

511 - }

512 - break;

507 + if (!empty)

508 + {

509 + containerStack.Push(currentContainer);

510 + currentContainer = element;

511 + }

512 + break;

513

514 - case XmlNodeType.EndElement:

515 - if (0 < reader.Depth || !include)

516 - {

517 - currentContainer = containerStack.Pop();

518 - }

519 - break;

514 + case XmlNodeType.EndElement:

515 + if (0 < reader.Depth || !include)

516 + {

517 + currentContainer = containerStack.Pop();

518 + }

519 + break;

520

521 - case XmlNodeType.Text:

522 - var postprocessedText = state.Helper.PreprocessString(state.Context, reader.Value);

523 - currentContainer.Add(postprocessedText);

524 - break;

521 + case XmlNodeType.Text:

522 + var postprocessedText = state.Helper.PreprocessString(state.Context, reader.Value);

523 + currentContainer.Add(postprocessedText);

524 + break;

525

526 - case XmlNodeType.CDATA:

527 - var postprocessedValue = state.Helper.PreprocessString(state.Context, reader.Value);

528 - currentContainer.Add(new XCData(postprocessedValue));

529 - break;

526 + case XmlNodeType.CDATA:

527 + var postprocessedValue = state.Helper.PreprocessString(state.Context, reader.Value);

528 + currentContainer.Add(new XCData(postprocessedValue));

529 + break;

530

531 - default:

532 - break;

531 + default:

532 + break;

533 }

534 }

535

@@ -695,6 +695,17 @@ namespace WixToolset.Core

695 var varName = reader.Value.Substring(0, indexOfInToken).Trim();

696 var varValuesString = reader.Value.Substring(indexOfInToken + 4).Trim();

697

698 + if (varValuesString.StartsWith("\"", StringComparison.Ordinal))

699 + {

700 + if (!varValuesString.EndsWith("\"", StringComparison.Ordinal))

701 + {

702 + throw new WixException(ErrorMessages.UnmatchedQuotesInExpression(state.Context.CurrentSourceLineNumber, varValuesString));

703 + }

704 +

705 + // cut the quotes off the string

706 + varValuesString = varValuesString.Substring(1, varValuesString.Length - 2);

707 + }

708 +

709 // preprocess the variable values string because it might be a variable itself

710 varValuesString = state.Helper.PreprocessString(state.Context, varValuesString);

711

@@ -725,23 +736,23 @@ namespace WixToolset.Core

736 {

737 switch (reader.LocalName)

738 {

728 - case "foreach":

729 - ++nestedForeachCount;

730 - // Output the foreach statement

731 - fragmentBuilder.AppendFormat("<?foreach {0}?>", reader.Value);

732 - break;

739 + case "foreach":

740 + ++nestedForeachCount;

741 + // Output the foreach statement

742 + fragmentBuilder.AppendFormat("<?foreach {0}?>", reader.Value);

743 + break;

744

734 - case "endforeach":

735 - --nestedForeachCount;

736 - if (0 != nestedForeachCount)

737 - {

738 - fragmentBuilder.Append("<?endforeach ?>");

739 - }

740 - break;

745 + case "endforeach":

746 + --nestedForeachCount;

747 + if (0 != nestedForeachCount)

748 + {

749 + fragmentBuilder.Append("<?endforeach ?>");

750 + }

751 + break;

752

742 - default:

743 - fragmentBuilder.AppendFormat("<?{0} {1}?>", reader.LocalName, reader.Value);

744 - break;

753 + default:

754 + fragmentBuilder.AppendFormat("<?{0} {1}?>", reader.LocalName, reader.Value);

755 + break;

756 }

757 }

758 else if (reader.NodeType == XmlNodeType.Element)

@@ -1224,17 +1235,17 @@ namespace WixToolset.Core

1235 {

1236 switch (operation)

1237 {

1227 - case PreprocessorOperation.And:

1228 - currentValue = currentValue && prevResult;

1229 - break;

1230 - case PreprocessorOperation.Or:

1231 - currentValue = currentValue || prevResult;

1232 - break;

1233 - case PreprocessorOperation.Not:

1234 - currentValue = !currentValue;

1235 - break;

1236 - default:

1237 - throw new WixException(ErrorMessages.UnexpectedPreprocessorOperator(state.Context.CurrentSourceLineNumber, operation.ToString()));

1238 + case PreprocessorOperation.And:

1239 + currentValue = currentValue && prevResult;

1240 + break;

1241 + case PreprocessorOperation.Or:

1242 + currentValue = currentValue || prevResult;

1243 + break;

1244 + case PreprocessorOperation.Not:

1245 + currentValue = !currentValue;

1246 + break;

1247 + default:

1248 + throw new WixException(ErrorMessages.UnexpectedPreprocessorOperator(state.Context.CurrentSourceLineNumber, operation.ToString()));

1249 }

1250 }

1251

src/wix/test/WixToolsetTest.Core/PreprocessorFixture.cs

+50

@@ -266,6 +266,56 @@ namespace WixToolsetTest.Core

266 WixAssert.CompareLineByLine(expected, actual);

267 }

268

269 + [Fact]

270 + public void CanPreprocessForeach()

271 + {

272 + var input = String.Join(Environment.NewLine,

273 + "<Wix>",

274 + "<?foreach value in A ; B ; C ?>",

275 + " <Fragment Id='$(value)' />",

276 + "<?endforeach?>",

277 + "</Wix>"

278 + );

279 + var expected = new[]

280 + {

281 + "<Wix>",

282 + " <Fragment Id=\"A \" />",

283 + " <Fragment Id=\" B \" />",

284 + " <Fragment Id=\" C\" />",

285 + "</Wix>"

286 + };

287 +

288 + var result = PreprocessFromString(input);

289 +

290 + var actual = result.Document.ToString().Split("\r\n");

291 + WixAssert.CompareLineByLine(expected, actual);

292 + }

293 +

294 + [Fact]

295 + public void CanPreprocessForeachWithQuotes()

296 + {

297 + var input = String.Join(Environment.NewLine,

298 + "<Wix>",

299 + "<?foreach value in \" A ; B ; C \" ?>",

300 + " <Fragment Id='$(value)' />",

301 + "<?endforeach?>",

302 + "</Wix>"

303 + );

304 + var expected = new[]

305 + {

306 + "<Wix>",

307 + " <Fragment Id=\" A \" />",

308 + " <Fragment Id=\" B \" />",

309 + " <Fragment Id=\" C \" />",

310 + "</Wix>"

311 + };

312 +

313 + var result = PreprocessFromString(input);

314 +

315 + var actual = result.Document.ToString().Split("\r\n");

316 + WixAssert.CompareLineByLine(expected, actual);

317 + }

318 +

319 private static IPreprocessResult PreprocessFromString(string xml)

320 {

321 using var stringReader = new StringReader(xml);