Signature: track data as a byte[] instead of a string. (#311)

This causes breaking change to the VariantValue type: - The GetSignature method now returns a Signature, and - The generic parameters of GetArray/GetDictionary require a 'Signature' type instead of a 'string' type.
tmds · Nov 10, 2024 · 2a41d14 · 2a41d14
1 parent 067b449
commit 2a41d14
Show file tree

Hide file tree

Showing 12 changed files with 68 additions and 37 deletions.
diff --git a/src/Tmds.DBus.Protocol/DBusConnection.cs b/src/Tmds.DBus.Protocol/DBusConnection.cs
@@ -288,7 +288,7 @@ await CallMethodAsync(
                 }
                 else if (message.MessageType == MessageType.MethodReturn)
                 {
-                    vtsState.SetResult(message.GetBodyReader().ReadString().ToString());
+                    vtsState.SetResult(message.GetBodyReader().ReadString());
                 }
                 else
                 {

diff --git a/src/Tmds.DBus.Protocol/Message.cs b/src/Tmds.DBus.Protocol/Message.cs
@@ -190,7 +190,7 @@ private void ParseHeader(UnixFdCollection? handles, bool isMonitor)
         while (reader.HasNext(headersEnd))
         {
             MessageHeader hdrType = (MessageHeader)reader.ReadByte();
-            ReadOnlySpan<byte> sig = reader.ReadSignature();
+            ReadOnlySpan<byte> sig = reader.ReadSignatureAsSpan();
             switch (hdrType)
             {
                 case MessageHeader.Path:
@@ -215,7 +215,7 @@ private void ParseHeader(UnixFdCollection? handles, bool isMonitor)
                     _sender.Set(reader.ReadStringAsSpan());
                     break;
                 case MessageHeader.Signature:
-                    _signature.Set(reader.ReadSignature());
+                    _signature.Set(reader.ReadSignatureAsSpan());
                     break;
                 case MessageHeader.UnixFds:
                     UnixFdCount = (int)reader.ReadUInt32();

diff --git a/src/Tmds.DBus.Protocol/MessageWriter.Basic.cs b/src/Tmds.DBus.Protocol/MessageWriter.Basic.cs
@@ -26,13 +26,15 @@ public ref partial struct MessageWriter
 
     public void WriteString(string value) => WriteStringCore(value);
 
-    public void WriteSignature(Utf8Span value)
+    public void WriteSignature(Signature value)
+        => WriteSignature(value.Data);
+
+    public void WriteSignature(ReadOnlySpan<byte> value)
     {
-        ReadOnlySpan<byte> span = value;
-        int length = span.Length;
+        int length = value.Length;
         WriteByte((byte)length);
         var dst = GetSpan(length);
-        span.CopyTo(dst);
+        value.CopyTo(dst);
         Advance(length);
         WriteByte((byte)0);
     }
@@ -50,6 +52,8 @@ public void WriteSignature(string s)
 
     public void WriteObjectPath(string value) => WriteStringCore(value);
 
+    public void WriteObjectPath(ObjectPath value) => WriteStringCore(value.ToString());
+
     public void WriteVariantBool(bool value)
     {
         WriteSignature(ProtocolConstants.BooleanSignature);

diff --git a/src/Tmds.DBus.Protocol/MessageWriter.WriteT.cs b/src/Tmds.DBus.Protocol/MessageWriter.WriteT.cs
@@ -47,11 +47,11 @@ internal void Write<T>(T value) where T : notnull
         }
         else if (typeof(T) == typeof(ObjectPath))
         {
-            WriteString(((ObjectPath)(object)value).ToString());
+            WriteObjectPath(((ObjectPath)(object)value));
         }
         else if (typeof(T) == typeof(Signature))
         {
-            WriteSignature(((Signature)(object)value).ToString());
+            WriteSignature(((Signature)(object)value));
         }
         else if (typeof(T) == typeof(Variant))
         {

diff --git a/src/Tmds.DBus.Protocol/Reader.Basic.cs b/src/Tmds.DBus.Protocol/Reader.Basic.cs
@@ -65,15 +65,19 @@ public unsafe double ReadDouble()
         return value;
     }
 
+    [Obsolete($"Use {nameof(ReadSignatureAsSpan)}.")]
     public Utf8Span ReadSignature()
+        => ReadSignatureAsSpan();
+
+    public Utf8Span ReadSignatureAsSpan()
     {
         int length = ReadByte();
         return ReadSpan(length);
     }
 
     public void ReadSignature(string expected)
     {
-        ReadOnlySpan<byte> signature = ReadSignature().Span;
+        ReadOnlySpan<byte> signature = ReadSignatureAsSpan().Span;
         if (signature.Length != expected.Length)
         {
             ThrowHelper.ThrowUnexpectedSignature(signature, expected);
@@ -97,9 +101,9 @@ public void ReadSignature(string expected)
 
     public string ReadString() => Encoding.UTF8.GetString(ReadSpan());
 
-    public Signature ReadSignatureAsSignature() => new Signature(ReadSignature().ToString());
+    public Signature ReadSignatureAsSignature() => new Signature(ReadSignatureAsSpan());
 
-    public string ReadSignatureAsString() => ReadSignature().ToString();
+    public string ReadSignatureAsString() => Encoding.UTF8.GetString(ReadSignatureAsSpan());
 
     private ReadOnlySpan<byte> ReadSpan()
     {

diff --git a/src/Tmds.DBus.Protocol/Reader.Variant.cs b/src/Tmds.DBus.Protocol/Reader.Variant.cs
@@ -7,7 +7,7 @@ public VariantValue ReadVariantValue()
 
     private VariantValue ReadVariantValue(byte nesting)
     {
-        Utf8Span signature = ReadSignature();
+        Utf8Span signature = ReadSignatureAsSpan();
         SignatureReader sigReader = new(signature);
         if (!sigReader.TryRead(out DBusType type, out ReadOnlySpan<byte> innerSignature))
         {

diff --git a/src/Tmds.DBus.Protocol/Signature.cs b/src/Tmds.DBus.Protocol/Signature.cs
@@ -2,11 +2,22 @@ namespace Tmds.DBus.Protocol;
 
 public struct Signature
 {
-    private string _value;
+    private byte[]? _value;
 
-    public Signature(string value) => _value = value;
+    internal byte[] Data => _value ?? Array.Empty<byte>();
 
-    public override string ToString() => _value ?? "";
+    [Obsolete("Use the constructor that accepts a ReadOnlySpan.")]
+    public Signature(string value)
+        => _value = Encoding.UTF8.GetBytes(value);
+
+    public Signature(ReadOnlySpan<byte> value)
+        => _value = value.ToArray();
+
+    public override string ToString()
+        => Encoding.UTF8.GetString(Data);
+
+    public static implicit operator Signature(ReadOnlySpan<byte> value)
+        => new Signature(value);
 
     public Variant AsVariant() => new Variant(this);
 }
diff --git a/src/Tmds.DBus.Protocol/Variant.cs b/src/Tmds.DBus.Protocol/Variant.cs
@@ -82,12 +82,12 @@ public Variant(ObjectPath value)
     public Variant(Signature value)
     {
         _l = (long)DBusType.Signature << TypeShift;
-        string s = value.ToString();
-        if (s.Length == 0)
+        byte[] data = value.Data;
+        if (data.Length == 0)
         {
             throw new ArgumentException(nameof(value));
         }
-        _o = s;
+        _o = data;
     }
     public Variant(SafeHandle value)
     {
@@ -336,10 +336,10 @@ private string GetObjectPath()
         DebugAssertTypeIs(DBusType.ObjectPath);
         return (_o as string)!;
     }
-    private string GetSignature()
+    private byte[] GetSignature()
     {
         DebugAssertTypeIs(DBusType.Signature);
-        return (_o as string)!;
+        return (_o as byte[])!;
     }
     private SafeHandle GetUnixFd()
     {
@@ -407,7 +407,7 @@ internal unsafe void WriteTo(ref MessageWriter writer)
                 writer.WriteVariantObjectPath(GetObjectPath());
                 break;
             case DBusType.Signature:
-                writer.WriteVariantSignature(GetSignature());
+                writer.WriteVariantSignature(new Utf8Span(GetSignature()));
                 break;
             case DBusType.UnixFd:
                 writer.WriteVariantHandle(GetUnixFd());

diff --git a/src/Tmds.DBus.Protocol/VariantValue.cs b/src/Tmds.DBus.Protocol/VariantValue.cs
@@ -221,12 +221,12 @@ internal VariantValue(Signature value) :
     internal VariantValue(Signature value, byte nesting)
     {
         _l = GetTypeMetadata(VariantValueType.Signature, nesting);
-        string s = value.ToString();
-        if (s.Length == 0)
+        byte[] data = value.Data;
+        if (data.Length == 0)
         {
             throw new ArgumentException(nameof(value));
         }
-        _o = s;
+        _o = data;
     }
     // Array
     internal VariantValue(VariantValueType itemType, object? itemSignature, VariantValue[] items, byte nesting)
@@ -520,10 +520,16 @@ public string GetObjectPath()
         return UnsafeGetString();
     }
 
-    public string GetSignature()
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    private Signature UnsafeGetSignature()
+    {
+        return new Signature((_o as byte[])!);
+    }
+
+    public Signature GetSignature()
     {
         EnsureTypeIs(VariantValueType.Signature);
-        return UnsafeGetString();
+        return UnsafeGetSignature();
     }
 
     public double GetDouble()
@@ -722,7 +728,11 @@ private static void EnsureCanUnsafeGet<T>(VariantValueType type)
         }
         else if (typeof(T) == typeof(string))
         {
-            EnsureTypeIs(type, [ VariantValueType.String, VariantValueType.Signature, VariantValueType.ObjectPath ]);
+            EnsureTypeIs(type, [ VariantValueType.String, VariantValueType.ObjectPath ]);
+        }
+        else if (typeof(T) == typeof(Signature))
+        {
+            EnsureTypeIs(type, VariantValueType.Signature);
         }
         else if (typeof(T) == typeof(VariantValue))
         { }
@@ -984,7 +994,7 @@ public string ToString(bool includeTypeSuffix)
             case VariantValueType.ObjectPath:
                 return $"{UnsafeGetString()}{TypeSuffix(includeTypeSuffix, type, nesting)}";
             case VariantValueType.Signature:
-                return $"{UnsafeGetString()}{TypeSuffix(includeTypeSuffix, type, nesting)}";
+                return $"{UnsafeGetSignature()}{TypeSuffix(includeTypeSuffix, type, nesting)}";
             case VariantValueType.Struct:
                 var values = (_o as VariantValue[]) ?? Array.Empty<VariantValue>();
                 return $"({string.Join(", ", values.Select(v => v.ToString(includeTypeSuffix: false)))}){TypeSuffix(includeTypeSuffix, type, nesting)}";
@@ -1069,8 +1079,9 @@ public bool Equals(VariantValue other)
                 return true;
             case VariantValueType.String:
             case VariantValueType.ObjectPath:
-            case VariantValueType.Signature:
                 return (_o as string)!.Equals(other._o as string, StringComparison.Ordinal);
+            case VariantValueType.Signature:
+                return (_o as byte[])!.SequenceEqual((other._o as byte[])!);
         }
         // Always return false for composite types and handles.
         return false;

diff --git a/test/Tmds.DBus.Protocol.Tests/ReaderTests.cs b/test/Tmds.DBus.Protocol.Tests/ReaderTests.cs
@@ -97,7 +97,7 @@ public void ReadDouble(double expected, int alignment, byte[] bigEndianData, byt
                           new byte[] { 3, (byte)'s', (byte)'i', (byte)'s', 0 })]
     public void ReadSignature(string expected, int alignment, byte[] bigEndianData, byte[] littleEndianData)
     {
-        TestRead(expected, (ref Reader reader) => reader.ReadSignature().ToString(), alignment, bigEndianData, littleEndianData);
+        TestRead(expected, (ref Reader reader) => reader.ReadSignatureAsString(), alignment, bigEndianData, littleEndianData);
     }
 
     [Theory]
@@ -300,7 +300,7 @@ public static IEnumerable<object[]> ReadVariantValueTestData
                 new object[] {new VariantValue(new ObjectPath("/a/b")),
                                                 new byte[] {1, 111, 0, 0, 0, 0, 0, 4, 47, 97, 47, 98, 0},
                                                 new byte[] {1, 111, 0, 0, 4, 0, 0, 0, 47, 97, 47, 98, 0}},
-                new object[] {new VariantValue(new Signature("sis")),
+                new object[] {new VariantValue(new Signature("sis"u8)),
                                                 new byte[] {1, 103, 0, 3, 115, 105, 115, 0},
                                                 new byte[] {1, 103, 0, 3, 115, 105, 115, 0}},
                 new object[] {new VariantValue(new long[] { 1, 2}),
@@ -349,7 +349,7 @@ public static IEnumerable<object[]> ReadVariantValueTestData
                 new object[] {VariantValue.CreateVariant(new VariantValue(new ObjectPath("/a/b"))),
                                                 new byte[] {1, 118, 0, 1, 111, 0, 0, 0, 0, 0, 0, 4, 47, 97, 47, 98, 0},
                                                 new byte[] {1, 118, 0, 1, 111, 0, 0, 0, 4, 0, 0, 0, 47, 97, 47, 98, 0}},
-                new object[] {VariantValue.CreateVariant(new VariantValue(new Signature("sis"))),
+                new object[] {VariantValue.CreateVariant(new VariantValue(new Signature("sis"u8))),
                                                 new byte[] {1, 118, 0, 1, 103, 0, 3, 115, 105, 115, 0},
                                                 new byte[] {1, 118, 0, 1, 103, 0, 3, 115, 105, 115, 0}},
                 new object[] {VariantValue.CreateVariant(new VariantValue(new long[] { 1, 2})),

diff --git a/test/Tmds.DBus.Protocol.Tests/VariantValueTests.cs b/test/Tmds.DBus.Protocol.Tests/VariantValueTests.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Text;
 using Microsoft.Win32.SafeHandles;
 using Xunit;
 
@@ -294,11 +295,11 @@ public void ObjectPath(string s, byte nesting)
     [InlineData("sis", 1)]
     public void Signature(string s, byte nesting)
     {
-        Signature value = new Signature(s);
+        Signature value = new Signature(Encoding.UTF8.GetBytes(s));
         VariantValue vv = nesting > 0 ? new(value, nesting) : new(value);
         UnwrapVariant(ref vv, nesting);
 
-        Assert.Equal(s, vv.GetSignature());
+        Assert.Equal(s, vv.GetSignature().ToString());
 
         Assert.Equal(VariantValueType.Signature, vv.Type);
         Assert.Equal(VariantValueType.Invalid, vv.ItemType);

diff --git a/test/Tmds.DBus.Protocol.Tests/WriterTests.cs b/test/Tmds.DBus.Protocol.Tests/WriterTests.cs
@@ -232,7 +232,7 @@ public static IEnumerable<object[]> WriteVariantAsObjectTestData
                                                 new byte[] {1, 115, 0, 0, 2, 0, 0, 0, 104, 119, 0}},
                 new object[] {new ObjectPath("/a/b"), new byte[] {1, 111, 0, 0, 0, 0, 0, 4, 47, 97, 47, 98, 0},
                                                       new byte[] {1, 111, 0, 0, 4, 0, 0, 0, 47, 97, 47, 98, 0}},
-                new object[] {new Signature("sis"), new byte[] {1, 103, 0, 3, 115, 105, 115, 0},
+                new object[] {new Signature("sis"u8), new byte[] {1, 103, 0, 3, 115, 105, 115, 0},
                                                     new byte[] {1, 103, 0, 3, 115, 105, 115, 0}},
                 new object[] {new long[] { 1, 2}, new byte[] {2, 97, 120, 0, 0, 0, 0, 16, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 2},
                                                   new byte[] {2, 97, 120, 0, 16, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, 0}},
@@ -281,7 +281,7 @@ public static IEnumerable<object[]> WriteVariantAsVariantTestData
                                                                 new byte[] {1, 115, 0, 0, 2, 0, 0, 0, 104, 119, 0}},
                 new object[] {new ObjectPath("/a/b").AsVariant(),  new byte[] {1, 111, 0, 0, 0, 0, 0, 4, 47, 97, 47, 98, 0},
                                                                     new byte[] {1, 111, 0, 0, 4, 0, 0, 0, 47, 97, 47, 98, 0}},
-                new object[] {new Signature("sis").AsVariant(),    new byte[] {1, 103, 0, 3, 115, 105, 115, 0},
+                new object[] {new Signature("sis"u8).AsVariant(),    new byte[] {1, 103, 0, 3, 115, 105, 115, 0},
                                                                     new byte[] {1, 103, 0, 3, 115, 105, 115, 0}},
                 new object[] {new Array<long>([1, 2]).AsVariant(),
                                                 new byte[] {2, 97, 120, 0, 0, 0, 0, 16, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 2},