Skip to content

Allow EOLN in quoted config value and report better message when parsing fails #1160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tmat merged 1 commit into from
Dec 15, 2023
Merged

Allow EOLN in quoted config value and report better message when parsing fails #1160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 54 additions & 28 deletions src/Microsoft.Build.Tasks.Git.UnitTests/GitConfigTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ public void Sections()
[Fact]
public void Sections_Errors()
{
Assert.Throws<InvalidDataException>(() => LoadFromString("/", "/config", @"
var e = Assert.Throws<InvalidDataException>(() => LoadFromString("/", "/config", @"
[s]
a =
1"));

AssertEx.AreEqual(string.Format(Resources.ErrorParsingConfigLineInFile, 4, "/config", string.Format(Resources.UnexpectedCharacter, "U+0031")), e.Message);
}

[Fact]
Expand Down Expand Up @@ -206,7 +206,9 @@ TextReader openFile(string path)
});
}

Assert.Throws<InvalidDataException>(() => new GitConfig.Reader(gitDirectory, gitDirectory, GitEnvironment.Empty, openFile).LoadFrom(Path.Combine(gitDirectory, "config")));
var e = Assert.Throws<InvalidDataException>(() => new GitConfig.Reader(gitDirectory, gitDirectory, GitEnvironment.Empty, openFile).LoadFrom(Path.Combine(gitDirectory, "config")));

AssertEx.AreEqual(string.Format(Resources.ConfigurationFileRecursionExceededMaximumAllowedDepth, 10), e.Message);
}

