Feature request for Open XML SDK - Exclude unsupported XML characters during file generation

[tauheedul]

I have encountered an scenario where a user has entered data into the database which included control characters (most likely from a 3rd party system).

Subequently when attempting to export the data to an OpenXML spreadsheet directly from the database e.g. creating a spreadsheet using a DataTable with no pre-processing, the Open XML plugin fails because of the invalid hex characters detected.

Suggested solution
Upon saving a datatable and while parsing each column value, detect Invalid Xml Characters and exclude them from the output file or transform them to an XML equivalent character where possible.

Something like this: https://stackoverflow.com/questions/8170739/dealing-with-invalid-xml-hexadecimal-characters

The workaround
The workaround for us is to find every field at source and exclude invalid characters being added to the database, but this isn't feasable for a large system.

The other solution is to manually map the data table and exclude special characters at that point, but that adds additional overhead for large datasets because the table will be iterated again when OpenXML opens the dataset.

Example error

• Exception message

'�', hexadecimal value 0x02, is an invalid character.

• Exception stacktrace

  at System.Xml.XmlUtf8RawTextWriter.InvalidXmlChar(Int32 ch, Byte* pDst, Boolean entitize)
  at System.Xml.XmlUtf8RawTextWriter.WriteElementTextBlock(Char* pSrc, Char* pSrcEnd)
  at System.Xml.XmlUtf8RawTextWriter.WriteString(String text)
  at System.Xml.XmlWellFormedWriter.WriteString(String text)
  at DocumentFormat.OpenXml.OpenXmlLeafTextElement.WriteContentTo(XmlWriter w)
  at DocumentFormat.OpenXml.OpenXmlElement.WriteTo(XmlWriter xmlWriter)
  at DocumentFormat.OpenXml.OpenXmlCompositeElement.WriteContentTo(XmlWriter w)
  at DocumentFormat.OpenXml.OpenXmlElement.WriteTo(XmlWriter xmlWriter)
  at DocumentFormat.OpenXml.OpenXmlCompositeElement.WriteContentTo(XmlWriter w)
  at DocumentFormat.OpenXml.OpenXmlElement.WriteTo(XmlWriter xmlWriter)
  at DocumentFormat.OpenXml.OpenXmlCompositeElement.WriteContentTo(XmlWriter w)
  at DocumentFormat.OpenXml.OpenXmlElement.WriteTo(XmlWriter xmlWriter)
  at DocumentFormat.OpenXml.OpenXmlCompositeElement.WriteContentTo(XmlWriter w)
  at DocumentFormat.OpenXml.OpenXmlPartRootElement.WriteTo(XmlWriter xmlWriter)
  at DocumentFormat.OpenXml.OpenXmlPartRootElement.Save(Stream stream)
  at DocumentFormat.OpenXml.OpenXmlPartRootElement.SaveToPart(OpenXmlPart openXmlPart)
  at DocumentFormat.OpenXml.OpenXmlPartRootElement.Save()
  at DocumentFormat.OpenXml.Packaging.OpenXmlPackage.TrySavePartContent(OpenXmlPart part)
  at DocumentFormat.OpenXml.Packaging.OpenXmlPackage.SavePartContents(Boolean save)
  at DocumentFormat.OpenXml.Packaging.OpenXmlPackage.Dispose(Boolean disposing)
  at DocumentFormat.OpenXml.Packaging.OpenXmlPackage.Dispose()
  at TestOpenXmlSpreadsheet.Helper.ExportToExcelDownload(DataSet dataSet) in C:\Users\user\Source\repos\TestOpenXmlSpreadsheet\TestOpenXmlSpreadsheet\Form1.cs:line 118
  at TestOpenXmlSpreadsheet.Form1.btnTest_Click(Object sender, EventArgs e) in C:\Users\user\Source\repos\TestOpenXmlSpreadsheet\TestOpenXmlSpreadsheet\Form1.cs:line 39
  at System.Windows.Forms.Control.OnClick(EventArgs e)
  at System.Windows.Forms.Button.OnClick(EventArgs e)
  at System.Windows.Forms.Button.OnMouseUp(MouseEventArgs mevent)
  at System.Windows.Forms.Control.WmMouseUp(Message& m, MouseButtons button, Int32 clicks)
  at System.Windows.Forms.Control.WndProc(Message& m)
  at System.Windows.Forms.ButtonBase.WndProc(Message& m)
  at System.Windows.Forms.Button.WndProc(Message& m)
  at System.Windows.Forms.Control.ControlNativeWindow.WndProc(Message& m)
  at System.Windows.Forms.NativeWindow.Callback(IntPtr hWnd, WM msg, IntPtr wparam, IntPtr lparam)

