Skip to content

Commit cf328d1

Browse files
paulmedynskipaulmedynski
committed
User Story 33291: Enhance client interface value sent with login
- Removed length sharing logic for OS and Runtime Info. - Replaced with simple max len of 44 for each field.
1 parent 643dcdd commit cf328d1

File tree

2 files changed

+97
-188
lines changed

2 files changed

+97
-188
lines changed

‎src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ClientInterface.cs‎

Lines changed: 13 additions & 136 deletions
Original file line numberDiff line numberDiff line change
@@ -65,21 +65,23 @@ internal static class ClientInterface
6565
/// ProcessArchitecture
6666
/// </see>
6767
/// for possible values. This value will never be longer than 10
68-
/// characters.
68+
/// Unicode characters.
6969
/// </para>
7070
/// <para>
7171
/// The <c>{OS Info}</c> part will be sourced from the the
7272
/// <see cref="RuntimeInformation.OSDescription">
7373
/// OSDescription
7474
/// </see>
7575
/// value, or "Unknown" if that value is empty or all whitespace.
76+
/// This value will never be longer than 44 Unicode characters.
7677
/// </para>
7778
/// <para>
7879
/// The <c>{Runtime Info}</c> part will be sourced from the
7980
/// <see cref="RuntimeInformation.FrameworkDescription">
8081
/// FrameworkDescription
8182
/// </see>
8283
/// value, or "Unknown" if that value is empty or all whitespace.
84+
/// This value will never be longer than 44 Unicode characters.
8385
/// </para>
8486
/// <para>
8587
/// This adheres to the TDS v37.0 spec, which specifies that the
@@ -89,19 +91,6 @@ internal static class ClientInterface
8991
/// parts or pipe ('|') delimiters.
9092
/// </para>
9193
/// <para>
92-
/// The maximum length is expected to be sufficient to accommodate
93-
/// the driver name, <c>{OS Type}</c>, and <c>{Arch}</c> parts, but
94-
/// those parts will be truncated as described above if necessary.
95-
/// </para>
96-
/// <para>
97-
/// The <c>{OS Info}</c> and <c>{Runtime Info}</c> parts will share
98-
/// any remaining space as evenly as possible, being truncated
99-
/// equally if both are longer than half of the remaining space.
100-
/// If one of these parts is shorter than half of the remaining
101-
/// space, the other part will consume as much remaining space as
102-
/// possible.
103-
/// </para>
104-
/// <para>
10594
/// Any characters that are not one of the following are replaced
10695
/// with underscore ('_'):
10796
/// <list type="bullet">
@@ -234,133 +223,19 @@ public static string Build(
234223
name.Append('|');
235224

236225
// Add the OS Type, truncating to its max length.
237-
name.Append(Truncate(Clean(osType), MaxLenOSType));
226+
name.Append(Truncate(Clean(osType), MaxLenOsType));
238227
name.Append('|');
239228

240229
// Add the Architecture, truncating to its max length.
241230
name.Append(Truncate(Clean(arch.ToString()), MaxLenArch));
242231
name.Append('|');
243232

244-
// String.Length is a signed 32-bit integer, but the API
245-
// guarantees it will never be negative. We will not explicitly
246-
// check for negative values during arithmetic operations.
247-
248-
// We should have appended at most 39 characters so far:
249-
//
250-
// 16 (driver name)
251-
// 1 (pipe)
252-
// 10 (OS name)
253-
// 1 (pipe)
254-
// 10 (architecture)
255-
// 1 (pipe)
256-
//
257-
// This leaves us with at least 89 characters for the OS and
258-
// Runtime Info.
259-
//
260-
Debug.Assert(name.Length <= 39);
261-
262-
// Obtain cleaned versions of OS and Runtime Info.
263-
osInfo = Clean(osInfo);
264-
runtimeInfo = Clean(runtimeInfo);
265-
266-
// How many more characters can we append?
267-
ushort remaining = 0;
268-
if (name.Length < maxLen)
269-
{
270-
remaining = (ushort)(maxLen - name.Length);
271-
}
272-
273-
// Do we have any remaining space?
274-
if (remaining > 0)
275-
{
276-
// Yes, so we want to end up with OS and Runtime Info
277-
// lengths like this:
278-
//
279-
// Remaining | OS | Pipe | Runtime
280-
// ----------|----|------| -------
281-
// 1 | 1 | 0 | 0
282-
// 2 | 1 | 1 | 0
283-
// 3 | 1 | 1 | 1
284-
// 4 | 2 | 1 | 1
285-
// 5 | 2 | 1 | 2
286-
// 6 | 3 | 1 | 2
287-
// 7 | 3 | 1 | 3
288-
//
289-
// And so on.
290-
//
291-
// If remaining is odd, we'll give the extra character
292-
// to the OS Info. Runtime Info is likely to have suitable
293-
// fidelity within its first 45 characters.
294-
295-
// If we have at least 2 characters left, then we will need
296-
// to leave room for the pipe character, so decrement
297-
// remaining accordingly.
298-
if (remaining >= 2)
299-
{
300-
--remaining;
301-
}
302-
303-
// Will both Info parts together be too long?
304-
if (
305-
// If the addition of both lengths would overflow, then
306-
// they are definitely too long.
307-
int.MaxValue - osInfo.Length < runtimeInfo.Length
308-
// Otherwise, check their sum versus remaining.
309-
|| osInfo.Length + runtimeInfo.Length > remaining)
310-
{
311-
// Yes, so we will have to truncate something.
312-
//
313-
// We want to keep the Info as balanced as possible, so
314-
// we'll truncate them each to no shorter than half of
315-
// the remaining space.
316-
//
317-
ushort osHalf = (ushort)(remaining / 2);
318-
ushort runtimeHalf = osHalf;
319-
320-
// If there's a remainder, give it to the OS Info.
321-
if (osHalf + runtimeHalf < remaining)
322-
{
323-
++osHalf;
324-
}
325-
326-
Debug.Assert(osHalf + runtimeHalf == remaining);
327-
328-
// Will the OS Info fit as-is?
329-
if (osInfo.Length <= osHalf)
330-
{
331-
// Yes, so the Runtime Info must be too long.
332-
// Truncate it as little as possible.
333-
runtimeInfo =
334-
runtimeInfo.Substring(
335-
0,
336-
remaining - osInfo.Length);
337-
}
338-
// Will the Runtime Info fit as-is?
339-
else if (runtimeInfo.Length <= runtimeHalf)
340-
{
341-
// Yes, so the OS Info must be too long. Truncate
342-
// it as little as possible.
343-
osInfo =
344-
osInfo.Substring(
345-
0,
346-
remaining - runtimeInfo.Length);
347-
}
348-
// Otherwise, we need to truncate them both.
349-
else
350-
{
351-
osInfo = osInfo.Substring(0, osHalf);
352-
runtimeInfo = runtimeInfo.Substring(0, runtimeHalf);
353-
}
354-
355-
Debug.Assert(
356-
osInfo.Length + runtimeInfo.Length <= remaining);
357-
}
358-
359-
// Append them now that they've been truncated if necessary.
360-
name.Append(osInfo);
361-
name.Append('|');
362-
name.Append(runtimeInfo);
363-
}
233+
// Add the OS Info, truncating to its max length.
234+
name.Append(Truncate(Clean(osInfo), MaxLenOsInfo));
235+
name.Append('|');
236+
237+
// Add the Runtime Info, truncating to its max length.
238+
name.Append(Truncate(Clean(runtimeInfo), MaxLenRuntimeInfo));
364239

365240
// Remember the name we've built up.
366241
result = name.ToString();
@@ -530,8 +405,10 @@ public static string Truncate(string value, ushort maxLength)
530405

531406
// Maximum part lengths as promised in our API.
532407
private const ushort MaxLenDriverName = 16;
533-
private const ushort MaxLenOSType = 10;
408+
private const ushort MaxLenOsType = 10;
534409
private const ushort MaxLenArch = 10;
410+
private const ushort MaxLenOsInfo = 44;
411+
private const ushort MaxLenRuntimeInfo = 44;
535412

536413
// The OS Type values we promise in our API.
537414
private const string Windows = "Windows";

‎src/Microsoft.Data.SqlClient/tests/FunctionalTests/ClientInterfaceTests.cs‎

Lines changed: 84 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -152,10 +152,13 @@ public void Name()
152152
Assert.True(parts[2] == "Unknown" || parts[2].Length > 0);
153153
Assert.True(parts[2].Length <= 10);
154154

155-
// The OS and Runtime Info have no guaranteed format/content, but
156-
// they must both be "Unknown" or non-empty.
155+
// OS Info must be non-empty and 44 characters or less.
157156
Assert.True(parts[3] == "Unknown" || parts[3].Length > 0);
157+
Assert.True(parts[3].Length <= 44);
158+
159+
// Runtime Info must be non-empty and 44 characters or less.
158160
Assert.True(parts[4] == "Unknown" || parts[4].Length > 0);
161+
Assert.True(parts[4].Length <= 44);
159162
}
160163

