From 65003ffe37898196d53718db731a5765745981da Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Tue, 9 Jun 2020 08:41:07 -0700 Subject: [PATCH 1/4] Remove check for 5F/60 --- src/Shared/CommunicationsUtilities.cs | 17 ++--------------- src/Shared/NodeEndpointOutOfProcBase.cs | 2 +- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/src/Shared/CommunicationsUtilities.cs b/src/Shared/CommunicationsUtilities.cs index d51c883edae..0634b29c040 100644 --- a/src/Shared/CommunicationsUtilities.cs +++ b/src/Shared/CommunicationsUtilities.cs @@ -377,7 +377,7 @@ internal static long ReadLongForHandshake(this PipeStream stream #endif ) { - return stream.ReadLongForHandshake((byte[])null, 0 + return stream.ReadLongForHandshake(0 #if NETCOREAPP2_1 || MONO , handshakeReadTimeout #endif @@ -388,7 +388,7 @@ internal static long ReadLongForHandshake(this PipeStream stream /// Extension method to read a series of bytes from a stream. /// If specified, leading byte matches one in the supplied array if any, returns rejection byte and throws IOException. /// - internal static long ReadLongForHandshake(this PipeStream stream, byte[] leadingBytesToReject, + internal static long ReadLongForHandshake(this PipeStream stream, byte rejectionByteToReturn #if NETCOREAPP2_1 || MONO , int timeout @@ -435,19 +435,6 @@ byte rejectionByteToReturn throw new IOException(String.Format(CultureInfo.InvariantCulture, "Unexpected end of stream while reading for handshake")); } - if (i == 0 && leadingBytesToReject != null) - { - foreach (byte reject in leadingBytesToReject) - { - if (read == reject) - { - stream.WriteByte(rejectionByteToReturn); // disconnect the host - - throw new IOException(String.Format(CultureInfo.InvariantCulture, "Client: rejected old host. Received byte {0} but this matched a byte to reject.", bytes[i])); // disconnect and quit - } - } - } - bytes[i] = Convert.ToByte(read); } } diff --git a/src/Shared/NodeEndpointOutOfProcBase.cs b/src/Shared/NodeEndpointOutOfProcBase.cs index 8f24e1c56c8..3aff39ab1d7 100644 --- a/src/Shared/NodeEndpointOutOfProcBase.cs +++ b/src/Shared/NodeEndpointOutOfProcBase.cs @@ -400,7 +400,7 @@ private void PacketPumpProc() // it will cause the host to reject us; new hosts expect 00 and old hosts expect F5 or 06). try { - long handshake = localReadPipe.ReadLongForHandshake(/* reject these leads */ new byte[] { 0x5F, 0x60 }, 0xFF /* this will disconnect the host; it expects leading 00 or F5 or 06 */ + long handshake = localReadPipe.ReadLongForHandshake(0xFF /* this will disconnect the host; it expects leading 00 or F5 or 06 */ #if NETCOREAPP2_1 || MONO , ClientConnectTimeout /* wait a long time for the handshake from this side */ #endif From f589e6573335386230bc4a2c4f0f39bd14d12ace Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Tue, 9 Jun 2020 12:25:09 -0700 Subject: [PATCH 2/4] Remove outdated comment --- src/Shared/NodeEndpointOutOfProcBase.cs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/Shared/NodeEndpointOutOfProcBase.cs b/src/Shared/NodeEndpointOutOfProcBase.cs index 3aff39ab1d7..c53cd7acc5f 100644 --- a/src/Shared/NodeEndpointOutOfProcBase.cs +++ b/src/Shared/NodeEndpointOutOfProcBase.cs @@ -384,20 +384,6 @@ private void PacketPumpProc() // respond with another long. Once the handshake is complete, both sides can be assured the // other is ready to accept data. // To avoid mixing client and server builds, the long is the MSBuild binary timestamp. - - // Compatibility issue here. - // Previous builds of MSBuild 4.0 would exchange just a byte. - // Host would send either 0x5F or 0x60 depending on whether it was the toolset or not respectively. - // Client would return either 0xF5 or 0x06 respectively. - // Therefore an old host on a machine with new clients running will hang, - // sending a byte and waiting for a byte until it eventually times out; - // because the new client will want 7 more bytes before it returns anything. - // The other way around is not a problem, because the old client would immediately return the (wrong) - // byte on receiving the first byte of the long sent by the new host, and the new host would disconnect. - // To avoid the hang, special case here: - // Make sure our handshakes always start with 00. - // If we received ONLY one byte AND it's 0x5F or 0x60, return 0xFF (it doesn't matter what as long as - // it will cause the host to reject us; new hosts expect 00 and old hosts expect F5 or 06). try { long handshake = localReadPipe.ReadLongForHandshake(0xFF /* this will disconnect the host; it expects leading 00 or F5 or 06 */ From da3a1b9161a0f46f4484a456f1e29d4ccaebb075 Mon Sep 17 00:00:00 2001 From: Nathan Mytelka Date: Tue, 9 Jun 2020 12:29:38 -0700 Subject: [PATCH 3/4] Unmask change --- src/Shared/CommunicationsUtilities.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Shared/CommunicationsUtilities.cs b/src/Shared/CommunicationsUtilities.cs index 911f443f72b..15372f0766f 100644 --- a/src/Shared/CommunicationsUtilities.cs +++ b/src/Shared/CommunicationsUtilities.cs @@ -311,11 +311,7 @@ private static long GenerateHostHandshakeFromBase(long baseHandshake) } } #endif - - // Mask out the first byte. Modern builds expect the first byte to be zero to indicate that they are modern - // and should be treated as such. Older builds used a non-zero initial byte. See here: - // https://github.com/microsoft/msbuild/blob/584ca5f11b28971f5651b4b8de5f173ad1cb2786/src/Shared/NodeEndpointOutOfProcBase.cs#L403. - return baseHandshake & 0x00FFFFFFFFFFFFFF; + return baseHandshake; } /// From 0ab87949aa08d96dbc2dc3bbc0689e0b6181cd6c Mon Sep 17 00:00:00 2001 From: Forgind Date: Tue, 9 Jun 2020 12:36:49 -0700 Subject: [PATCH 4/4] Update src/Shared/NodeEndpointOutOfProcBase.cs Co-authored-by: Rainer Sigwald --- src/Shared/NodeEndpointOutOfProcBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Shared/NodeEndpointOutOfProcBase.cs b/src/Shared/NodeEndpointOutOfProcBase.cs index c53cd7acc5f..eaf14bde735 100644 --- a/src/Shared/NodeEndpointOutOfProcBase.cs +++ b/src/Shared/NodeEndpointOutOfProcBase.cs @@ -386,7 +386,7 @@ private void PacketPumpProc() // To avoid mixing client and server builds, the long is the MSBuild binary timestamp. try { - long handshake = localReadPipe.ReadLongForHandshake(0xFF /* this will disconnect the host; it expects leading 00 or F5 or 06 */ + long handshake = localReadPipe.ReadLongForHandshake(0xFF /* this will disconnect a < 4.5 host; it expects leading 00 or F5 or 06 */ #if NETCOREAPP2_1 || MONO , ClientConnectTimeout /* wait a long time for the handshake from this side */ #endif