Skip to content

Commit

Permalink
Fix incorrect "illegal XML comment chars" check (#74787)
Browse files Browse the repository at this point in the history
Also adds regression tests
  • Loading branch information
GrabYourPitchforks authored Aug 30, 2022
1 parent 2ed931d commit f825814
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ public override void WriteComment(string? text)
{
try
{
if (null != text && (text.Contains("--") || text.StartsWith('-')))
if (null != text && (text.Contains("--") || text.EndsWith('-')))
{
throw new ArgumentException(SR.Xml_InvalidCommentChars);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@
<Compile Include="WriteWithEncoding.cs" />
<Compile Include="WriteWithEncodingWithFallback.cs" />
<Compile Include="WriteWithInvalidSurrogate.cs" />
<Compile Include="XmlTextWriterTests.cs" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.IO;
using Xunit;

namespace System.Xml.Tests
{
public class XmlTextWriterTests
{
public static IEnumerable<object[]> PositiveTestCases
{
get
{
yield return new string[] { null }; // will be normalized to empty string
yield return new string[] { "" };
yield return new string[] { "This is some data." };
yield return new string[] { " << brackets and whitespace >> " }; // brackets & surrounding whitespace are ok
yield return new string[] { "&amp;" }; // entities are ok (treated opaquely)
yield return new string[] { "Hello\r\nthere." }; // newlines are ok
yield return new string[] { "\U0001F643 Upside-down smiley \U0001F643" }; // correctly paired surrogates are ok
yield return new string[] { "\uFFFD\uFFFE\uFFFF" }; // replacement char & private use are ok
}
}

public static IEnumerable<object[]> BadSurrogateTestCases
{
get
{
yield return new string[] { "\uD800 Unpaired high surrogate." };
yield return new string[] { "\uDFFF Unpaired low surrogate." };
yield return new string[] { "Unpaired high surrogate at end. \uD800" };
yield return new string[] { "Unpaired low surrogate at end. \uDFFF" };
yield return new string[] { "Unpaired surrogates \uDFFF\uD800 in middle." };
}
}

[Theory]
[MemberData(nameof(PositiveTestCases))]
[InlineData("]]")] // ]] without trailing > is ok
[InlineData("-->")] // end of comment marker ok (meaningless to cdata tag)
public void WriteCData_SuccessCases(string cdataText)
{
StringWriter sw = new StringWriter();
XmlTextWriter xw = new XmlTextWriter(sw);

xw.WriteCData(cdataText);

Assert.Equal($"<![CDATA[{cdataText}]]>", sw.ToString());
}

[Theory]
[MemberData(nameof(BadSurrogateTestCases), DisableDiscoveryEnumeration = true)] // disable enumeration to avoid test harness misinterpreting unpaired surrogates
[InlineData("]]>")] // end of cdata marker forbidden (ambiguous close tag)
public void WriteCData_FailureCases(string cdataText)
{
StringWriter sw = new StringWriter();
XmlTextWriter xw = new XmlTextWriter(sw);

Assert.Throws<ArgumentException>(() => xw.WriteCData(cdataText));
}

[Theory]
[MemberData(nameof(PositiveTestCases))]
[InlineData("-12345")] // hyphen at beginning is ok
[InlineData("123- -45")] // single hyphens are ok in middle
[InlineData("]]>")] // end of cdata marker ok (meaningless to comment tag)
public void WriteComment_SuccessCases(string commentText)
{
StringWriter sw = new StringWriter();
XmlTextWriter xw = new XmlTextWriter(sw);

xw.WriteComment(commentText);

Assert.Equal($"<!--{commentText}-->", sw.ToString());
}

[Theory]
[MemberData(nameof(BadSurrogateTestCases), DisableDiscoveryEnumeration = true)] // disable enumeration to avoid test harness misinterpreting unpaired surrogates
[InlineData("123--45")] // double-hyphen in middle is forbidden (ambiguous comment close tag)
[InlineData("12345-")] // hyphen at end is forbidden (ambiguous comment close tag)
[InlineData("-->")] // end of comment marker forbidden (ambiguous close tag)
public void WriteComment_FailureCases(string commentText)
{
StringWriter sw = new StringWriter();
XmlTextWriter xw = new XmlTextWriter(sw);

Assert.Throws<ArgumentException>(() => xw.WriteComment(commentText));
}
}
}

0 comments on commit f825814

Please sign in to comment.