161164
// Test the Build() function when it truncates the overall length.
@@ -235,69 +238,98 @@ public void Build_Truncate_Arch()
235238
#endif // NET
236239
}
237240

238-
// Test the Build() function when one or both of OS and/or Runtime Info
239-
// are truncated.
241+
// Test the Build() function when it truncates the OS Info.
240242
[Fact]
241-
public void Build_Truncate_OS_Runtime_Info()
243+
public void Build_Truncate_OS_Info()
242244
{
243-
// There is no space remaining.
244-
Assert.Equal(
245-
"A|B|X64|",
246-
DoBuild(8, "A", "B", Architecture.X64, "C", "D"));
247-
248-
// There is 1 char remaining, which is given to the OS Info.
245+
// The OS Info puts the overall length over the max.
249246
Assert.Equal(
250-
"A|B|X64|C",
251-
DoBuild(9, "A", "B", Architecture.X64, "CCC", "DDD"));
247+
"A|B|X64|LongOsI",
248+
DoBuild(15, "A", "B", Architecture.X64, "LongOsInfo", "D"));
252249

253-
// There are 2 chars remaining; 1 for each Info field, but the pipe
254-
// character gets in the way and Runtime Info is truncated.
250+
// The OS Type is longer than its per-field max length of 44.
255251
Assert.Equal(
256-
"A|B|X64|C|",
257-
DoBuild(10, "A", "B", Architecture.X64, "CCC", "DDD"));
258-
259-
// There are 3 chars remaining; 1 for each Info field, and 1 for
260-
// the pipe.
252+
"A|B|X64|01234567890123456789012345678901234567890123|D",
253+
DoBuild(
254+
128, "A", "B", Architecture.X64,
255+
"01234567890123456789012345678901234567890123456789",
256+
"D"));
257+
}
258+
259+
// Test the Build() function when it truncates the Runtime Info.
260+
[Fact]
261+
public void Build_Truncate_Runtime_Info()
262+
{
263+
// The Runtime Info puts the overall length over the max.
261264
Assert.Equal(
262-
"A|B|X64|C|D",
263-
DoBuild(11, "A", "B", Architecture.X64, "CCC", "DDD"));
265+
"A|B|X64|C|LongRunt",
266+
DoBuild(18, "A", "B", Architecture.X64, "C",
267+
"LongRuntimeInfo"));
264268

