XmlParser - Allow InnerText with greater-than character #5905
Conversation
WalkthroughThe changes update the XML parser's error handling logic to no longer throw an exception when encountering the '>' character in element values, only flagging an error for unencoded '<'. Additionally, new unit tests were added to verify parsing of XML elements containing expressions with the '>' character. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as XmlParserTests
participant Parser as XmlParser
Test->>Parser: Parse XML with element containing 'level >= LogLevel.Warn'
Parser->>Parser: ReadUntilChar processes value
alt Value contains '>'
Parser-->>Test: Successfully parses element value
else Value contains unencoded '<'
Parser-->>Test: Throws exception with encoding suggestion
end
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/NLog/Internal/XmlParser.cs (1)
493-494: Fix the XML entity syntax in the error message.The logic change to allow '>' characters in XML content is correct and aligns with the PR objective. However, the error message suggests incorrect XML entity syntax.
Apply this diff to fix the XML entity syntax:
- throw new XmlParserException($"Invalid XML document. Cannot parse value with '<', maybe encode to $lt;"); + throw new XmlParserException($"Invalid XML document. Cannot parse value with '<', maybe encode to <");The standard XML entity for '<' is
<, not$lt;.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/NLog/Internal/XmlParser.cs(1 hunks)tests/NLog.UnitTests/Internal/XmlParserTests.cs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/NLog/Internal/XmlParser.cs (1)
src/NLog/Config/XmlParserException.cs (4)
XmlParserException(41-68)XmlParserException(46-48)XmlParserException(54-57)XmlParserException(64-67)
🔇 Additional comments (1)
tests/NLog.UnitTests/Internal/XmlParserTests.cs (1)
300-300: Excellent test coverage for the '>' character parsing enhancement.The new test cases provide comprehensive coverage for the XML parser changes, testing both attribute values and element inner text scenarios with the '>' character in
level >= LogLevel.Warnexpressions. The tests are well-structured and align perfectly with the PR objective.Also applies to: 303-303
44ad943 to
09a43a6
Compare
|
Resolves #5903 - Followup to #5712