[Theory]
Expand Down Expand Up @@ -286,22 +288,31 @@ public void HierarchicalLoad(bool enableProgramData, bool enableXdg, string expe
[InlineData("[X \"/*-\\a\"]", "x", "/*-a")]
public void ReadSectionHeader(string str, string name, string subsectionName)
{
GitConfig.Reader.ReadSectionHeader(new StringReader(str), new StringBuilder(), out var actualName, out var actualSubsectionName);
GitConfig.Reader.ReadSectionHeader(
new GitConfig.LineCountingReader(new StringReader(str), "path"), new StringBuilder(), out var actualName, out var actualSubsectionName);

Assert.Equal(name, actualName);
Assert.Equal(subsectionName, actualSubsectionName);
}

[Theory]
[InlineData("[")]
[InlineData("[x")]
[InlineData("[x x x]")]
[InlineData("[* \"\\")]
[InlineData("[* \"\\\"]")]
[InlineData("[* \"*\"]")]
[InlineData("[x \"y\" ]")]
public void ReadSectionHeader_Errors(string str)
[InlineData("[", -1)]
[InlineData("[x", -1)]
[InlineData("[x x x]", 'x')]
[InlineData("[* \"\\", '*')]
[InlineData("[* \"\\\"]", '*')]
[InlineData("[* \"*\"]", '*')]
[InlineData("[x \"y\" ]", ' ')]
public void ReadSectionHeader_Errors(string str, int unexpectedChar)
{
Assert.Throws<InvalidDataException>(() => GitConfig.Reader.ReadSectionHeader(new StringReader(str), new StringBuilder(), out _, out _));
var e = Assert.Throws<InvalidDataException>(() => GitConfig.Reader.ReadSectionHeader(
new GitConfig.LineCountingReader(new StringReader(str), "path"), new StringBuilder(), out _, out _));

var message = (unexpectedChar == -1)
? Resources.UnexpectedEndOfFile
: string.Format(Resources.UnexpectedCharacter, $@"U+{unexpectedChar:x4}");

AssertEx.AreEqual(string.Format(Resources.ErrorParsingConfigLineInFile, 1, "path", message), e.Message);
}

[Theory]
Expand Down Expand Up @@ -336,29 +347,44 @@ public void ReadSectionHeader_Errors(string str)
[InlineData("name= 1\\\"", "name", "1\"")]
[InlineData("name= ", "name", "")]
[InlineData("name=", "name", "")]
[InlineData("name=\"a\rb\"", "name", "a\rb")]
[InlineData("name=\"a\nb\"", "name", "a\nb")]
[InlineData("name=\"a\r\nb\"", "name", "a\r\nb")]
public void ReadVariableDeclaration(string str, string name, string value)
{
GitConfig.Reader.ReadVariableDeclaration(new StringReader(str), new StringBuilder(), out var actualName, out var actualValue);
GitConfig.Reader.ReadVariableDeclaration(
new GitConfig.LineCountingReader(new StringReader(str), "path"), new StringBuilder(), out var actualName, out var actualValue);

Assert.Equal(name, actualName);
Assert.Equal(value, actualValue);
}

[Theory]
[InlineData("")]
[InlineData("*")]
[InlineData("-=1")]
[InlineData("_=1")]
[InlineData("5=1")]
[InlineData("a_=1")]
[InlineData("a*=1")]
[InlineData("name=\\j")]
[InlineData("name=\"")]
[InlineData("name=\"a")]
[InlineData("name=\"a\n")]
[InlineData("name=\"a\nb")]
public void ReadVariableDeclaration_Errors(string str)
[InlineData("", -1)]
[InlineData("*", '*')]
[InlineData("-=1", '-')]
[InlineData("_=1", '_')]
[InlineData("5=1", '5')]
[InlineData("a_=1", '_')]
[InlineData("a*=1", '*')]
[InlineData("name=\\j", '\\')]
[InlineData("name=\"", -1)]
[InlineData("name=\"a", -1)]
[InlineData("name=\"a\n", -1, 2)]
[InlineData("name=\"a\nb", -1, 2)]
[InlineData("name=\"a\rb", -1, 2)]
[InlineData("name=\"a\r\nb", -1, 2)]
[InlineData("name=\"a\r\rb", -1, 3)]
public void ReadVariableDeclaration_Errors(string str, int unexpectedChar, int line = 1)
{
Assert.Throws<InvalidDataException>(() => GitConfig.Reader.ReadVariableDeclaration(new StringReader(str), new StringBuilder(), out _, out _));
var e = Assert.Throws<InvalidDataException>(() => GitConfig.Reader.ReadVariableDeclaration(
new GitConfig.LineCountingReader(new StringReader(str), "path"), new StringBuilder(), out _, out _));

var message = (unexpectedChar == -1)
? Resources.UnexpectedEndOfFile
: string.Format(Resources.UnexpectedCharacter, $@"U+{unexpectedChar:x4}");

AssertEx.AreEqual(string.Format(Resources.ErrorParsingConfigLineInFile, line, "path", message), e.Message);
}

[Theory]
Expand Down
93 changes: 71 additions & 22 deletions src/Microsoft.Build.Tasks.Git/GitDataReader/GitConfig.Reader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,40 @@ namespace Microsoft.Build.Tasks.Git
{
partial class GitConfig
{
internal class Reader
internal sealed class LineCountingReader(TextReader reader, string path)
{
/// <summary>
/// 1-based current line number.
/// </summary>
private int _lineNumber = 1;

private int _last = -1;

public int Read()
{
var c = reader.Read();

if (c == '\r' || c == '\n' && _last != '\r')
{
_lineNumber++;
}

_last = c;
return c;
}

public int Peek()
=> reader.Peek();

public void UnexpectedCharacter()
=> UnexpectedCharacter(Peek());

public void UnexpectedCharacter(int c)
=> throw new InvalidDataException(string.Format(Resources.ErrorParsingConfigLineInFile, _lineNumber, path,
(c == -1) ? Resources.UnexpectedEndOfFile : string.Format(Resources.UnexpectedCharacter, $"U+{c:x4}")));
}

internal sealed class Reader
{
private const int MaxIncludeDepth = 10;

Expand Down Expand Up @@ -169,11 +202,11 @@ internal void LoadVariablesFrom(string path, Dictionary<GitVariableName, List<st
throw new InvalidDataException(string.Format(Resources.ConfigurationFileRecursionExceededMaximumAllowedDepth, MaxIncludeDepth));
}

TextReader reader;
TextReader textReader;

try
{
reader = _fileOpener(path);
textReader = _fileOpener(path);
}
catch (Exception e) when (e is FileNotFoundException or DirectoryNotFoundException)
{
Expand All @@ -184,8 +217,10 @@ internal void LoadVariablesFrom(string path, Dictionary<GitVariableName, List<st
throw new IOException(e.Message, e);
}

using (reader)
using (textReader)
{
var reader = new LineCountingReader(textReader, path);

string sectionName = "";
string subsectionName = "";

Expand Down Expand Up @@ -320,7 +355,7 @@ private bool IsIncludePath(GitVariableName key, string configFilePath)
}

// internal for testing
internal static void ReadSectionHeader(TextReader reader, StringBuilder reusableBuffer, out string name, out string subsectionName)
internal static void ReadSectionHeader(LineCountingReader reader, StringBuilder reusableBuffer, out string name, out string subsectionName)
{
var nameBuilder = reusableBuffer.Clear();

Expand All @@ -345,7 +380,7 @@ internal static void ReadSectionHeader(TextReader reader, StringBuilder reusable
c = reader.Read();
if (c != ']')
{
throw new InvalidDataException();
reader.UnexpectedCharacter(c);
}

break;
Expand All @@ -358,7 +393,7 @@ internal static void ReadSectionHeader(TextReader reader, StringBuilder reusable
}
else
{
throw new InvalidDataException();
reader.UnexpectedCharacter(c);
}
}

Expand All @@ -381,14 +416,14 @@ internal static void ReadSectionHeader(TextReader reader, StringBuilder reusable
}
}

private static string ReadSubsectionName(TextReader reader, StringBuilder reusableBuffer)
private static string ReadSubsectionName(LineCountingReader reader, StringBuilder reusableBuffer)
{
SkipWhitespace(reader);

int c = reader.Read();
if (c != '"')
{
throw new InvalidDataException();
reader.UnexpectedCharacter(c);
}

var subsectionName = reusableBuffer.Clear();
Expand All @@ -397,7 +432,7 @@ private static string ReadSubsectionName(TextReader reader, StringBuilder reusab
c = reader.Read();
if (c <= 0)
{
throw new InvalidDataException();
reader.UnexpectedCharacter(c);
}

if (c == '"')
Expand All @@ -412,7 +447,7 @@ private static string ReadSubsectionName(TextReader reader, StringBuilder reusab
c = reader.Read();
if (c <= 0)
{
throw new InvalidDataException();
reader.UnexpectedCharacter(c);
}
}

Expand All @@ -421,12 +456,12 @@ private static string ReadSubsectionName(TextReader reader, StringBuilder reusab
}

// internal for testing
internal static void ReadVariableDeclaration(TextReader reader, StringBuilder reusableBuffer, out string name, out string value)
internal static void ReadVariableDeclaration(LineCountingReader reader, StringBuilder reusableBuffer, out string name, out string value)
{
name = ReadVariableName(reader, reusableBuffer);
if (name.Length == 0)
{
throw new InvalidDataException();
reader.UnexpectedCharacter();
}

SkipWhitespace(reader);
Expand All @@ -447,7 +482,7 @@ internal static void ReadVariableDeclaration(TextReader reader, StringBuilder re

if (c != '=')
{
throw new InvalidDataException();
reader.UnexpectedCharacter(c);
}

reader.Read();
Expand All @@ -457,7 +492,7 @@ internal static void ReadVariableDeclaration(TextReader reader, StringBuilder re
value = ReadVariableValue(reader, reusableBuffer);
}

private static string ReadVariableName(TextReader reader, StringBuilder reusableBuffer)
private static string ReadVariableName(LineCountingReader reader, StringBuilder reusableBuffer)
{
var nameBuilder = reusableBuffer.Clear();
int c;
Expand All @@ -472,10 +507,12 @@ private static string ReadVariableName(TextReader reader, StringBuilder reusable
return nameBuilder.ToString().ToLowerInvariant();
}

private static string ReadVariableValue(TextReader reader, StringBuilder reusableBuffer)
private static string ReadVariableValue(LineCountingReader reader, StringBuilder reusableBuffer)
{
// Allowed:
// name = "a"x"b" `axb`
// name = "a
// b" `a\n b`
// name = "b"#"a" `b`
// name = \
// abc `abc`
Expand All @@ -494,11 +531,22 @@ private static string ReadVariableValue(TextReader reader, StringBuilder reusabl
while (true)
{
int c = reader.Read();
if (c == -1 || IsEndOfLine(c))
if (c == -1)
{
if (inQuotes)
{
reader.UnexpectedCharacter(c);
}

break;
}

if (IsEndOfLine(c))
{
if (inQuotes)
{
throw new InvalidDataException();
builder.Append((char)c);
continue;
}

break;
Expand Down Expand Up @@ -536,7 +584,8 @@ private static string ReadVariableValue(TextReader reader, StringBuilder reusabl
continue;

default:
throw new InvalidDataException();
reader.UnexpectedCharacter(c);
break;
}
}

Expand All @@ -563,23 +612,23 @@ private static string ReadVariableValue(TextReader reader, StringBuilder reusabl
return builder.ToString(0, lengthIgnoringTrailingWhitespace);
}

private static void SkipMultilineWhitespace(TextReader reader)
private static void SkipMultilineWhitespace(LineCountingReader reader)
{
while (IsWhitespaceOrEndOfLine(reader.Peek()))
{
reader.Read();
}
}

private static void SkipWhitespace(TextReader reader)
private static void SkipWhitespace(LineCountingReader reader)
{
while (IsWhitespace(reader.Peek()))
{
reader.Read();
}
}

private static void ReadToLineEnd(TextReader reader)
private static void ReadToLineEnd(LineCountingReader reader)
{
while (true)
{
Expand Down
9 changes: 9 additions & 0 deletions src/Microsoft.Build.Tasks.Git/Resources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,4 +198,13 @@
<data name="RepositoryUrlEvaluationExceededMaximumAllowedDepth" xml:space="preserve">
<value>Repository URL evaluation exceeded maximum allowed depth of {0} local repositories.</value>
</data>
<data name="UnexpectedEndOfFile" xml:space="preserve">
<value>Unexpected end of file.</value>
</data>
<data name="UnexpectedCharacter" xml:space="preserve">
<value>Unexpected character {0}.</value>
</data>
<data name="ErrorParsingConfigLineInFile" xml:space="preserve">
<value>Error parsing config line {0} in file '{1}': {2}</value>
</data>
</root>
Loading