Fix for DebugAssertException detection during testing + VIntUtils length assertions (#1400)

Author: NightOwl888

src/Lucene.Net.TestFramework/Support/Util/LuceneTestFrameworkInitializer.cs

@@ -3,7 +3,6 @@
 using NUnit.Framework;
 using System;
 using System.Diagnostics.CodeAnalysis;
-using System.IO;
 
 namespace Lucene.Net.Util
 {
@@ -358,11 +357,13 @@ internal void InitializeStaticState()
             // Identify the Debug.Assert() exception so it can be excluded from being swallowed by catch blocks.
             // These types are internal, so we can identify them using Reflection.
             Lucene.ExceptionExtensions.DebugAssertExceptionType =
+                // Microsoft.NET.Test.Sdk 16.6.0+ (used by Visual Studio Test Explorer)
+                Type.GetType("Microsoft.VisualStudio.TestPlatform.TestHost.DebugAssertException, testhost")
                 // .NET 5/.NET Core 3.x
-                Type.GetType("System.Diagnostics.DebugProvider+DebugAssertException, System.Private.CoreLib")
+                ?? Type.GetType("System.Diagnostics.DebugProvider+DebugAssertException, System.Private.CoreLib")
                 // .NET Core 2.x
                 ?? Type.GetType("System.Diagnostics.Debug+DebugAssertException, System.Private.CoreLib");
-                // .NET Framework doesn't throw in this case
+            // .NET Framework doesn't throw in this case
         }
 
         /// <summary>

src/Lucene.Net.Tests.AllProjects/Support/AssemblyScanningTestCase.cs

@@ -59,10 +59,23 @@ public abstract class AssemblyScanningTestCase : LuceneTestCase
         };
 
 
-        public static readonly Assembly[] DotNetAssemblies = new Assembly[]
+        public static readonly Assembly[] DotNetAssemblies = LoadDotNetAssemblies();
+
+        private static Assembly[] LoadDotNetAssemblies()
         {
-            typeof(Exception).Assembly
-        };
+            var list = new List<Assembly>()
+            {
+                typeof(Exception).Assembly
+            };
+
+            Assembly testHostAssembly = Type.GetType("Microsoft.VisualStudio.TestPlatform.TestHost.DebugAssertException, testhost")?.Assembly;
+            if (testHostAssembly is not null)
+            {
+                list.Add(testHostAssembly);
+            }
+
+            return list.ToArray();
+        }
 
         public static readonly Assembly[] NUnitAssemblies = new Assembly[]
         {

src/Lucene.Net.Tests.AllProjects/Support/ExceptionHandling/ExceptionScanningTestCase.cs

@@ -39,11 +39,21 @@ namespace Lucene.Net.Support.ExceptionHandling
     public abstract class ExceptionScanningTestCase : AssemblyScanningTestCase
     {
         // Internal types references
-        private static readonly Type DebugAssertExceptionType =
+        private static readonly Type ClrDebugAssertExceptionType =
             // .NET 5/.NET Core 3.x
             Type.GetType("System.Diagnostics.DebugProvider+DebugAssertException, System.Private.CoreLib")
             // .NET Core 2.x
             ?? Type.GetType("System.Diagnostics.Debug+DebugAssertException, System.Private.CoreLib");
+
+        private static readonly Type TestHostDebugAssertExceptionType =
+            // Microsoft.NET.Test.Sdk 16.6.0+ (used by Visual Studio Test Explorer)
+            Type.GetType("Microsoft.VisualStudio.TestPlatform.TestHost.DebugAssertException, testhost");
+
+        private static readonly Type DebugAssertExceptionType =
+            // Microsoft.NET.Test.Sdk 16.6.0+ (used by Visual Studio Test Explorer)
+            TestHostDebugAssertExceptionType
+            // .NET Core
+            ?? ClrDebugAssertExceptionType;
         // .NET Framework doesn't throw in this case
 
         private static readonly Type MetadataExceptionType =
@@ -465,15 +475,28 @@ private static IDictionary<Type, Func<Type, string, object>> LoadNonStandardExce
             };
 
             // Special case - this doesn't exist on .NET Framework, so we only add it if not null
-            if (DebugAssertExceptionType is not null)
+            if (ClrDebugAssertExceptionType is not null)
             {
-                result[DebugAssertExceptionType] = (exceptionType, message) =>
+                result[ClrDebugAssertExceptionType] = (exceptionType, message) =>
                 {
                     //private sealed class DebugAssertException : Exception
                     //{
                     //    internal DebugAssertException(string? message, string? detailMessage, string? stackTrace)
                     BindingFlags flags = BindingFlags.NonPublic | BindingFlags.Instance;
-                    return Activator.CreateInstance(DebugAssertExceptionType, flags, null, new object[] { message, null, null }, CultureInfo.InvariantCulture);
+                    return Activator.CreateInstance(ClrDebugAssertExceptionType, flags, null, new object[] { message, null, null }, CultureInfo.InvariantCulture);
+                };
+            }
+
+            // Special case - Microsoft.NET.Test.Sdk 16.6.0+ contains its own Debug.Assert type that is thrown when debugging tests
+            if (TestHostDebugAssertExceptionType is not null)
+            {
+                result[TestHostDebugAssertExceptionType] = (exceptionType, message) =>
+                {
+                    //internal sealed class DebugAssertException : Exception
+                    //{
+                    //public DebugAssertException(string message, string stackTrace) : base(message)
+                    BindingFlags flags = BindingFlags.NonPublic | BindingFlags.Instance;
+                    return Activator.CreateInstance(TestHostDebugAssertExceptionType, flags, null, new object[] { message, string.Empty }, CultureInfo.InvariantCulture);
                 };
             }
 

src/Lucene.Net.Tests.AllProjects/Support/ExceptionHandling/TestExceptionExtensions.cs

@@ -381,6 +381,29 @@ public void TestIsIllegalArgumentException_TestEnvironment(Type exceptionType, b
             }
         }
 
