Fix string length detection

- Unicode string length detection only checked one byte instead of both - UTF32 string length detection only checked one byte instead of all four - Added Unit tests to test for special chars which would provoke wrong string length detection
badcel · Oct 9, 2022 · 86c0d9f · 86c0d9f
1 parent dde2b2e
commit 86c0d9f
Show file tree

Hide file tree

Showing 10 changed files with 111 additions and 35 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -49,6 +49,10 @@ jobs:
         run: dotnet format --no-restore --verify-no-changes
         working-directory: './src'
 
+      - name: Run unit tests
+        run: dotnet test --no-restore -c ${{ env.configuration }}
+        working-directory: './src'
+
       - name: Test Linux 64 bit
         if: ${{ matrix.os == 'ubuntu-22.04' }}
         run: |

diff --git a/src/Directory.Build.props b/src/Directory.Build.props
@@ -0,0 +1,8 @@
+<Project>
+  <PropertyGroup>
+    <TargetFramework>net6.0</TargetFramework>
+    <ImplicitUsings>enable</ImplicitUsings>
+    <Nullable>enable</Nullable>
+    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
+  </PropertyGroup>
+</Project>
diff --git a/src/HidApi.Net.Benchmark/HidApi.Net.Benchmark.csproj b/src/HidApi.Net.Benchmark/HidApi.Net.Benchmark.csproj
@@ -1,16 +1,11 @@
 <Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+    <IsPackable>false</IsPackable>
+  </PropertyGroup>
 
-    <PropertyGroup>
-        <OutputType>Exe</OutputType>
-        <TargetFramework>net6.0</TargetFramework>
-        <ImplicitUsings>enable</ImplicitUsings>
-        <Nullable>enable</Nullable>
-        <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
-        <IsPackable>false</IsPackable>
-    </PropertyGroup>
-
-    <ItemGroup>
-        <PackageReference Include="BenchmarkDotNet" Version="0.13.2" />
-    </ItemGroup>
-
+  <ItemGroup>
+    <PackageReference Include="BenchmarkDotNet" Version="0.13.2"/>
+  </ItemGroup>
 </Project>
diff --git a/src/HidApi.Net.Tester/HidApi.Net.Tester.csproj b/src/HidApi.Net.Tester/HidApi.Net.Tester.csproj
@@ -1,16 +1,12 @@
 <Project Sdk="Microsoft.NET.Sdk">
-
   <PropertyGroup>
     <OutputType>Exe</OutputType>
-    <TargetFramework>net6.0</TargetFramework>
-    <ImplicitUsings>enable</ImplicitUsings>
-    <Nullable>enable</Nullable>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <IsPackable>false</IsPackable>
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="System.CommandLine" Version="2.0.0-beta4.22272.1" />
-    <ProjectReference Include="..\HidApi.Net\HidApi.Net.csproj" />
+    <PackageReference Include="System.CommandLine" Version="2.0.0-beta4.22272.1"/>
+    <ProjectReference Include="..\HidApi.Net\HidApi.Net.csproj"/>
   </ItemGroup>
 </Project>
diff --git a/src/HidApi.Net.Tests/HidApi.Net.Tests.csproj b/src/HidApi.Net.Tests/HidApi.Net.Tests.csproj
@@ -0,0 +1,17 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <IsPackable>false</IsPackable>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="FluentAssertions" Version="6.7.0" />
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.3.2" />
+    <PackageReference Include="MSTest.TestAdapter" Version="2.2.10" />
+    <PackageReference Include="MSTest.TestFramework" Version="2.2.10" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="..\HidApi.Net\HidApi.Net.csproj" />
+  </ItemGroup>
+</Project>
diff --git a/src/HidApi.Net.Tests/StringTests.cs b/src/HidApi.Net.Tests/StringTests.cs
@@ -0,0 +1,56 @@
+using FluentAssertions;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+namespace HidApi.Tests;
+
+[TestClass]
+public class StringTests
+{
+    [TestMethod]
+    public void TestUnicode()
+    {
+        var data = new byte[6];
+        data[0] = 0;
+        data[1] = 1;
+        data[2] = 0;
+        data[3] = 1;
+        data[4] = 0;
+        data[5] = 0;
+
+        unsafe
+        {
+            fixed (byte* p = data)
+            {
+                var str = Unicode.Read(p);
+                str.Should().Be(new string(new[] { '\u0100', '\u0100' }));
+            }
+        }
+    }
+
+    [TestMethod]
+    public void TestUtf32()
+    {
+        var data = new byte[12];
+        data[0] = 0;
+        data[1] = 1;
+        data[2] = 0;
+        data[3] = 0;
+        data[4] = 0;
+        data[5] = 1;
+        data[6] = 0;
+        data[7] = 0;
+        data[8] = 0;
+        data[9] = 0;
+        data[10] = 0;
+        data[11] = 0;
+
+        unsafe
+        {
+            fixed (byte* p = data)
+            {
+                var str = Utf32.Read(p);
+                str.Should().Be(new string(new[] { '\u0100', '\u0100' }));
+            }
+        }
+    }
+}
diff --git a/src/HidApi.Net.sln b/src/HidApi.Net.sln
@@ -9,12 +9,15 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "SolutionItems", "SolutionIt
 		..\.editorconfig = ..\.editorconfig
 		..\.github\workflows\ci.yml = ..\.github\workflows\ci.yml
 		..\.github\dependabot.yml = ..\.github\dependabot.yml