Reproducible test
My test file generation logic is derived from the following sample code
https://www.codeindotnet.com/export-dataset-to-excel-in-c-net-core-openxml/

A single column datatable has a record added with the invalid text input by the user.
This is passed into the OpenXML file creation logic and the failure occurs after the Workbook.Save() is invoked.

• Failure text
  Invalid Character.txt

This example is produced on .NET 5 with 3.0.0 Beta 3
I have also reproduced this on .NET Framework 4.5 with 2.20.0

[twsouthwick]

do you have a repro project to try locally?

My first thought is that this kind of validation probably doesn't belong here. If you are constructing the document, you can inject some validation/sanitizer methods at that level. If you have an idea of what code pattern you are hoping to enable, we can potentially add something here, but it doesn't feel right at this level.

[tauheedul]

Hello @twsouthwick I've attached my sample test project to replicate the issue manually.
TestOpenXmlSpreadsheet.zip

The issue could occur in situations you would directly query the database and return the results as a DataTable with no pre-processing. Most developers would not check the record before adding it to the sheetData as they would expect the package to handle breaking XML characters internally, for example on instantiation of a new CellValue() or on cell.Append()

Presently, the raw text is added to the sheet data, but no exception is raised until after the application invokes the Save method.
workbookpart.Workbook.Save();

The XML error will be thrown from the OpenXML package as described.

[twsouthwick]

@tauheedul I see how it might make it easier to inject it here. I'd be open to a PR here if you're interested in contributing. Looking at where the error lies, it seems like the best way to enable this would be to centralize (and abstract) our creation of XmlWriter instances.

Design Proposal

API Design

namespace DocumentFormat.OpenXml.Packaging;

public static class PackageSanitizationExtensions
{
  void UseXmlTextSanitization(this OpenXmlPackage package);
}

Usage:

var package = WordprocessingDocument.Create(....);
package.UseTextSanitization();

// use as normal

Implementation Details

The key principles for this as it is implemented should be:

• It should be pay-for-play: if someone doesn't want to incur the cost of sanitizing, they shouldn't
• Should be an orthogonal behavior to anything else going on

My recommendation to how we may want to do it (feel free to give a different design):

• We make use of features to abstract behavior away (if you're familiar with ASP.NET Core HttpContext.Features, it's a similar idea)
• Right now, I believe we create XmlWriter instances throughout the code base without a single place to do so
• Move all of those into a single location
• Plumb that through into features
• Expose an extension method that replaces the default feature with one that sanitizes things by giving it a derived XmlWriter that does sanitizing. It may be configurable for different definitions of sanitize, but starting off it could do some reasonable default behavior.

This means, that from an end-user (developer) perspective, the usage would be:

If I were to break this into tasks I'd submit as separate PRs, it becomes three dependent steps:

• Move all XmlWriter creation into a shared method
• Create a new feature for XmlWriter creation and hook it up to the feature collection, point to that shared method
• Add an extension method UseTestSantization(this OpenXmlPackage, ... /*options?*/) that would call the original feature, and then wrap in a custom XmlWriter that handles sanitizing

Happy to accept other ideas for how this should be done. If you're interested in using the plan I proposed above, feel free to submit PRs (potentially incremental so you can get used to our process) to enable that.

[tipa]

I am facing the same problem and would welcome a fix! I am currently filtering out invalid characters in my own code

[twsouthwick]

Happy to take a PR to enable any of the steps I listed above :) I don't have time currently to commit to any feature work, but happy to review/guide if someone wants to claim this one!