265-
// Continue to expand max length until both Info values fit.
266-
Assert.Equal(
267-
"A|B|X64|CC|D",
268-
DoBuild(12, "A", "B", Architecture.X64, "CCC", "DDD"));
269+
// The Runtime Type is longer than its per-field max length of 44.
269270
Assert.Equal(
270-
"A|B|X64|CC|DD",
271-
DoBuild(13, "A", "B", Architecture.X64, "CCC", "DDD"));
272-
Assert.Equal(
273-
"A|B|X64|CCC|DD",
274-
DoBuild(14, "A", "B", Architecture.X64, "CCC", "DDD"));
275-
Assert.Equal(
276-
"A|B|X64|CCC|DDD",
277-
DoBuild(15, "A", "B", Architecture.X64, "CCC", "DDD"));
271+
"A|B|X64|C|01234567890123456789012345678901234567890123",
272+
DoBuild(
273+
128, "A", "B", Architecture.X64, "C",
274+
"01234567890123456789012345678901234567890123456789"));
275+
}
278276

279-
// OS Info is shorter than half, so Runtime Info gets the rest.
280-
Assert.Equal(
281-
"A|B|X64|CC|DDD",
282-
DoBuild(14, "A", "B", Architecture.X64, "CC", "DDDD"));
283-
Assert.Equal(
284-
"A|B|X64|CC|DDDD",
285-
DoBuild(15, "A", "B", Architecture.X64, "CC", "DDDD"));
277+
// Test the Build() function when most of the fields are truncated.
278+
[Fact]
279+
public void Build_Truncate_Most()
280+
{
281+
var name =
282+
DoBuild(
283+
128,
284+
// Driver name > 16 chars.
285+
"A01234567890123456789",
286+
// OS Type > 10 chars.
287+
"B01234567890123456789",
288+
// Architecture isn't truncated (because .NET Framework
289+
// doesn't have any enum values long enough).
290+
Architecture.X64,
291+
// OS Info > 44 chars.
292+
"C01234567890123456789012345678901234567890123456789",
293+
// Runtime Info > 44 chars.
294+
"D01234567890123456789012345678901234567890123456789");
295+
Assert.Equal(121, name.Length);
286296
Assert.Equal(
287-
"A|B|X64|CC|DDDD",
288-
DoBuild(16, "A", "B", Architecture.X64, "CC", "DDDD"));
297+
"A012345678901234|" +
298+
"B012345678|" +
299+
"X64|" +
300+
"C0123456789012345678901234567890123456789012|" +
301+
"D0123456789012345678901234567890123456789012",
302+
name);
303+
}
289304