+		Directory.Build.props = Directory.Build.props
 	EndProjectSection
 EndProject
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "HidApi.Net.Benchmark", "HidApi.Net.Benchmark\HidApi.Net.Benchmark.csproj", "{0AFE95CC-90A8-490A-8FE5-2604A1BB13CE}"
 EndProject
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "HidApi.Net.Tester", "HidApi.Net.Tester\HidApi.Net.Tester.csproj", "{D7FE837A-D24D-4D56-B538-CCC6968DDF1E}"
 EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "HidApi.Net.Tests", "HidApi.Net.Tests\HidApi.Net.Tests.csproj", "{3F7CF192-630B-4981-963A-6BA97F8C65EA}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -33,5 +36,9 @@ Global
 		{D7FE837A-D24D-4D56-B538-CCC6968DDF1E}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{D7FE837A-D24D-4D56-B538-CCC6968DDF1E}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{D7FE837A-D24D-4D56-B538-CCC6968DDF1E}.Release|Any CPU.Build.0 = Release|Any CPU
+		{3F7CF192-630B-4981-963A-6BA97F8C65EA}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{3F7CF192-630B-4981-963A-6BA97F8C65EA}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{3F7CF192-630B-4981-963A-6BA97F8C65EA}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{3F7CF192-630B-4981-963A-6BA97F8C65EA}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 EndGlobal
diff --git a/src/HidApi.Net/HidApi.Net.csproj b/src/HidApi.Net/HidApi.Net.csproj
@@ -2,11 +2,7 @@
   <PropertyGroup>
     <VersionPrefix>0.1.1</VersionPrefix>
 
-    <TargetFramework>net6.0</TargetFramework>
-    <ImplicitUsings>enable</ImplicitUsings>
-    <Nullable>enable</Nullable>
     <RootNamespace>HidApi</RootNamespace>
-    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
 
     <Authors>Marcel Tiede</Authors>
@@ -32,6 +28,7 @@
   <ItemGroup>
     <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.1.1" PrivateAssets="All"/>
     <InternalsVisibleTo Include="$(AssemblyName).Tester" />
+    <InternalsVisibleTo Include="$(AssemblyName).Tests" />
     <None Include="../../readme.md" Pack="true" PackagePath="/" />
   </ItemGroup>
 </Project>
diff --git a/src/HidApi.Net/Internal/Unicode.cs b/src/HidApi.Net/Internal/Unicode.cs
@@ -4,11 +4,9 @@ namespace HidApi;
 
 internal static class Unicode
 {
-    private const int UnicodeSize = 2;
-
     public static ReadOnlySpan<byte> CreateBuffer(int size)
     {
-        return new byte[size * UnicodeSize];
+        return new byte[size * sizeof(ushort)];
     }
 
     public static unsafe string Read(byte* ptr)
@@ -25,13 +23,13 @@ private static unsafe int Length(byte* ptr)
         //check code to throw exception in case of arithmethic overflow
         checked
         {
-            var current = ptr;
+            var current = (ushort*) ptr;
             while (*current != 0)
             {
-                current += UnicodeSize; //Jump to next unicode char
+                current += 1; //Jump to next unicode char
             }
 
-            return (int) (current - ptr);
+            return (int) ((byte*) current - ptr);
         }
     }
 }
diff --git a/src/HidApi.Net/Internal/Utf32.cs b/src/HidApi.Net/Internal/Utf32.cs
@@ -4,11 +4,9 @@ namespace HidApi;
 
 internal static class Utf32
 {
-    private const int Utf32Size = 4;
-
     public static ReadOnlySpan<byte> CreateBuffer(int size)
     {
-        return new byte[size * Utf32Size];
+        return new byte[size * sizeof(uint)];
     }
 
     public static unsafe string Read(byte* ptr)
@@ -25,13 +23,13 @@ private static unsafe int Length(byte* ptr)
         //check code to throw exception in case of arithmethic overflow
         checked
         {
-            var current = ptr;
+            var current = (uint*) ptr;
             while (*current != 0)
             {
-                current += Utf32Size; //Jump to next UTF32 char
+                current += 1; //Jump to next UTF32 char
             }
 
-            return (int) (current - ptr);
+            return (int) ((byte*) current - ptr);
         }
     }
 }