Runtime: mistakes on WebUtility.HtmlEncode

[kbetmp001]

https://github.com/dotnet/corefx/blob/99ea2b5aebf7361a5baa638c978cb6214fca5217/src/System.Runtime.Extensions/src/System/Net/WebUtility.cs#L651

should be
char leadingSurrogate = input[index];

[kbetmp001]

the input (double character emoji) "😁😂😃" was replaced to "😁😁😁" while HtmlEncode, after some debug, i found the index was not correctly set on GetNextUnicodeScalarValueFromUtf16Surrogate method.

[stephentoub]

cc: @GrabYourPitchforks

[GrabYourPitchforks]

As a side note, we should deprecate this API. It's not the recommended API for performing HTML encoding (and in fact its use is banned within MS first-party code).

[davidsh]

Originally we had the System.Web.HttpUtility class. Then during Windows 8 timeframe, we added the System.Net.WebUtility class (because we couldn't use System.Web.* in the Modern SDK).

@GrabYourPitchforks Are you proposing deprecating the entire System.Net.WebUtility class? What about System.Web.HttpUtility which has similar APIs? Should that be deprecated also?

What is the replacement API that developers should use now for HTML encoding?

System.Uri also has some APIs that look useful (but different) for encoding, i.e. Uri.EscapeUriString.

[GrabYourPitchforks]

What is the replacement API that developers should use now for HTML encoding?

Here's the rundown:

• WebUtility.HtmlEncode: __not__ SDL-approved.

• HttpUtility.HtmlEncode: SDL-approved __only when__ (a) running inside an ASP.NET Full Framework application, __and__ (b) that application targets ASP.NET 4.5 or higher.

• System.Text.Encodings.Web: SDL-approved on all platforms.

I'm not suggesting deprecating the entire class, just some (most?) of the APIs on the classes for which SDL-approved alternatives exist. It doesn't even have to be [Obsolete], perhaps [Discouraged] or similar. But along those lines, I'd prefer to say "these APIs are essentially end-of-life and we're not fixing bugs in them."

[GrabYourPitchforks]

Clarification: Steve pointed out that this might be a regression from Full Framework. In that case I'd definitely vote to fix it. My earlier comments were written with the assumption that this is a bug that is present _both_ in Full Framework _and_ in Core. Such bugs I would advocate not fixing because IMO we should be discouraging general use of this API anyway.

[stephentoub]

This regressed in .NET Core 3.0 as part of https://github.com/dotnet/corefx/pull/27250/commits/eded8f891bb3648299fa9fcff7be93ed15412d0b.
cc: @ViktorHofer

[stephentoub]

Reopening to track potential port to release/3.1.

[danmosemsft]

Thank you @kbetmp001 for reporting this.

[ericstj]

Adjusting milestone to reflect remaining work.

[danmosemsft]

@ericstj is anything left here? it's still for 3.x?

removing servicing label as that goes on PR's.

[ericstj]

I believe at the time I made that comment we were sitting on the 3.0 PR waiting to merge it when branches unlocked. PR https://github.com/dotnet/corefx/pull/41584 merged yesterday so this should be good.