The BinaryData format property can't handle byte[] data properly.

[ArthurMa1978]

[Yao725]

After investigation, here is what I found from this issue:

• Generally speaking, if we want to construct the BinaryData in management plane SDK from byte array, we should ensure that the byte array is a valid json value, this needs special attention especially when the byte array is representing a json string. Here the action should be similar to what we did with the method BinaryData.FromString("\"foo\""). Take a look at below sample code for details.

string sample = "this is the sample code.";

byte[] input = UTF8Encoding.UTF8.GetBytes($"\"{sample}\"");
// byte[] input = UTF8Encoding.UTF8.GetBytes(sample);  wrong way to get the byte array when faced with a string

BinaryData data = BinaryData.FromBytes(input);

• Back to this issue, another thing that we need to take care of is the encode and decode. BinaryData is actually based on the UTF-8, but the value of X509Thumbprint is based on Base64. That are totally two different things, which means even if the value of X509Thumbprint is a valid json value, we still cannot get the correct result if we directly use these bytes to construct the BinaryData.

Based on above findings, we need to update the code so that the serialization could work as expected:

Thumbprint = BinaryData.FromString($"\"{Convert.ToHexString(certificate.Properties.X509Thumbprint)}\"");

Finally, I think we really need a full documentation for BinaryData and this documentation could be part of the management plane SDK FAQ, and this documentation should contain below contents at least:

• what the BinaryData is
• Why we introduced it in our SDK
• How to transform between the value (such as string, JosnObject, etc.) and BinaryData
• Additional notes on using BinaryData

[live1206]

There is nothing wrong with BinaryData here, the below test will succeed with BinaryData and Base64Url encoding and decoding.

            var str = "hello world";
            var encodedStr = Base64Url.EncodeString(str);
            var decodedBytes = Base64Url.Decode(encodedStr);
            var result = BinaryData.FromBytes(decodedBytes).ToString();
            Assert.AreNotEqual(str, result);

The error was thrown from JSON serialization, because the original content of X509Thumbprint was a UTF-16 format string and we are using JsonElement in the implementation, which is expecting UTF-8 format.

The user can convert the original string to a hex string, which is what the Azure Portal display text does, then it can be serialized by JsonElement.

Then the question is, does the server side always expect a hex string for Thumbprint? If so, we should return a hex string for X509Thumbprint from GetCertificateAsync in the first place.

@heaths Could you provide your thoughts here? Thanks!

[heaths]

I'm not sure how GetCertificateAsync relates to CDN's return of a thumbprint. Those are from separate endpoints and in different SDKs. GetCertificatesAsync returns a valid X509 certificate, so the problem appears to be in how CDN's thumbprint is being interpretted.

Thumbprints, yes, are typically represented as a hash string. You'd have to talk with the CDN folks about how their thumbprint is represented and should be used, though.

[live1206]

In mgmt, we will add a new property named X509ThumbprintAsHexString, which returns Convert.ToHexString(this.Thumbprint), and make Thumbprint to EB.Never

[heaths]

In mgmt, we will add a new property named X509ThumbprintAsHexString, which returns Convert.ToHexString(this.Thumbprint), and make Thumbprint to EB.Never

@live1206 that is not congruent with the latest conversation between @KrzysztofCwalina, @m-nash, and myself. Has the plan changed? As Krzysztof noted, he doesn't agree we should add a simple property getter that converts the byte[] to a hex string, so what value is there in accepting on in the management plane SDK when you could marshal the byte[] array instead?

[KrzysztofCwalina]

To clarify my position: we should try to endup with one representation (either string or byte[]). I originally suggested byte[], but if string is more common in these scenarios, string (hex string) is fine with me. What I think we should avoid is having two representation and adding conversion APIs.

[heaths]

Key Vault SDK long ago shipped a property of type byte[]. I could add a property e.g.,

public string X509ThumbprintAsHexString => Convert.ToHexString(X509Thumbprint);

But you said,

I am totally on board with exposing byte[] (or binarydata) property in the management plane (as proposed below), but I am not sure we need a new property in the dataplane that simply converts bytes to hex string. This is a simple conversion that users can do themselves.

Seems we'd want congruent properties to avoid confusion.

[m-nash]

Agreed if we think hex string is the correct common format then adding the extra property in keyvault and switching from BinaryData to string in mgmt would solve the iterop issue here.

[heaths]

Ping @KrzysztofCwalina for feedback on the last couple comments. /cc @tg-msft

[KrzysztofCwalina]

I am fine with using hex string as the main interchange format (for thumbprints).

[heaths]

I am fine with using hex string as the main interchange format (for thumbprints).

@KrzysztofCwalina the specific question was whether the Key Vault SDK should add a property that just returns the hex string or not. Previously you were opposed.

[KrzysztofCwalina]

@KrzysztofCwalina the specific question was whether the Key Vault SDK should add a property that just returns the hex string or not. Previously you were opposed.

I was opposed to having both representations being "first class". I was (and am) arguing for picking one representation and then evolving the APIs in that direction. I don't think we should have both representations exposed prominently and asking users to pick.

[heaths]

The Key Vault SDK already returns a byte[] array, so exposing anything else means adding a property.

[heaths]

Discussing offline, we've decided to add a string X509ThumbprintString property to both Key Vault and managed SDKs. KV SDKs will also hide the X509Thumbprint but not remove it.

@m-nash let's use this bug to track the management side. I'll open a new one for the data plane SDK.

[live1206]

The new version of corresponding SDKs will be released soon.