+#if DEBUG
+        /// <summary>
+        /// Tests to ensure that the DebugAssertException type string is correct for the current test environment.
+        /// If this test fails, it means that the DebugAssertException has changed location and the
+        /// <see cref="LuceneTestFrameworkInitializer.InitializeStaticState()"/> method needs to be updated with
+        /// the new type string.
+        /// <para/>
+        /// Microsoft controls this location, it is internal, and changes from time to time. Currently, it is in
+        /// Microsoft.NET.Test.Sdk as of version 16.6.0.
+        /// <para/>
+        /// Note that this exception only applies when the test SDK is attached. If the test SDK is not attached,
+        /// then this exception falls back to a different exception type in the current .NET SDK. This can happen
+        /// in DEBUG mode when running as a console application, for example.
+        /// </summary>
+        [Test]
+        public void TestDebugAssertExceptionTypeString()
+        {
+            Assert.IsNotNull(
+                Type.GetType("Microsoft.VisualStudio.TestPlatform.TestHost.DebugAssertException, testhost"),
+                "DebugAssertException type not found for the current test enviornment. This means that the DebugAssertException has changed location and the LuceneTestFrameworkInitializer.InitializeStaticState() method needs to be updated with the new type string.");
+        }
+#endif
+
         private class MyException : Exception
         {
             public MyException(string message) : base(message)

src/Lucene.Net.Tests/Support/TestVIntUtils.cs

@@ -171,5 +171,93 @@ public void TestMaxLengths()
             assertEquals(VIntUtils.MaxVInt32Length, EncodeVInt32(-1).Length);
             assertEquals(VIntUtils.MaxVInt64Length, EncodeVInt64(long.MaxValue).Length);
         }
