Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix XmlException when writing Html log with certain test names #4576

Merged
merged 2 commits into from Jun 28, 2023

Conversation

danmoseley
Copy link
Member

@danmoseley danmoseley commented Jun 26, 2023

Fix #3136

Bug: In the repro case, the test metadata (display name here) contains characters like  that are not valid in XML (outside of CDATA perhaps) per spec:

https://www.w3.org/TR/2008/REC-xml-20081126/#NT-Char

[2] Char ::= #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x10000-#x10FFFF] /* any Unicode character, excluding the surrogate blocks, FFFE, and FFFF. */

The HtmlLogger serializes the TestResult to XML using DCS. DCS is tolerant of these invalid characters -- it will serialize as eg  and read that too. XmlTextWriter will by default serialize the same way. But in default configuration XmlReader will not read it, and throw XmlException causing the production of the Html log to fail.

Fix: set XmlWriterSettings.CheckCharacters to false.

I verified that this fixes the problem locally. I tried hard to make a unit test (see first commit) but concluded that it's not mockable using public API: HtmlTransformer is internal. I guess it could be mocked using reflection or we could use an end to end test instead? I don't have more time to spend, so perhaps it's acceptable without a test.

This reverts commit d7766ee99830598a7537d9126d7d23949ed37fa4.
@danmoseley
Copy link
Member Author

compare lines in original code:
https://github.com/dotnet/runtime/blob/b00ebc853fdb46425842cf6cae8186d65031e9ef/src/libraries/System.Private.Xml/src/System/Xml/Xslt/XslCompiledTransform.cs#L365-L366

was calling

        public void Transform(string inputUri, string resultsFile)
        {
            ArgumentNullException.ThrowIfNull(inputUri);
            ArgumentNullException.ThrowIfNull(resultsFile);

            // SQLBUDT 276415: Prevent wiping out the content of the input file if the output file is the same
            using XmlReader reader = XmlReader.Create(inputUri);
            using XmlWriter writer = XmlWriter.Create(resultsFile, OutputSettings);
            Transform(reader, null, writer, CreateDefaultResolver());

now calling

        public void Transform(XmlReader input, XmlWriter results)
        {
            ArgumentNullException.ThrowIfNull(input);
            ArgumentNullException.ThrowIfNull(results);

            Transform(input, null, results, CreateDefaultResolver());
        }

I believe OutputSettings above was null.

@nohwnd
Copy link
Member

nohwnd commented Jun 27, 2023

Thanks, will have a look tomorrow.

@nohwnd nohwnd merged commit f6517a0 into microsoft:main Jun 28, 2023
7 checks passed
@danmoseley danmoseley deleted the fix.invalid.char branch June 28, 2023 18:12
@danmoseley
Copy link
Member Author

danmoseley commented Jun 28, 2023

in case it's useful in future, here's the half attempt I had at a test, that I gave up on

-    [TestMethod]
-    public void TestCompleteHandlerShouldHandleInvalidCharReferences()
-    {
-        VisualStudio.TestPlatform.Extensions.HtmlLogger.HtmlLogger hl = new(_mockFileHelper.Object, new Mock<IHtmlTransformer>().Object, new DataContractSerializer(typeof(TestRunDetails)));
-        hl.Initialize(_events.Object, _parameters);
-
-        MemoryStream xmlStream = new();
-        _mockFileHelper.Setup(x => x.GetStream(It.IsAny<string>(), FileMode.Open, FileAccess.Read)).Returns(xmlStream);
-        _mockFileHelper.Setup(x => x.Exists(It.IsAny<string>())).Returns(false);
-
-        MemoryStream htmlStream = new();
-        _mockFileHelper.Setup(x => x.GetStream(It.IsAny<string>(), FileMode.OpenOrCreate, FileAccess.ReadWrite)).Returns(htmlStream);
-        _mockFileHelper.Setup(x => x.Exists(It.IsAny<string>())).Returns(false);
-
-        var testCase = new TestCase("TestName", new Uri("some://uri"), "TestSource");
-        var testResult = new ObjectModel.TestResult(testCase) { Outcome = TestOutcome.Failed };
-        testResult.Messages.Add(new(TestResultMessage.StandardErrorCategory, "\uFFFF"));
-
-        hl.TestResultHandler(new object(), new TestResultEventArgs(testResult));
-
-        hl.TestRunCompleteHandler(new object(), new TestRunCompleteEventArgs(null, false, true, null, null, null, TimeSpan.Zero));
-
-        //Assert.AreEqual(htmlFileContent, new StreamReader(htmlStream).ReadToEnd());
-    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Html Logger Error when running System.Text.RegularExpressions.Tests
2 participants