Add feature switch to disable custom .resources reader support (UserResourceSet)

[marek-safar]

Support for external custom resource-set types seems to be very advanced and possibly never used feature in practice. It makes trimming very complicated and brings large unnecessary dependencies as well as opens doors for potential security problems. The idea would be to add a new feature switch to block setting mediator.UserResourceSet and validate that .resources files require only System.Resources.RuntimeResourceSet and System.Resources.ResourceReader which I suspect all netcore apps use anyway.

Effectively replacing CreateResourceSet with

internal static RuntimeResourceSet CreateResourceSet(Stream store)
{
	// TODO: +Do checking for resource header versions
	return new RuntimeResourceSet(store, permitDeserialization: true);
}

This would have two effects

1. Removing all resources related warnings from illinker because of linker unsafe behaviour
2. Large size savings in refactored code due to fewer dependencies

The feature switch would also help with size trimming work a lot because it would allow propagating the information that we are dealing with RuntimeResourceSet only everywhere. In practice removing, for example, all code in ResourceReader which handles non-_ums code paths (including Unicode encoding dependency, etc).

@eerhardt @vitek-karas

[ghost]

Tagging subscribers to this area: @tarekgh, @buyaa-n, @krwq, @jeffhandley
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Support for external custom resource-set types seems to be very advanced and possibly never used feature in practice. It makes trimming very complicated and brings large unnecessary dependencies as well as opens doors for potential security problems. The idea would be to add a new feature switch to block setting mediator.UserResourceSet and validate that .resources files require only System.Resources.RuntimeResourceSet and System.Resources.ResourceReader which I suspect all netcore apps use anyway.

Effectively replacing CreateResourceSet with

internal static RuntimeResourceSet CreateResourceSet(Stream store)
{
	// TODO: +Do checking for resource header versions
	return new RuntimeResourceSet(store, permitDeserialization: true);
}

This would have two effects

1. Removing all resources related warnings from illinker because of linker unsafe behaviour
2. Large size savings in refactored code due to fewer dependencies

The feature switch would also help with size trimming work a lot because it would allow propagating the information that we are dealing with RuntimeResourceSet only everywhere. In practice removing, for example, all code in ResourceReader which handles non-_ums code paths (including Unicode encoding dependency, etc).

@eerhardt @vitek-karas

Author:	marek-safar
Assignees:	-
Labels:	area-System.Resources, untriaged
Milestone:	-

[eerhardt]

My thinking along these lines was that we would have one big feature switch for System.Resources to disable all the linker-unsafe code in ResourceManager identified in #32862 (sans the BinaryFormatter code, since we already have that).

Basically something like System.Resources.AllowCustomResourceTypes which could be set to false to remove all the code and throw if it encounters a custom type in the stream.

---

Support for external custom resource-set types seems to be very advanced and possibly never used feature in practice.

I see that dotnet/winforms has a custom ResourceSet type - https://github.com/dotnet/winforms/blob/master/src/System.Windows.Forms/src/System/Resources/ResXResourceSet.cs. Would this affect that at all?

cc @ericstj