+
+#if DEBUG
+
+        [TestCase(0)]
+        [TestCase(1)]
+        [TestCase(127)]
+        [TestCase(128)]
+        [TestCase(16383)]
+        [TestCase(16384)]
+        [TestCase(2097151)]
+        [TestCase(2097152)]
+        [TestCase(268435455)]
+        [TestCase(int.MaxValue)]
+        [TestCase(-1)]
+        [LuceneNetSpecific]
+        public void TestReadVInt32_DebugAssertRequiresCompleteSpan(int value)
+        {
+            byte[] encoded = EncodeVInt32(value);
+
+            // Every prefix shorter than the encoded value should assert.
+            for (int length = 0; length < encoded.Length; length++)
+            {
+                Assert.Throws(Lucene.ExceptionExtensions.DebugAssertExceptionType, () =>
+                {
+                    VIntUtils.TryReadVInt32(encoded.AsSpan(0, length), out _, out _);
+                }, $"Expected {Lucene.ExceptionExtensions.DebugAssertExceptionType.AssemblyQualifiedName} for value {value} with span length {length}.");
+            }
+
+            // The exact encoded length should succeed.
+            {
+                Assert.DoesNotThrow(() =>
+                {
+                    bool ok = VIntUtils.TryReadVInt32(encoded, out int result, out int count);
+
+                    assertTrue(ok);
+                    assertEquals(value, result);
+                    assertEquals(encoded.Length, count);
+                });
+            }
+        }
+
+        [TestCase(0L)]
+        [TestCase(1L)]
+        [TestCase(127L)]
+        [TestCase(128L)]
+        [TestCase(16383L)]
+        [TestCase(16384L)]
+        [TestCase(2097151L)]
+        [TestCase(2097152L)]
+        [TestCase(268435455L)]
+        [TestCase(268435456L)]
+        [TestCase(34359738367L)]
+        [TestCase(34359738368L)]
+        [TestCase(4398046511103L)]
+        [TestCase(4398046511104L)]
+        [TestCase(562949953421311L)]
+        [TestCase(562949953421312L)]
+        [TestCase(72057594037927935L)]
+        [TestCase(72057594037927936L)]
+        [TestCase(long.MaxValue)]
+        [LuceneNetSpecific]
+        public void TestReadVInt64_DebugAssertRequiresCompleteSpan(long value)
+        {
+            byte[] encoded = EncodeVInt64(value);
+
+            // Every prefix shorter than the encoded value should assert.
+            for (int length = 0; length < encoded.Length; length++)
+            {
+                Assert.Throws(Lucene.ExceptionExtensions.DebugAssertExceptionType, () =>
+                {
+                    VIntUtils.TryReadVInt64(encoded.AsSpan(0, length), out _, out _);
+                }, $"Expected {Lucene.ExceptionExtensions.DebugAssertExceptionType.AssemblyQualifiedName} for value {value} with span length {length}.");
+            }
+
+            // The exact encoded length should succeed.
+            {
+                Assert.DoesNotThrow(() =>
+                {
+                    bool ok = VIntUtils.TryReadVInt64(encoded, out long result, out int count);
+
+                    assertTrue(ok);
+                    assertEquals(value, result);
+                    assertEquals(encoded.Length, count);
+                });
+            }
+        }
+
+#endif
     }
 }

src/Lucene.Net/Support/VIntUtils.cs

@@ -55,7 +55,7 @@ internal static class VIntUtils
         /// extra high bits set.</returns>
         public static bool TryReadVInt32(ReadOnlySpan<byte> source, out int value, out int count)
         {
-            Debug.Assert(source.Length >= 1);
+            AssertCanContainVInt32(source);
             byte b = source[0];
             if (b <= sbyte.MaxValue) // LUCENENET: Optimized equivalent of "if ((sbyte)b >= 0)"
             {
@@ -109,7 +109,7 @@ public static bool TryReadVInt32(ReadOnlySpan<byte> source, out int value, out i
         /// the continuation bit set (which would indicate a negative value, disallowed).</returns>
         public static bool TryReadVInt64(ReadOnlySpan<byte> source, out long value, out int count)
         {
-            Debug.Assert(source.Length >= 1);
+            AssertCanContainVInt64(source);
             byte b = source[0];
             if (b <= sbyte.MaxValue)
             {
@@ -181,6 +181,42 @@ public static bool TryReadVInt64(ReadOnlySpan<byte> source, out long value, out
             return b <= sbyte.MaxValue;
         }
 
+        [Conditional("DEBUG")]
+        private static void AssertCanContainVInt32(ReadOnlySpan<byte> source)
+        {
+            int length = source.Length;
+            if (length >= MaxVInt32Length) return;
+
+            int limit = Math.Min(length, MaxVInt32Length);
+
+            for (int i = 0; i < limit; i++)
+            {
+                if ((source[i] & 0x80) == 0)
+                    return;
+            }
+
+            Debug.Assert(length >= MaxVInt32Length,
+                "The span is too short to contain a complete VInt32.");
+        }
+
+        [Conditional("DEBUG")]
+        private static void AssertCanContainVInt64(ReadOnlySpan<byte> source)
+        {
+            int length = source.Length;
+            if (length >= MaxVInt64Length) return;
+
+            int limit = Math.Min(length, MaxVInt64Length);
+
+            for (int i = 0; i < limit; i++)
+            {
+                if ((source[i] & 0x80) == 0)
+                    return;
+            }
+
+            Debug.Assert(length >= MaxVInt64Length,
+                "The span is too short to contain a complete VInt64.");
+        }
+
         // LUCENENET: The throw sites below are factored into dedicated, non-inlined helpers so the
         // hot Read*VInt* overrides that call them stay small enough for the JIT to inline. Keeping
         // the `throw` + message string out of the caller body avoids bloating its IL (the inliner