290-
// Runtime Info is shorter than half, so OS Info gets the rest.
291-
Assert.Equal(
292-
"A|B|X64|CCC|DD",
293-
DoBuild(14, "A", "B", Architecture.X64, "CCCC", "DD"));
294-
Assert.Equal(
295-
"A|B|X64|CCCC|DD",
296-
DoBuild(15, "A", "B", Architecture.X64, "CCCC", "DD"));
305+
#if NET
306+
// Test the Build() function when all the fields are truncated.
307+
[Fact]
308+
public void Build_Truncate_All()
309+
{
310+
var name =
311+
DoBuild(
312+
128,
313+
// Driver name > 16 chars.
314+
"A01234567890123456789",
315+
// OS Type > 10 chars.
316+
"B01234567890123456789",
317+
// Architecture > 10 chars.
318+
Architecture.LoongArch64,
319+
// OS Info > 44 chars.
320+
"C01234567890123456789012345678901234567890123456789",
321+
// Runtime Info > 44 chars.
322+
"D01234567890123456789012345678901234567890123456789");
323+
Assert.Equal(128, name.Length);
297324
Assert.Equal(
298-
"A|B|X64|CCCC|DD",
299-
DoBuild(16, "A", "B", Architecture.X64, "CCCC", "DD"));
325+
"A012345678901234|" +
326+
"B012345678|" +
327+
"LoongArch6|" +
328+
"C0123456789012345678901234567890123456789012|" +
329+
"D0123456789012345678901234567890123456789012",
330+
name);
300331
}
332+
#endif // NET
301333

302334
// Test the Clean() function.
303335
[Fact]

0 commit comments

Comments
 (0)