Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 6 additions & 7 deletions GVFS/GVFS.Common/NamedPipes/NamedPipeClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ public class NamedPipeClient : IDisposable
{
private string pipeName;
private NamedPipeClientStream clientStream;
private StreamReader reader;
private StreamWriter writer;
private NamedPipeStreamReader reader;
private NamedPipeStreamWriter writer;

public NamedPipeClient(string pipeName)
{
Expand Down Expand Up @@ -37,8 +37,8 @@ public bool Connect(int timeoutMilliseconds = 3000)
return false;
}

this.reader = new StreamReader(this.clientStream);
this.writer = new StreamWriter(this.clientStream);
this.reader = new NamedPipeStreamReader(this.clientStream);
this.writer = new NamedPipeStreamWriter(this.clientStream);

return true;
}
Expand Down Expand Up @@ -68,8 +68,7 @@ public void SendRequest(string message)

try
{
this.writer.WritePlatformIndependentLine(message);
this.writer.Flush();
this.writer.WriteMessage(message);
}
catch (IOException e)
{
Expand All @@ -81,7 +80,7 @@ public string ReadRawResponse()
{
try
{
string response = this.reader.ReadLine();
string response = this.reader.ReadMessage();
if (response == null)
{
throw new BrokenPipeException("Unable to read from pipe", null);
Expand Down
45 changes: 26 additions & 19 deletions GVFS/GVFS.Common/NamedPipes/NamedPipeServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ private void OpenListeningPipe()
private void OnNewConnection(IAsyncResult ar)
{
if (!this.isStopping)
{
{
this.OnNewConnection(ar, createNewThreadIfSynchronous: true);
}
}
Expand Down Expand Up @@ -137,14 +137,14 @@ private void OnNewConnection(IAsyncResult ar, bool createNewThreadIfSynchronous)

if (!connectionBroken)
{
try
{
this.handleConnection(new Connection(pipe, () => this.isStopping));
}
catch (Exception e)
{
this.LogErrorAndExit("Unhandled exception in connection handler", e);
}
try
{
this.handleConnection(new Connection(pipe, this.tracer, () => this.isStopping));
}
catch (Exception e)
{
this.LogErrorAndExit("Unhandled exception in connection handler", e);
}
}
}
}
Expand Down Expand Up @@ -174,16 +174,18 @@ private void LogErrorAndExit(string message, Exception e)
public class Connection
{
private NamedPipeServerStream serverStream;
private StreamReader reader;
private StreamWriter writer;
private NamedPipeStreamReader reader;
private NamedPipeStreamWriter writer;
private ITracer tracer;
private Func<bool> isStopping;

public Connection(NamedPipeServerStream serverStream, Func<bool> isStopping)
public Connection(NamedPipeServerStream serverStream, ITracer tracer, Func<bool> isStopping)
{
this.serverStream = serverStream;
this.tracer = tracer;
this.isStopping = isStopping;
this.reader = new StreamReader(this.serverStream);
this.writer = new StreamWriter(this.serverStream);
this.reader = new NamedPipeStreamReader(this.serverStream);
this.writer = new NamedPipeStreamWriter(this.serverStream);
}

public bool IsConnected
Expand All @@ -200,10 +202,17 @@ public string ReadRequest()
{
try
{
return this.reader.ReadLine();
return this.reader.ReadMessage();
}
catch (IOException)
catch (IOException e)
{
EventMetadata metadata = new EventMetadata();
metadata.Add("ExceptionMessage", e.Message);
metadata.Add("StackTrace", e.StackTrace);
this.tracer.RelatedError(
metadata: metadata,
message: $"Error reading message from NamedPipe: {e.Message}");

return null;
}
}
Expand All @@ -212,9 +221,7 @@ public bool TrySendResponse(string message)
{
try
{
this.writer.WritePlatformIndependentLine(message);
this.writer.Flush();

this.writer.WriteMessage(message);
return true;
}
catch (IOException)
Expand Down
92 changes: 92 additions & 0 deletions GVFS/GVFS.Common/NamedPipes/NamedPipeStreamReader.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text;

namespace GVFS.Common.NamedPipes
{
/// <summary>
/// Implements the NamedPipe protocol as described in NamedPipeServer.
/// </summary>
public class NamedPipeStreamReader
{
private const int DefaultBufferSize = 1024;
private const byte TerminatorByte = 0x3;

private int bufferSize;
private byte[] buffer;
private Stream stream;

public NamedPipeStreamReader(Stream stream, int bufferSize)
{
this.stream = stream;
this.bufferSize = bufferSize;
this.buffer = new byte[this.bufferSize];
}

public NamedPipeStreamReader(Stream stream)
: this(stream, DefaultBufferSize)
{
}

/// <summary>
/// Read a message from the stream.
/// </summary>
/// <returns>The message read from the stream, or null if the end of the input stream has been reached. </returns>
public string ReadMessage()
{
int bytesRead = this.Read();
if (bytesRead == 0)
{
// The end of the stream has been reached - return null to indicate this.
return null;
}

// If we have read in the entire message in the first read (mainline scenario),
// then just process the data directly from the buffer.
if (this.buffer[bytesRead - 1] == TerminatorByte)
{
return Encoding.UTF8.GetString(this.buffer, 0, bytesRead - 1);
}

// We need to process multiple chunks - collect data from multiple chunks
// into a single list
List<byte> bytes = new List<byte>(this.bufferSize * 2);

while (true)
{
bool encounteredTerminatorByte = this.buffer[bytesRead - 1] == TerminatorByte;
int lengthToCopy = encounteredTerminatorByte ? bytesRead - 1 : bytesRead;

bytes.AddRange(new ArraySegment<byte>(this.buffer, 0, lengthToCopy));
if (encounteredTerminatorByte)
{
break;
}

bytesRead = this.Read();

if (bytesRead == 0)
{
// We have read a partial message (the last byte received does not indicate that
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we log the partial message?

// this was the end of the message), but the stream has been closed. Throw an exception
// and let upper layer deal with this condition.

throw new IOException("Incomplete message read from stream. The end of the stream was reached without the expected terminating byte.");
}
}

return Encoding.UTF8.GetString(bytes.ToArray());
}

/// <summary>
/// Read the next chunk of bytes from the stream.
/// </summary>
/// <returns>The number of bytes read.</returns>
private int Read()
{
return this.stream.Read(this.buffer, 0, this.buffer.Length);
}
}
}
24 changes: 24 additions & 0 deletions GVFS/GVFS.Common/NamedPipes/NamedPipeStreamWriter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
using System.IO;
using System.Text;

namespace GVFS.Common.NamedPipes
{
public class NamedPipeStreamWriter
{
private const byte TerminatorByte = 0x3;
private const string TerminatorByteString = "\x3";
private Stream stream;

public NamedPipeStreamWriter(Stream stream)
{
this.stream = stream;
}

public void WriteMessage(string message)
{
byte[] byteBuffer = Encoding.UTF8.GetBytes(message + TerminatorByteString);
this.stream.Write(byteBuffer, 0, byteBuffer.Length);
this.stream.Flush();
}
}
}
16 changes: 0 additions & 16 deletions GVFS/GVFS.Common/NamedPipes/StreamWriterExtensions.cs

This file was deleted.

10 changes: 10 additions & 0 deletions GVFS/GVFS.FunctionalTests/Tests/GitCommands/GitCommandsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,16 @@ public void AddFileInSubfolderAndCommitOnNewBranchSwitchDeleteFolderAndSwitchBac
this.FileShouldHaveContents(newFileContents, newFilePath);
}

[TestCase]
public void CommitWithNewlinesInMessage()
{
this.ValidateGitCommand("checkout -b tests/functional/commit_with_uncommon_arguments");
this.CreateFile();
this.ValidateGitCommand("status");
this.ValidateGitCommand("add .");
this.RunGitCommand("commit -m \"Message that contains \na\nnew\nline\"");
}

[TestCase]
public void CaseOnlyRenameFileAndChangeBranches()
{
Expand Down
10 changes: 10 additions & 0 deletions GVFS/GVFS.FunctionalTests/Tests/GitCommands/UpdateIndexTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,16 @@ public void UpdateIndexRemoveAddFileOpenForWrite()
GitHelpers.InvokeGitAgainstGVFSRepo(this.Enlistment.RepoRoot, "update-index --add Test_ConflictTests/AddedFiles/AddedByBothDifferentContent.txt");
}

[TestCase]
[Category(Categories.MacTODO.M4)]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this fail on Mac? If so, what's the error (out of curiosity)?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not tried running this on the Mac - I will test and see what happens (or remove this, if possible)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test fails on the "cleanup" code (not related to the protocol changes). After "resetting" the repository, there are unexpected changes in the test repository. Because of this (and because the failure is not related to the protocol changes), I will leave this as a to for Mac

public void UpdateIndexWithCacheInfo()
{
// Update Protocol.md with the contents from blob 583f1...
string command = $"update-index --cacheinfo 100644 \"583f1a56db7cc884d54534c5d9c56b93a1e00a2b\n\" Protocol.md";

this.ValidateGitCommand(command);
}

protected override void CreateEnlistment()
{
base.CreateEnlistment();
Expand Down
7 changes: 5 additions & 2 deletions GVFS/GVFS.Hooks/GVFS.Hooks.Mac.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,11 @@
<Compile Include="..\GVFS.Common\NamedPipes\NamedPipeClient.cs">
<Link>Common\NamedPipes\NamedPipeClient.cs</Link>
</Compile>
<Compile Include="..\GVFS.Common\NamedPipes\StreamWriterExtensions.cs">
<Link>Common\NamedPipes\StreamWriterExtensions.cs</Link>
<Compile Include="..\GVFS.Common\NamedPipes\NamedPipeStreamReader.cs">
<Link>Common\NamedPipes\NamedPipeStreamReader.cs</Link>
</Compile>
<Compile Include="..\GVFS.Common\NamedPipes\NamedPipeStreamWriter.cs">
<Link>Common\NamedPipes\NamedPipeStreamWriter.cs</Link>
</Compile>
<Compile Include="..\GVFS.Common\NativeMethods.Shared.cs">
<Link>Common\NativeMethods.Shared.cs</Link>
Expand Down
7 changes: 5 additions & 2 deletions GVFS/GVFS.Hooks/GVFS.Hooks.Windows.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,11 @@
<Compile Include="..\GVFS.Common\NamedPipes\NamedPipeClient.cs">
<Link>Common\NamedPipes\NamedPipeClient.cs</Link>
</Compile>
<Compile Include="..\GVFS.Common\NamedPipes\StreamWriterExtensions.cs">
<Link>Common\NamedPipes\StreamWriterExtensions.cs</Link>
<Compile Include="..\GVFS.Common\NamedPipes\NamedPipeStreamReader.cs">
<Link>Common\NamedPipes\NamedPipeStreamReader.cs</Link>
</Compile>
<Compile Include="..\GVFS.Common\NamedPipes\NamedPipeStreamWriter.cs">
<Link>Common\NamedPipes\NamedPipeStreamWriter.cs</Link>
</Compile>
<Compile Include="..\GVFS.Common\NativeMethods.Shared.cs">
<Link>Common\NativeMethods.Shared.cs</Link>
Expand Down
6 changes: 3 additions & 3 deletions GVFS/GVFS.ReadObjectHook/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
#define DLO_REQUEST_LENGTH (4 + SHA1_LENGTH + 1)

// Expected response:
// "S\n" -> Success
// "F\n" -> Failure
// "S\x3" -> Success
// "F\x3" -> Failure
#define DLO_RESPONSE_LENGTH 2

enum ReadObjectHookErrorReturnCode
Expand All @@ -33,7 +33,7 @@ int DownloadSHA(PIPE_HANDLE pipeHandle, const char *sha1)
// Format: "DLO|<40 character SHA>"
// Example: "DLO|920C34DCDDFC8F07AC4704C8C0D087D6F2095729"
char request[DLO_REQUEST_LENGTH+1];
if (snprintf(request, DLO_REQUEST_LENGTH+1, "DLO|%s\n", sha1) != DLO_REQUEST_LENGTH)
if (snprintf(request, DLO_REQUEST_LENGTH+1, "DLO|%s\x3", sha1) != DLO_REQUEST_LENGTH)
{
die(ReturnCode::InvalidSHA, "First argument must be a 40 character SHA, actual value: %s\n", sha1);
}
Expand Down
Loading