Skip to content

Commit 0f8f4ba

Browse files
AndyHeap-NeoTechAndyHeap-NeoTech
authored
Feature/new handshake (#837)
* New Handshake Manifest style. Implementation of the new handshake as per ADR-30. Main logic is contained in the BoltHandShaker class. There have also been updates to the BoltProtocolFactory and BoltProtocolVersion to support identifying versions. A new 'type' VarLong has been added to the util folder, this implements VarInt logic the description of which can be found in the ADR and all good search engines. * Changed the handling of version ranges to take a int specifiying how many verisons. Previously it was erroniously reading a full version thinking that that represented the lower bound. Added support for testkits Feature:Bolt:HandshakeManifestV1 Fixed a bug where the encoded response was being sent with an incorrect length. * Added debug level logging so that the bolt messages read in and written out are logged. Removed member variables from the class as it is used as a singleton and state was being carried between multiple handshake negotiations. Means more variables being passed around but I don't want to reengineer at this point. Updated tests to accomodate the change to member variables. * Ensured all member methods of the singleton handshaker class were static. * Added comment to explain IComparable interface implementation of CompareTo Changed type in VarLong.AddSegment to byte.
1 parent cfff47c commit 0f8f4ba

File tree

10 files changed

+484
-49
lines changed

10 files changed

+484
-49
lines changed

‎Neo4j.Driver/Neo4j.Driver.Tests.TestBackend/SupportedFeatures.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ static SupportedFeatures()
6868
"Feature:Bolt:5.6",
6969
"Feature:Bolt:5.7",
7070
"Feature:Bolt:Patch:UTC",
71+
"Feature:Bolt:HandshakeManifestV1",
7172
"Feature:Impersonation",
7273
//"Feature:TLS:1.1",
7374
"Feature:TLS:1.2",

‎Neo4j.Driver/Neo4j.Driver.Tests/Connector/BoltHandshakerTests.cs‎

Lines changed: 107 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,24 +25,91 @@
2525
namespace Neo4j.Driver.Tests.Connector;
2626

2727
public class BoltHandshakerTests
28-
{
29-
[Fact]
30-
public async Task DoHandshakeAsyncShouldReturnBoltVersion()
28+
{
29+
[Theory]
30+
//Server response Legacy Handshake
31+
[InlineData(new byte[] { 0x00, 0x00, 4, 4 }, 4, 4)] //Capabilities not used in this test
32+
//Modern negotiation with manifest
33+
[InlineData(new byte[]
34+
{
35+
0x00, 0x00, 0x01, 0xFF, //Identifies this as a modern negotiation with manifest v1
36+
0x03, //3 versions to follow
37+
0x00, 0x04, 0x04, 0x04, //Supports 4.0-4.4
38+
0x00, 0x05, 0x05, 0x05, //Supports 5.0-5.5
39+
0x00, 0x00, 0x07, 0x05, //Supports version 5.7
40+
0x8F, //Support capabilities 1-4 inclusive
41+
0x01 //Support capability 8
42+
}, 5, 7)]
43+
//Should handle zero capabilities
44+
[InlineData(
45+
new byte[]
46+
{
47+
0x00, 0x00, 0x01, 0xFF, //Identifies this as a modern negotiation with manifest v1
48+
0x01, //1 versions to follow
49+
0x00, 0x00, 0x07, 0x05, //Supports version 5.7
50+
0x00, //no capability flags set
51+
},
52+
5,
53+
7)]
54+
[InlineData(
55+
new byte[]
56+
{
57+
0x00, 0x00, 0x01, 0xFF, //Identifies this as a modern negotiation with manifest v1
58+
0x01, //1 versions to follow
59+
0x00, 0x05, 0x07, 0x05, //Supports version 5.7 - 5.0
60+
0x00, //no capability flags set
61+
},
62+
5,
63+
7)]
64+
private async Task DoHandshakeAsyncShouldReturnBoltVersion(byte[] streamData, int majorVersion, int minorVersion)
3165
{
66+
var version = new BoltProtocolVersion(majorVersion, minorVersion);
67+
var readerStream = new MemoryStream(streamData);
3268
var socket = new Mock<ITcpSocketClient>();
3369
var writerStream = new MemoryStream();
70+
3471
socket.SetupGet(x => x.WriterStream).Returns(writerStream);
35-
var readerStream = new MemoryStream(new byte[] { 0x00, 0x00, 4, 4 });
3672
socket.SetupGet(x => x.ReaderStream).Returns(readerStream);
3773

3874
var boltProtocolVersion = await BoltHandshaker.Default.DoHandshakeAsync(
3975
socket.Object,
4076
new Mock<ILogger>().Object,
4177
CancellationToken.None);
4278

43-
boltProtocolVersion.Should().Be(new BoltProtocolVersion(4, 4));
79+
boltProtocolVersion.Should().Be(version);
4480
}
81+
82+
[Fact]
83+
private async Task DoHandshakeAsyncShouldSelectBoltVersionInRange()
84+
{
85+
var minorVersionPlus = (byte)(BoltProtocolVersion.LatestVersion.MinorVersion + 3);
86+
var majorVersion = (byte)BoltProtocolVersion.LatestVersion.MajorVersion;
87+
var inputData = new byte[]
88+
{
89+
0x00, 0x00, 0x01, 0xFF, //Identifies this as a modern negotiation with manifest v1
90+
0x02, //2 versions to follow
91+
0x00, 0x04, 0x04, 0x04,
92+
0x00, majorVersion,
93+
minorVersionPlus, majorVersion, //set to higher than actually supported
94+
0x00, //no capability flags set
95+
};
96+
97+
var readerStream = new MemoryStream(inputData);
98+
var socket = new Mock<ITcpSocketClient>();
99+
var writerStream = new MemoryStream();
100+
101+
socket.SetupGet(x => x.WriterStream).Returns(writerStream);
102+
socket.SetupGet(x => x.ReaderStream).Returns(readerStream);
103+
104+
var boltProtocolVersion = await BoltHandshaker.Default.DoHandshakeAsync(
105+
socket.Object,
106+
new Mock<ILogger>().Object,
107+
CancellationToken.None);
45108

109+
boltProtocolVersion.Should().Be(new BoltProtocolVersion(BoltProtocolVersion.LatestVersion.MajorVersion,
110+
BoltProtocolVersion.LatestVersion.MinorVersion));
111+
}
112+
46113
[Fact]
47114
public async Task DoHandshakeAsyncShouldThrowIfNotCorrectLengthResult()
48115
{
@@ -60,4 +127,39 @@ public async Task DoHandshakeAsyncShouldThrowIfNotCorrectLengthResult()
60127

61128
exception.Should().BeOfType<IOException>();
62129
}
130+
131+
[Theory]
132+
//Should throw on unrecognized manifest version
133+
[InlineData(
134+
new byte[]
135+
{
136+
0x00, 0x00, 0x02, 0xFF, //Identifies this as a modern negotiation with manifest v2 which is not known - ERROR
137+
0x01, //1 versions to follow
138+
0x00, 0x00, 0x07, 0x05, //Supports version 5.7
139+
0x00, //no capability flags set
140+
})]
141+
//Should throw on zero number protocols being supplied
142+
[InlineData(
143+
new byte[]
144+
{
145+
0x00, 0x00, 0x02, 0xFF, //Identifies this as a modern negotiation with manifest v2
146+
0x00 //0 versions to follow - ERROR
147+
})]
148+
private async Task DoHandshakeAsyncShouldThrowProtocolException(
149+
byte[] streamData)
150+
{
151+
var readerStream = new MemoryStream(streamData);
152+
var socket = new Mock<ITcpSocketClient>();
153+
var writerStream = new MemoryStream();
154+
155+
socket.SetupGet(x => x.WriterStream).Returns(writerStream);
156+
socket.SetupGet(x => x.ReaderStream).Returns(readerStream);
157+
158+
var exception = await Record.ExceptionAsync(() => BoltHandshaker.Default.DoHandshakeAsync(
159+
socket.Object,
160+
new Mock<ILogger>().Object,
161+
CancellationToken.None)).ConfigureAwait(false);
162+
163+
exception.Should().BeOfType<ProtocolException>();
164+
}
63165
}

‎Neo4j.Driver/Neo4j.Driver.Tests/Internal/Protocol/BoltProtocolVersionTests.cs‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
using System;
1717
using FluentAssertions;
18+
using Neo4j.Driver.Internal.IO;
1819
using Neo4j.Driver.Internal.Protocol;
1920
using Xunit;
2021

@@ -203,19 +204,9 @@ public void ProtocolBoundsTest(int majorVersion, int minorVersion)
203204
public void ProtocolLargeBoundsTest()
204205
{
205206
var successLargeNumber = 1213486160; ////0x‭48 54 54 50 - or HTTP in ascii codes...
206-
const int majorVersion = 80,
207-
minorVersion = 84;
208-
207+
209208
var bv = new BoltProtocolVersion(successLargeNumber);
210-
(bv.MajorVersion == majorVersion && bv.MinorVersion == minorVersion).Should().BeTrue();
211-
212-
var errorMessage =
213-
"Attempting to create a BoltProtocolVersion with a large (error code) version number. Resulting Major and Minor are in range of valid versions, which is not allowed: ";
214-
215-
var failureLargeNumber = new BoltProtocolVersion(majorVersion - 1, minorVersion - 1).PackToInt();
216-
var exception = Record.Exception(() => new BoltProtocolVersion(failureLargeNumber));
217-
exception.Should().BeOfType<NotSupportedException>();
218-
exception.Message.Should().StartWith(errorMessage);
209+
(bv.MajorVersion == 80 && bv.MinorVersion == 84).Should().BeTrue();
219210
}
220211

221212
[Theory]
@@ -271,4 +262,24 @@ public void PackToRangeSuccess(int major, int minor, int lowerMajor, int lowerMi
271262
var packedValue = version.PackToIntRange(lowerVersion);
272263
packedValue.Should().Equals(expectedValue);
273264
}
265+
266+
[Theory]
267+
[InlineData(new Byte[] { 0x00, 0x1B, 0x1B, 0x05 }, 27, 27, 5)] // Range - 6, Version - 5.27
268+
[InlineData(new Byte[] { 0x00, 0x05, 0x1A, 0x05 }, 5, 26, 5)] // Range - 5, Version - 5.26
269+
public void UnpackVersionAndRangeSuccess(
270+
Byte[] sourcePackedInt,
271+
int range,
272+
int minorVersion,
273+
int majorVersion)
274+
{
275+
var packedInt = new Byte[4];
276+
sourcePackedInt.CopyTo(packedInt, 0); //Do a copy here because it turns out that ToInt32 (and all the other conversion methods change the supplied byte array as it's a ref type...maybebug?)
277+
var protocolVersion = BoltProtocolVersion.FromPackedInt(PackStreamBitConverter.ToInt32(packedInt));
278+
sourcePackedInt.CopyTo(packedInt, 0); //Do a copy here because it turns out that ToInt32 (and all the other conversion methods change the supplied byte array as it's a ref type...maybebug?)
279+
var rangeValue = BoltProtocolVersion.RangeFromPackedInt(PackStreamBitConverter.ToInt32(packedInt));
280+
281+
protocolVersion.MajorVersion.Should().Be(majorVersion);
282+
protocolVersion.MinorVersion.Should().Be(minorVersion);
283+
rangeValue.Should().Be(range);
284+
}
274285
}

‎Neo4j.Driver/Neo4j.Driver.Tests/Internal/Util/VarLongTests.cs‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
// Copyright (c) "Neo4j"
2+
// Neo4j Sweden AB [https://neo4j.com]
3+
//
4+
// Licensed under the Apache License, Version 2.0 (the "License").
5+
// You may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
16+
using System;
17+
using FluentAssertions;
18+
using Neo4j.Driver.Internal.IO;
19+
using Neo4j.Driver.Internal.Util;
20+
using Xunit;
21+
22+
namespace Neo4j.Driver.Tests.Internal.Util;
23+
24+
public class VarLongTests
25+
{
26+
public class AddSegmentMethod
27+
{
28+
[Fact]
29+
public void ShouldAddNewSegment()
30+
{
31+
var varLong = new VarLong();
32+
const byte newSegment = 0x1;
33+
varLong.AddSegment(newSegment);
34+
35+
varLong.Value.Should().Be(newSegment);
36+
}
37+
38+
[Theory]
39+
[InlineData(new byte[] { 0x8F, 0x01 }, 143)] // = 0000001 0001111
40+
[InlineData(new byte[] { 0xFF, 0x01 }, 255)] // = 0000001 1111111
41+
[InlineData(new byte[] { 0xFF, 0xFF, 0x01 }, 32767)] // = 0000001 1111111 1111111
42+
[InlineData(new byte[] { 0x81, 0x81, 0x01 }, 16513)] // = 0000001 0000001 0000001
43+
[InlineData(new byte[] { 0x8F, 0x8F, 0x04 }, 67471)] // = 0000100 0001111 0001111
44+
public void ShouldAddMultipleNewSegments(byte[] data, long finalValue)
45+
{
46+
var varLong = new VarLong();
47+
48+
foreach (var element in data)
49+
{
50+
varLong.AddSegment(element);
51+
}
52+
53+
varLong.Value.Should().Be(finalValue);
54+
}
55+
56+
[Fact]
57+
public void ShouldSegmentFaultOnTooManyAdds()
58+
{
59+
var varLong = new VarLong();
60+
const byte newSegment = 0x1;
61+
62+
var exception = Record.Exception(
63+
() =>
64+
{
65+
for (var i = 0; i < 9; i++)
66+
{
67+
varLong.AddSegment(newSegment);
68+
}
69+
});
70+
71+
exception.Should().BeOfType<ArgumentException>().Which.Message.Should().Be("VarLong Segment overflow");
72+
}
73+
}
74+
}

0 commit comments

Comments
 (0)