NetFX compatibility fixes for X500DistinguishedName.

[filipnavara]

Fixes https://github.com/dotnet/corefx/issues/27466.

[filipnavara]

@bartonjs, can you review please?

[bartonjs]

Please add a comment to explain that explicit commas beats newline, but that commas are the implicit default.

[bartonjs]

Please break these long constants up into multiple lines.

[bartonjs]

I don't know how reliably o-slash is going to roundtrip through various git clients and text editors. Please replace it with \u00d8.

[filipnavara]

Good point!

[bartonjs]

I'm guessing that it's actually Windows-1252, or, more transparently:

char[] chars = new char[contentLength];

for (int i = 0; i < contentLength; i++)
{
    chars[i] = (char)_data[_position + i];
}

string t61string = new string(chars);

[filipnavara]

I actually tried couple of 1252 characters on Windows that are missing in iso-8859-1 and they were missing from the output.

[bartonjs]

Sounds like a test to add 😄

[filipnavara]

Turns out it is neither 1252 nor iso-8859-1. Windows actually interprets it as UTF-8. I will add test case for that.

[bartonjs]

0603550406 is "2.5.4.6" => id-countryName ("C=", not "CN=").

[bartonjs]

on macOS the "Møbelhandel" didn't decode correctly (it got a FFFD replacement character).

So maybe it's something more nuanced. UTF-8 unless it had a decode error (e.g. had to add an FFFD), and Windows-1252 otherwise.

Looks like, maybe, this could be

private static Text.Encoding MakeT61Encoding()
{
    // Using Clone() allows DecoderFallback to be assigned.
    var utf8 = Text.Encoding.Utf8.Clone();
    utf8.DecoderFallback = new TheFallbackWouldHaveToBeWritten();
    // Until further need.
    utf8.EncoderFallback = EncoderFallback.ExceptionFallback;
}

Or

s_t61Utf8 = new Utf8Encoding(false, true);

try
{
    s_t61Utf8.GetWhatever(...);
}
catch (DecoderFallbackException)
{
    bytes-to-chars it is, then();
}

depending on if the fallback is "whole string" or "per character".

[filipnavara]

Looks like macOS is not the only problematic one. The OpenSSL decoder fails on the UTF-8 string. It could be easier to just find out what OpenSSL does and mimic that instead of Windows.

[filipnavara]

The NETFX test failure is unrelated to the changes.

[bartonjs]

What happens if the payload had a UTF-8 multi-byte character followed by an invalid UTF-8 sequence?

[filipnavara]

300E310C300A06035504071403C2A2F8 -> L=墿 (https://dotnetfiddle.net/ERdAyB), will add it to the tests

[bartonjs]

I think I'm fine with dumping X500NameEncoder.OpenSslDecode.cs as part of this change, and just using the same managed implementation on macOS and Linux. It'll cut down on a lot of SafeHandles and P/Invokes, as well as unify the behaviors.

[filipnavara]

Will do.

[bartonjs]

@dotnet-bot Test NETFX x86 Release Build please (now that #30565 has merged and X509Certificates tests are enabled for netfx)

[bartonjs]

This could be simplified to new Utf8Encoding(false, true) since it's using the ExceptionFallback.

I'd probably promote this and the ISO-8859-1 encoding objects to statics, no strong opinion on fieldinit or LazyInitializer.EnsureInitialized or just if (s_utf8WithExceptionsEncoding == null) { s_utf8WithExceptionsEncoding = new Utf8Encoding(false, true); }

[filipnavara]

I didn't use new Utf8Encoding(false, true) the first time because of the additional reference to System.Text.Encoding.Extensions. LazyInitializer.EnsureInitialized seems like a reasonable option since it's already used elsewhere in the class.

[bartonjs]

Aside from the last comment about the two Encoding objects I think I'm happy with it. @stephentoub would you mind giving it a once-over?

[stephentoub]

Other than the one issue about not creating a new Encoding for each use, LGTM.

[bartonjs]

Thanks, @filipnavara :)