[tauheedul]

Happy to take a PR to enable any of the steps I listed above :) I don't have time currently to commit to any feature work, but happy to review/guide if someone wants to claim this one!

Hello @twsouthwick I am unable to allocate sufficient time to fulfill the suggested design, however I have created a sample function that can assist anyone who is able to pick up this enhancement.

/// <summary>
/// Remove Invalid unsupported characters for XML
/// </summary>
/// <param name="input">Cell value</param>
/// <returns>Sanitized text suitable for XML</returns>
private string SanitizedTextForXML(string input)
{
	string output = string.Empty;

	if (!string.IsNullOrEmpty(input))
	{
		string result = input.Replace("<", "&lt;")
							 .Replace(">", "&gt;")
							 .Replace("&", "&amp;")
							 .Replace("\"", "&quot;")
							 .Replace("\'", "&apos;")
							 .Replace("&amp;amp;", "&amp;") // replace escaped text applied twice (where the input value was already escaped)
							 .ToString();

		try
		{

			if (!IsValidXmlChars(result))
			{
				char[] validXmlChars = result.Where(ch => XmlConvert.IsXmlChar(ch)).ToArray();
				output = new string(validXmlChars); ;
			}
			else
			{
				output = result;
			}

		}
		catch (XmlException ex)
		{
			Console.WriteLine("SanitizedTextForXML XmlException: " + ex.Message + " - " + ex.StackTrace);
		}
		catch (Exception ex)
		{
			Console.WriteLine("SanitizedTextForXML Exception: " + ex.Message + " - " + ex.StackTrace);
		}
	}

	return output;
}

/// <summary>
/// Credit: https://stackoverflow.com/a/14912673/1165173
/// </summary>
/// <param name="input"></param>
/// <returns></returns>
private bool IsValidXmlChars(string input)
{
	try
	{
		if (!string.IsNullOrEmpty(XmlConvert.VerifyXmlChars(input)))
                            {
			return true;
                            }
	}
	catch (Exception ex)
	{
		Console.WriteLine("IsValidXmlChars Exception: " + ex.Message + " - " + ex.StackTrace);
	}
	return false;
}

[twsouthwick]

Thanks @tauheedul! I'll leave this here for anyone that wants to take it on.

Note: Another example would be from WCF that uses a bit more performant way of doing it. It would be great to use SearchValues for the .NET 8 build of this to optimize it even more.

[Nick-HK]

@twsouthwick

I have a quick question before I proceed with creating a PR for the initial task. Would it be more appropriate to create the internal static class for XmlWriter under /src/common, or should it be nested under DocumentFormat.OpenXml.Framework?

This function will be required for my upcoming project, and I’m eager to make my first contribution to this issue. I look forward to your guidance and suggestions.

[twsouthwick]

Do it in the framework project. The common is mostly for poly fills to support old. Net versions. Thanks for jumping in for contributing!

[Nick-HK]

Hi team, I have revised our current creation of the XmlWriter based on the suggestion from @twsouthwick. I might properly create a XmlWriterFactory class to share the XmlWriter creation. Something like this maybe? Not sure if that's the best way to start with but feel free to comment/contirbute!

public static class XmlWriterFactory
{
    public static XmlWriter Create(/* parameters as needed */)
    {
        // Create and return an XmlWriter instance?
    }
}

[twsouthwick]

That's fine to start with, but make it internal. For public consumption, I prefer to stay away from static things and use https://learn.microsoft.com/en-us/office/open-xml/general/features.

For example:

public interface IXmlWriterFeature
{
  XmlWriter Create(XmlWriterSettings? settings = null);
}

internal static class XmlWriterExtensions
{
  public static XmlWriter CreateXmlWriter(this OpenXmlPartContainer container, XmlWriterSettings? settings = null)
    => container.Features.GetRequiredFeature<IXmlWriterFeature>().Create(settings);

Implementing the feature collection for OpenXmlpartcontainer with IXmlWriterFeature should allow this to light up while still being overrideable if needed.

As I mentioned above, feel free to first work on getting things extracted so that the writers are all created in the right place, then we can figure out what the public API should look like.