Skip to content

Commit

Permalink
Detect whitespace in property name (#5672)
Browse files Browse the repository at this point in the history
Fixes #5615
  • Loading branch information
mfkl authored Nov 4, 2020
1 parent 841e091 commit 0ad5f83
Show file tree
Hide file tree
Showing 17 changed files with 134 additions and 4 deletions.
31 changes: 31 additions & 0 deletions src/Build.UnitTests/Scanner_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

using Microsoft.Build.Evaluation;
using Microsoft.Build.Exceptions;
using Microsoft.Build.Utilities;
using Xunit;


Expand Down Expand Up @@ -100,6 +101,36 @@ public void IllFormedProperty()
Assert.Equal("IllFormedPropertyOpenParenthesisInCondition", lexer.GetErrorResource());
}

/// <summary>
/// Tests the space errors case
/// </summary>
[Fact]
public void SpaceProperty()
{
Scanner lexer = new Scanner("$(x )", ParserOptions.AllowProperties);
AdvanceToScannerError(lexer);
Assert.Equal("IllFormedPropertySpaceInCondition", lexer.GetErrorResource());

lexer = new Scanner("$( x)", ParserOptions.AllowProperties);
AdvanceToScannerError(lexer);
Assert.Equal("IllFormedPropertySpaceInCondition", lexer.GetErrorResource());
}

[Fact]
public void SpacePropertyOptOutWave16_10()
{
using TestEnvironment env = TestEnvironment.Create();
env.SetChangeWave(ChangeWaves.Wave16_10);

Scanner lexer = new Scanner("$(x )", ParserOptions.AllowProperties);
AdvanceToScannerError(lexer);
Assert.Null(lexer.UnexpectedlyFound);

lexer = new Scanner("$( x)", ParserOptions.AllowProperties);
AdvanceToScannerError(lexer);
Assert.Null(lexer.UnexpectedlyFound);
}

/// <summary>
/// Tests the special errors for "@(" and "@x" and similar cases.
/// </summary>
Expand Down
33 changes: 29 additions & 4 deletions src/Build/Evaluation/Conditionals/Scanner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Diagnostics;

using Microsoft.Build.Shared;
using Microsoft.Build.Utilities;

namespace Microsoft.Build.Evaluation
{
Expand Down Expand Up @@ -283,8 +284,17 @@ private string ParsePropertyOrItemMetadata()
return null;
}

_parsePoint = ScanForPropertyExpressionEnd(_expression, _parsePoint++);
var result = ScanForPropertyExpressionEnd(_expression, _parsePoint++, out int indexResult);
if (!result)
{
_errorState = true;
_errorPosition = indexResult;
_errorResource = "IllFormedPropertySpaceInCondition";
_unexpectedlyFound = Convert.ToString(_expression[indexResult], CultureInfo.InvariantCulture);
return null;
}

_parsePoint = indexResult;
// Maybe we need to generate an error for invalid characters in property/metadata name?
// For now, just wait and let the property/metadata evaluation handle the error case.
if (_parsePoint >= _expression.Length)
Expand All @@ -303,9 +313,15 @@ private string ParsePropertyOrItemMetadata()
/// <summary>
/// Scan for the end of the property expression
/// </summary>
private static int ScanForPropertyExpressionEnd(string expression, int index)
/// <param name="expression">property expression to parse</param>
/// <param name="index">current index to start from</param>
/// <param name="indexResult">If successful, the index corresponds to the end of the property expression.
/// In case of scan failure, it is the error position index.</param>
/// <returns>result indicating whether or not the scan was successful.</returns>
private static bool ScanForPropertyExpressionEnd(string expression, int index, out int indexResult)
{
int nestLevel = 0;
bool whitespaceCheck = false;

unsafe
{
Expand All @@ -317,18 +333,26 @@ private static int ScanForPropertyExpressionEnd(string expression, int index)
if (character == '(')
{
nestLevel++;
whitespaceCheck = true;
}
else if (character == ')')
{
nestLevel--;
whitespaceCheck = false;
}
else if (whitespaceCheck && char.IsWhiteSpace(character) && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave16_10))
{
indexResult = index;
return false;
}

// We have reached the end of the parenthesis nesting
// this should be the end of the property expression
// If it is not then the calling code will determine that
if (nestLevel == 0)
{
return index;
indexResult = index;
return true;
}
else
{
Expand All @@ -337,7 +361,8 @@ private static int ScanForPropertyExpressionEnd(string expression, int index)
}
}
}
return index;
indexResult = index;
return true;
}

/// <summary>
Expand Down
4 changes: 4 additions & 0 deletions src/Build/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,10 @@
<value>MSB4110: Expected a property at position {1} in condition "{0}". Did you forget the opening parenthesis after the '$'? To use a literal '$', use '%24' instead.</value>
<comment>{StrBegin="MSB4110: "}</comment>
</data>
<data name="IllFormedPropertySpaceInCondition" xml:space="preserve">
<value>MSB4259: Unexpected space at position "{1}" of condition "{0}". Did you forget to remove a space?</value>
<comment>{StrBegin="MSB4259: "}</comment>
</data>
<data name="IllFormedQuotedStringInCondition" xml:space="preserve">
<value>MSB4101: Expected a closing quote after position {1} in condition "{0}".</value>
<comment>{StrBegin="MSB4101: "}</comment>
Expand Down
5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.en.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.ja.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.ko.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.pl.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.pt-BR.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.ru.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.tr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.zh-Hans.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.zh-Hant.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 0ad5f83

Please sign in to comment.