SqlHierarchyId Write/Read roundtrip causes data corruption

[dotmorten]

SqlHierarchyId Write/Read roundtrip causes data corruption

[dotMorten]

Could you share an example that demonstrates this problem?

[masonwheeler]

Here's a very simple case:

using Microsoft.SqlServer.Types;
using System;
using System.IO;

namespace SqlHierarchy.Repro
{
    internal class Program
    {
        static void Main(string[] args)
        {
            var stream = new MemoryStream();
            var writer = new BinaryWriter(stream, System.Text.Encoding.UTF8, true);
            var hierarchy = SqlHierarchyId.Parse(new System.Data.SqlTypes.SqlString("/1/"));
            Console.WriteLine(hierarchy);
            hierarchy.Write(writer);
            for (int i = 0; i < 10; i++)
            {
                writer.Write(i);
            }
            writer.Close();
            stream.Position = 0;
            var reader = new BinaryReader(stream);
            hierarchy = new SqlHierarchyId();
            hierarchy.Read(reader);
            Console.WriteLine(hierarchy);
        }
    }
}

Expected: This should print /1/ twice.

Observed: Prints /1/ once, then crashes with:

Microsoft.SqlServer.Types.HierarchyIdException: '24000: SqlHierarchyId operation failed because HierarchyId object was constructed from an invalid binary string. '

[dotMorten]

@masonwheeler Thanks this is awesome. I tried this with the official .NET Framework implementation and it gives a very similar exception, so from what I can tell, this is "as designed".
Only thing I can think of doing here is slightly tweaking the error message to match.

Unit test used to compare behavior:

        [TestMethod]
        [WorkItem(64)]
        public void ReadFromContinuousStream()
        {
            var stream = new MemoryStream();
            var writer = new BinaryWriter(stream, System.Text.Encoding.UTF8, true);
            var hierarchy = SqlHierarchyId.Parse(new System.Data.SqlTypes.SqlString("/1/"));
            hierarchy.Write(writer);
            //Add extra bytes after
            for (int i = 0; i < 10; i++)
            {
                writer.Write(i);
            }
            writer.Close();
            stream.Position = 0;
            var reader = new BinaryReader(stream);
            var hierarchy2 = new SqlHierarchyId();
            hierarchy2.Read(reader);
            Assert.AreEqual(hierarchy.ToString(), hierarchy2.ToString());
        }

[dotMorten]

Error message thrown now matches .NET Framework implementation

[masonwheeler]

So where would I find the official repo to report this? It may be "as designed" but the design is completely broken.

[dotMorten]

If you clone this repo and run the unit test, it'll repeat the tests for both this implementation as well as the official Microsoft .NET Framework implementation. AFAIK there is no official repo for that library, so I'm not sure where to report it. Having said that, I don't really see how this can be supported, since there doesn't appear to be any indicator in a binary stream that you've reached the end of a hierarchy object and should stop reading.

[masonwheeler]

Yeah, that's the entire point of my complaint, as I noted in the original post. This is inconsistent with the binary serialization format of the other types, which are length-prefixed. That's how you fix it: change the serialization format to something sane.

[dotMorten]

You can try and get the sql server team to change a well-established format. I wish you the best of luck :-p

This is inconsistent with the binary serialization format of the other types

They are very different and serve different purposes, and AFAIK implemented by different teams.

[dotMorten]

Perhaps when you write it, add \0 after each hierarchy and when reading it back in, use that as delimiter before feeding it into the reader? Would be simple to do to address your scenario.