Add ability to trim DtdParser when using default XmlReader settings

[vitek-karas]

When using XmlReader created via XmlReader.Create DTD parsing is prohibited by default (for security reasons). So it's very likely that vast majority of apps have it turned off. Trimming such app (either via PublishTrimmed=true or PublishAot=true) should ideally be able to remove all code related to DTD parsing, as it will never be used. Currently that's not the case.

Long time ago we had a similar problem where XmlReader would bring in XmlSchema and related validation classes in the default case - even though XSD validation is also off by default. This has been fixed in dotnet/corefx#23867

We should do something similar for DTD parsing. The way to fix this is to tie the dependency to DtdParser to setting the XmlReaderSettings.DtdProcessing. By default this property is Prohibit, so if it's not set, it's value will be Prohibit and thus DtdParser is not needed. If the app sets the DtdProcessing property we would have to assume that it enables it (static analysis currently can't tell the value) and make DtdParser available.

Note that the fix is not going to be as simple as it was for the XSD validation. The XmlReaderImpl has several places where it depends on DtdParser and the value of the DtdProcessing setting is copied from the settings into the reader and the reader also exposes it. So we would have to "guard" the DtdParser creation in multiple places (basically everywhere the DtdProcessing for the reader can be modified), but it definitely seems doable.

A simple experiment I did:

• Build a simple app which uses default XmlReader settings to read a simple XML file
• Trim it
• Apply a change by commenting out all uses of DtdParser from the reader
• Trim the app again

The size difference in System.Private.Xml.dll is almost 160KB, before the change the trimmed dll is 531KB, after the change it's 373KB. That is 30% size decrease for that dll. The overall size of all managed code in the sample app is 4.2 MB.

[ghost]

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr
See info in area-owners.md if you want to be subscribed.

Issue Details

---

When using XmlReader created via XmlReader.Create DTD parsing is prohibited by default (for security reasons). So it's very likely that vast majority of apps have it turned off. Trimming such app (either via PublishTrimmed=true or PublishAot=true) should ideally be able to remove all code related to DTD parsing, as it will never be used. Currently that's not the case.

Long time ago we had a similar problem where XmlReader would bring in XmlSchema and related validation classes in the default case - even though XSD validation is also off by default. This has been fixed in dotnet/corefx#23867

We should do something similar for DTD parsing. The way to fix this is to tie the dependency to DtdParser to setting the XmlReaderSettings.DtdProcessing. By default this property is Prohibit, so if it's not set, it's value will be Prohibit and thus DtdParser is not needed. If the app sets the DtdProcessing property we would have to assume that it enables it (static analysis currently can't tell the value) and make DtdParser available.

Note that the fix is not going to be as simple as it was for the XSD validation. The XmlReaderImpl has several places where it depends on DtdParser and the value of the DtdProcessing setting is copied from the settings into the reader and the reader also exposes it. So we would have to "guard" the DtdParser creation in multiple places (basically everywhere the DtdProcessing for the reader can be modified), but it definitely seems doable.

A simple experiment I did:

• Build a simple app which uses default XmlReader settings to read a simple XML file
• Trim it
• Apply a change by commenting out all uses of DtdParser from the reader
• Trim the app again

The size difference in System.Private.Xml.dll is almost 160KB, before the change the trimmed dll is 531KB, after the change it's 373KB. That is 30% size decrease for that dll. The overall size of all managed code in the sample app is 4.2 MB.

Author:	vitek-karas
Assignees:	-
Labels:	linkable-framework
Milestone:	-

[teo-tsirpanis]

Also tagging @dotnet/area-system-xml.

[krwq]

@vitek-karas is this important for 8.0? (move back if it is 😄)

[vitek-karas]

@eerhardt does XML matter for the cloud native scenarios? This affects any scenario which will parse XML (no matter the API used to do that).

[eerhardt]

For .NET 8, the main scenarios we are focused on are listed in dotnet/aspnetcore#45910.

For "Stage 1", there are no XML dependencies right now. I'm not certain about "Stage 2". But doing a quick search for XmlReader in https://source.dot.net/#System.Private.Xml/System/Xml/Core/XmlReader.cs,086471e5cca0825f,references, I'm not seeing any ASP.NET code using XmlReader. (But obviously this isn't an exhaustive search.)