From 61e4fa4c328c232445618dd4ae7a0c5f31776fc5 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 29 Oct 2024 11:31:13 +0000 Subject: [PATCH 1/3] streams: only consider LF and CRLF as newlines Git only considers LF (`\n`) and CRLF (`\r\n`) as valid line endings for the Git credential protocol. Lone carriage-returns should be treated as part of the value, and not a delimiter in the credential protocol. Override the behaviour of the standard `StreamReader`'s `ReadLineAsync` method to only break on LF or CRLF, in alignment with Git. Note that we also override the non-async `ReadLine` method too as this is also implemented separatley in the base class from the async version. We must also make allowances for .NET Framework where the override of `ReadLineAsync` that takes a `CancellationToken` does not exist. --- src/shared/Core.Tests/GitStreamReaderTests.cs | 193 ++++++++++++++++++ src/shared/Core/GitStreamReader.cs | 70 +++++++ src/shared/Core/StandardStreams.cs | 2 +- 3 files changed, 264 insertions(+), 1 deletion(-) create mode 100644 src/shared/Core.Tests/GitStreamReaderTests.cs create mode 100644 src/shared/Core/GitStreamReader.cs diff --git a/src/shared/Core.Tests/GitStreamReaderTests.cs b/src/shared/Core.Tests/GitStreamReaderTests.cs new file mode 100644 index 000000000..bf656d102 --- /dev/null +++ b/src/shared/Core.Tests/GitStreamReaderTests.cs @@ -0,0 +1,193 @@ +using System.IO; +using System.Text; +using System.Threading.Tasks; +using Xunit; + +namespace GitCredentialManager.Tests; + +public class GitStreamReaderTests +{ + #region ReadLineAsync + + [Fact] + public async Task GitStreamReader_ReadLineAsync_LF() + { + // hello\n + // world\n + + byte[] buffer = Encoding.UTF8.GetBytes("hello\nworld\n"); + using var stream = new MemoryStream(buffer); + var reader = new GitStreamReader(stream, Encoding.UTF8); + + string actual1 = await reader.ReadLineAsync(); + string actual2 = await reader.ReadLineAsync(); + string actual3 = await reader.ReadLineAsync(); + + Assert.Equal("hello", actual1); + Assert.Equal("world", actual2); + Assert.Null(actual3); + } + + [Fact] + public async Task GitStreamReader_ReadLineAsync_CR() + { + // hello\rworld\r + + byte[] buffer = Encoding.UTF8.GetBytes("hello\rworld\r"); + using var stream = new MemoryStream(buffer); + var reader = new GitStreamReader(stream, Encoding.UTF8); + + string actual1 = await reader.ReadLineAsync(); + string actual2 = await reader.ReadLineAsync(); + + Assert.Equal("hello\rworld\r", actual1); + Assert.Null(actual2); + } + + [Fact] + public async Task GitStreamReader_ReadLineAsync_CRLF() + { + // hello\r\n + // world\r\n + + byte[] buffer = Encoding.UTF8.GetBytes("hello\r\nworld\r\n"); + using var stream = new MemoryStream(buffer); + var reader = new GitStreamReader(stream, Encoding.UTF8); + + string actual1 = await reader.ReadLineAsync(); + string actual2 = await reader.ReadLineAsync(); + string actual3 = await reader.ReadLineAsync(); + + Assert.Equal("hello", actual1); + Assert.Equal("world", actual2); + Assert.Null(actual3); + } + + [Fact] + public async Task GitStreamReader_ReadLineAsync_Mixed() + { + // hello\r\n + // world\rthis\n + // is\n + // a\n + // \rmixed\rnewline\r\n + // \n + // string\n + + byte[] buffer = Encoding.UTF8.GetBytes("hello\r\nworld\rthis\nis\na\n\rmixed\rnewline\r\n\nstring\n"); + using var stream = new MemoryStream(buffer); + var reader = new GitStreamReader(stream, Encoding.UTF8); + + string actual1 = await reader.ReadLineAsync(); + string actual2 = await reader.ReadLineAsync(); + string actual3 = await reader.ReadLineAsync(); + string actual4 = await reader.ReadLineAsync(); + string actual5 = await reader.ReadLineAsync(); + string actual6 = await reader.ReadLineAsync(); + string actual7 = await reader.ReadLineAsync(); + string actual8 = await reader.ReadLineAsync(); + + Assert.Equal("hello", actual1); + Assert.Equal("world\rthis", actual2); + Assert.Equal("is", actual3); + Assert.Equal("a", actual4); + Assert.Equal("\rmixed\rnewline", actual5); + Assert.Equal("", actual6); + Assert.Equal("string", actual7); + Assert.Null(actual8); + } + + #endregion + + #region ReadLine + + [Fact] + public void GitStreamReader_ReadLine_LF() + { + // hello\n + // world\n + + byte[] buffer = Encoding.UTF8.GetBytes("hello\nworld\n"); + using var stream = new MemoryStream(buffer); + var reader = new GitStreamReader(stream, Encoding.UTF8); + + string actual1 = reader.ReadLine(); + string actual2 = reader.ReadLine(); + string actual3 = reader.ReadLine(); + + Assert.Equal("hello", actual1); + Assert.Equal("world", actual2); + Assert.Null(actual3); + } + + [Fact] + public void GitStreamReader_ReadLine_CR() + { + // hello\rworld\r + + byte[] buffer = Encoding.UTF8.GetBytes("hello\rworld\r"); + using var stream = new MemoryStream(buffer); + var reader = new GitStreamReader(stream, Encoding.UTF8); + + string actual1 = reader.ReadLine(); + string actual2 = reader.ReadLine(); + + Assert.Equal("hello\rworld\r", actual1); + Assert.Null(actual2); + } + + [Fact] + public void GitStreamReader_ReadLine_CRLF() + { + // hello\r\n + // world\r\n + + byte[] buffer = Encoding.UTF8.GetBytes("hello\r\nworld\r\n"); + using var stream = new MemoryStream(buffer); + var reader = new GitStreamReader(stream, Encoding.UTF8); + + string actual1 = reader.ReadLine(); + string actual2 = reader.ReadLine(); + string actual3 = reader.ReadLine(); + + Assert.Equal("hello", actual1); + Assert.Equal("world", actual2); + Assert.Null(actual3); + } + + [Fact] + public void GitStreamReader_ReadLine_Mixed() + { + // hello\r\n + // world\rthis\n + // is\n + // a\n + // \rmixed\rnewline\r\n + // \n + // string\n + + byte[] buffer = Encoding.UTF8.GetBytes("hello\r\nworld\rthis\nis\na\n\rmixed\rnewline\r\n\nstring\n"); + using var stream = new MemoryStream(buffer); + var reader = new GitStreamReader(stream, Encoding.UTF8); + + string actual1 = reader.ReadLine(); + string actual2 = reader.ReadLine(); + string actual3 = reader.ReadLine(); + string actual4 = reader.ReadLine(); + string actual5 = reader.ReadLine(); + string actual6 = reader.ReadLine(); + string actual7 = reader.ReadLine(); + string actual8 = reader.ReadLine(); + + Assert.Equal("hello", actual1); + Assert.Equal("world\rthis", actual2); + Assert.Equal("is", actual3); + Assert.Equal("a", actual4); + Assert.Equal("\rmixed\rnewline", actual5); + Assert.Equal("", actual6); + Assert.Equal("string", actual7); + Assert.Null(actual8); + } + + #endregion +} diff --git a/src/shared/Core/GitStreamReader.cs b/src/shared/Core/GitStreamReader.cs new file mode 100644 index 000000000..6512b2efc --- /dev/null +++ b/src/shared/Core/GitStreamReader.cs @@ -0,0 +1,70 @@ +using System.IO; +using System.Text; +using System.Threading; +using System.Threading.Tasks; + +namespace GitCredentialManager; + +/// +/// StreamReader that does NOT consider a lone carriage-return as a new-line character, +/// only a line-feed or carriage-return immediately followed by a line-feed. +/// +/// The only major operating system that uses a lone carriage-return as a new-line character +/// is the classic Macintosh OS (before OS X), which is not supported by Git. +/// +public class GitStreamReader : StreamReader +{ + public GitStreamReader(Stream stream, Encoding encoding) : base(stream, encoding) { } + + public override string ReadLine() + { +#if NETFRAMEWORK + return ReadLineAsync().ConfigureAwait(false).GetAwaiter().GetResult(); +#else + return ReadLineAsync(CancellationToken.None).ConfigureAwait(false).GetAwaiter().GetResult(); +#endif + } + +#if NETFRAMEWORK + public override async Task ReadLineAsync() +#else + public override async ValueTask ReadLineAsync(CancellationToken cancellationToken) +#endif + { + int nr; + var sb = new StringBuilder(); + var buffer = new char[1]; + bool lastWasCR = false; + + while ((nr = await base.ReadAsync(buffer, 0, 1).ConfigureAwait(false)) > 0) + { + char c = buffer[0]; + + // Only treat a line-feed as a new-line character. + // Carriage-returns alone are NOT considered new-line characters. + if (c == '\n') + { + if (lastWasCR) + { + // If the last character was a carriage-return we should remove it from the string builder + // since together with this line-feed it is considered a new-line character. + sb.Length--; + } + + // We have a new-line character, so we should stop reading. + break; + } + + lastWasCR = c == '\r'; + + sb.Append(c); + } + + if (sb.Length == 0 && nr == 0) + { + return null; + } + + return sb.ToString(); + } +} diff --git a/src/shared/Core/StandardStreams.cs b/src/shared/Core/StandardStreams.cs index d0b3042b0..45f9f6cc7 100644 --- a/src/shared/Core/StandardStreams.cs +++ b/src/shared/Core/StandardStreams.cs @@ -39,7 +39,7 @@ public TextReader In { if (_stdIn == null) { - _stdIn = new StreamReader(Console.OpenStandardInput(), EncodingEx.UTF8NoBom); + _stdIn = new GitStreamReader(Console.OpenStandardInput(), EncodingEx.UTF8NoBom); } return _stdIn; From 99e2f7f60e7364fe807e7925f361a81f3c47bd1b Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Wed, 30 Oct 2024 09:50:25 +0000 Subject: [PATCH 2/3] release.yml: use gatewatcher mac app certificate Use Gatewatcher to provision the Application certificate and password secrets in our release workflow. --- .github/workflows/release.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 3a2e708ca..59e7c8bc1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -57,8 +57,8 @@ jobs: - name: Set up signing/notarization infrastructure env: - A1: ${{ secrets.APPLICATION_CERTIFICATE_BASE64 }} - A2: ${{ secrets.APPLICATION_CERTIFICATE_PASSWORD }} + A1: ${{ secrets.GATEWATCHER_DEVELOPER_ID_CERT }} + A2: ${{ secrets.GATEWATCHER_DEVELOPER_ID_PASSWORD }} I1: ${{ secrets.INSTALLER_CERTIFICATE_BASE64 }} I2: ${{ secrets.INSTALLER_CERTIFICATE_PASSWORD }} N1: ${{ secrets.APPLE_TEAM_ID }} From 786ab03440ddc82e807a97c0e540f5247e44cec6 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Wed, 30 Oct 2024 09:52:19 +0000 Subject: [PATCH 3/3] VERSION: bump to 2.6.1 --- VERSION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION b/VERSION index 82f00d533..cfad4122e 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.6.0.0 +2.6.1.0