HighlightingLoader.Load(XmlReader) mis-parses non-normalizing readers (e.g. XDocument.CreateReader): silently empty main RuleSet, or misleading "Could not find main RuleSet." #454

New issue

Open

opened 2026-09-18 02:51:28 -05:00 by GRNBY · 0 comments

GRNBY commented

2026-09-18 02:51:28 -05:00

(Migrated from github.com)

Summary

HighlightingLoader.Load(XmlReader, IHighlightingManager) implicitly requires the caller's reader to report insignificant whitespace as XmlNodeType.Whitespace. When it does not — XDocument.CreateReader() (i.e. XDocument / XElement, public API) is the easy way to hit this — the hand-written parser in V2Loader desynchronizes and either returns a definition whose main RuleSet is empty (no exception, Rules.Count == 0, highlighting silently does nothing), or throws HighlightingDefinitionInvalidException: Could not find main RuleSet.

Which one you get depends on the number of whitespace runs / XML comments in the file: adding or removing a single  flips the outcome, even though comments are never parsed. The input is fixed, yet the result looks random.

Minimal reproduction

Valid .xshd (loads correctly on its own):

<?xml version="1.0" encoding="utf-8" ?>
<SyntaxDefinition name="CustomVariable Highlighting"
                  xmlns="http://icsharpcode.net/sharpdevelop/syntaxdefinition/2008">
  <!-- this comment is irrelevant to the parser, but changes the outcome -->
  <RuleSet>
    <Keywords foreground="Brown">
      <Word>[V]</Word>
    </Keywords>
    <Rule foreground="Brown" fontWeight="bold">\[(Aux_V_|Aux_T_)([1-9]|[1-5][0-9]|6[0-4])\]</Rule>
  </RuleSet>
</SyntaxDefinition>

// A) WORKS: Rules.Count == 3
using (Stream s = asm.GetManifestResourceStream("...CustomVariableHighlighting.xshd"))
using (XmlReader r = new XmlTextReader(s))
    def = HighlightingLoader.Load(r, HighlightingManager.Instance);

// B) BROKEN: Rules.Count == 0, no exception
//    (with one extra <!-- ... --> in the file: throws "Could not find main RuleSet.")
using (Stream s = asm.GetManifestResourceStream("...CustomVariableHighlighting.xshd"))
{
    XDocument doc = XDocument.Load(new XmlTextReader(s));
    def = HighlightingLoader.Load(doc.CreateReader(), HighlightingManager.Instance);
}

Identical bytes, only the reader differs: A → 3 rules, B → 0.

Root cause

XDocument.CreateReader() returns an XNodeReader that reports whitespace-only text as XmlNodeType.Text, not Whitespace. A node dump shows one such node per indentation run (d1 Text <> NS=[] val=[\r\n\t], d2 Text <> NS=[] val=[\r\n\t\t], …); with XmlTextReader the same document produces zero such nodes.
GetValidatingReader wraps the caller's reader with XmlReaderSettings { IgnoreComments = true, IgnoreWhitespace = true, ValidationType = Schema, Schemas = ModeV2 }. IgnoreWhitespace discards only nodes whose NodeType is Whitespace, so for a non-normalizing reader it is a no-op and every indentation run reaches the parser. (IgnoreComments does drop comments — but each comment splits the surrounding whitespace into two text runs, which is exactly why the comment count changes the outcome.)
V2Loader.ParseElements calls reader.Skip() for every node whose NamespaceURI != Namespace, inside while (reader.Read() && reader.NodeType != XmlNodeType.EndElement). Text nodes have an empty namespace, so every whitespace node takes the Skip() branch; Skip() on a non-element node is just a Read(), and the loop Read()s again at the top of the next iteration. Two node positions are consumed per iteration, so element start tags are stepped over and never become the current node (they are only "hit" when the parity happens to line up).
Consequences:
- parity "hits" <RuleSet>, but all children are consumed as text siblings → XshdRuleSet.Elements empty → main RuleSet with zero rules → no exception, no highlighting;
- one extra whitespace/comment node shifts parity by one → the <RuleSet> start tag itself is stepped over → no unnamed RuleSet in def.Elements → throw new HighlightingDefinitionInvalidException("Could not find main RuleSet.").

Single-step trace (V2Loader's state machine replicated verbatim, run on the wrapped reader of case B):

ParseDefinition at Element <SyntaxDefinition> d0
<def.Elements>: enter ParseElements
  NS-MISMATCH -> Skip: Text <> d1 val=[\r\n\t]
  PARSE RuleSet: Element <RuleSet> d1
    <RuleSet.Elements>: enter ParseElements
      NS-MISMATCH -> Skip: Text <> d2 val=[\r\n\t\t]
      NS-MISMATCH -> Skip: Text <> d3 val=[\r\n\t\t\t]
      ...
      <RuleSet.Elements> loop EXIT at EOF -> parsed 0 V2 elements
<def.Elements> loop EXIT at EOF -> parsed 1 V2 elements

With one extra  in the same file the PARSE RuleSet line never appears and the log ends with <def.Elements> ... parsed 0 → the exception.

Why this looks like a library-side bug

The Load(XmlReader) contract is not documented as "the reader must normalize whitespace", and returning Text for whitespace-only content is legal for an XmlReader — XNodeReader does exactly that.
One branch fails silently (0 rules, no exception), so users only see "highlighting doesn't work"; the other blames an innocent "main RuleSet" that is present in the file.
The outcome changes when an XML comment is added. A parser should be immune to that — this is what made the bug so expensive to find.

Suggested fixes (any one closes it)

Only skip element nodes in the namespace check:

if (reader.NodeType == XmlNodeType.Element && reader.NamespaceURI != Namespace)
{
    if (!reader.IsEmptyElement) reader.Skip();
    continue;
}

A non-element node then falls through the loop and costs only the single Read() it is supposed to cost, keeping alignment intact.

Or harden GetValidatingReader: filter whitespace-only Text/SignificantWhitespace nodes coming from the inner reader (a small delegating XmlReader). The parser only cares about significant content, so that is the safest place to normalize.
Or improve diagnostics: when no unnamed RuleSet is found, report the parsed element count and hint that the input reader must normalize whitespace (or fall back to the first RuleSet with a warning). The current message sends users hunting in the wrong place.

Workaround for anyone hitting this

var ms = new MemoryStream();
doc.Save(ms);
ms.Position = 0;
using (var reader = new XmlTextReader(ms))
    definition = HighlightingLoader.Load(reader, HighlightingManager.Instance);

Verified: 3 rules for the file above, 4 rules after appending a rule via XElement.Add.

Happy to attach the standalone repro harness (a small .cs that dumps both node streams and runs the replicated state machine) and the raw logs. Thanks for the library — this is the only issue we have hit in years of using it.

### Summary `HighlightingLoader.Load(XmlReader, IHighlightingManager)` implicitly requires the caller's reader to report insignificant whitespace as `XmlNodeType.Whitespace`. When it does not — `XDocument.CreateReader()` (i.e. `XDocument` / `XElement`, public API) is the easy way to hit this — the hand-written parser in `V2Loader` desynchronizes and either returns a definition whose main `RuleSet` is **empty** (no exception, `Rules.Count == 0`, highlighting silently does nothing), or throws `HighlightingDefinitionInvalidException: Could not find main RuleSet.` Which one you get depends on the number of whitespace runs / XML comments in the file: adding or removing a single `` flips the outcome, even though comments are never parsed. The input is fixed, yet the result looks random. ### Minimal reproduction Valid `.xshd` (loads correctly on its own): ```xml <?xml version="1.0" encoding="utf-8" ?> <SyntaxDefinition name="CustomVariable Highlighting" xmlns="http://icsharpcode.net/sharpdevelop/syntaxdefinition/2008">  <RuleSet> <Keywords foreground="Brown"> <Word>[V]</Word> </Keywords> <Rule foreground="Brown" fontWeight="bold">\[(Aux_V_|Aux_T_)([1-9]|[1-5][0-9]|6[0-4])\]</Rule> </RuleSet> </SyntaxDefinition> ``` ```csharp // A) WORKS: Rules.Count == 3 using (Stream s = asm.GetManifestResourceStream("...CustomVariableHighlighting.xshd")) using (XmlReader r = new XmlTextReader(s)) def = HighlightingLoader.Load(r, HighlightingManager.Instance); // B) BROKEN: Rules.Count == 0, no exception // (with one extra  in the file: throws "Could not find main RuleSet.") using (Stream s = asm.GetManifestResourceStream("...CustomVariableHighlighting.xshd")) { XDocument doc = XDocument.Load(new XmlTextReader(s)); def = HighlightingLoader.Load(doc.CreateReader(), HighlightingManager.Instance); } ``` Identical bytes, only the reader differs: A → 3 rules, B → 0. ### Root cause 1. **`XDocument.CreateReader()` returns an `XNodeReader` that reports whitespace-only text as `XmlNodeType.Text`**, not `Whitespace`. A node dump shows one such node per indentation run (`d1 Text <> NS=[] val=[\r\n\t]`, `d2 Text <> NS=[] val=[\r\n\t\t]`, …); with `XmlTextReader` the same document produces **zero** such nodes. 2. **`GetValidatingReader` wraps the caller's reader with `XmlReaderSettings { IgnoreComments = true, IgnoreWhitespace = true, ValidationType = Schema, Schemas = ModeV2 }`.** `IgnoreWhitespace` discards only nodes whose `NodeType` is `Whitespace`, so for a non-normalizing reader it is a no-op and every indentation run reaches the parser. (`IgnoreComments` does drop comments — but each comment splits the surrounding whitespace into **two** text runs, which is exactly why the comment count changes the outcome.) 3. **`V2Loader.ParseElements` calls `reader.Skip()` for every node whose `NamespaceURI != Namespace`**, inside `while (reader.Read() && reader.NodeType != XmlNodeType.EndElement)`. Text nodes have an empty namespace, so every whitespace node takes the `Skip()` branch; `Skip()` on a non-element node is just a `Read()`, and the loop `Read()`s again at the top of the next iteration. **Two node positions are consumed per iteration, so element start tags are stepped over and never become the current node** (they are only "hit" when the parity happens to line up). 4. Consequences: - parity "hits" `<RuleSet>`, but all children are consumed as text siblings → `XshdRuleSet.Elements` empty → main RuleSet with zero rules → **no exception, no highlighting**; - one extra whitespace/comment node shifts parity by one → the `<RuleSet>` start tag itself is stepped over → no unnamed RuleSet in `def.Elements` → `throw new HighlightingDefinitionInvalidException("Could not find main RuleSet.")`. Single-step trace (V2Loader's state machine replicated verbatim, run on the wrapped reader of case B): ``` ParseDefinition at Element <SyntaxDefinition> d0 <def.Elements>: enter ParseElements NS-MISMATCH -> Skip: Text <> d1 val=[\r\n\t] PARSE RuleSet: Element <RuleSet> d1 <RuleSet.Elements>: enter ParseElements NS-MISMATCH -> Skip: Text <> d2 val=[\r\n\t\t] NS-MISMATCH -> Skip: Text <> d3 val=[\r\n\t\t\t] ... <RuleSet.Elements> loop EXIT at EOF -> parsed 0 V2 elements <def.Elements> loop EXIT at EOF -> parsed 1 V2 elements ``` With one extra `` in the same file the `PARSE RuleSet` line never appears and the log ends with `<def.Elements> ... parsed 0` → the exception. ### Why this looks like a library-side bug - The `Load(XmlReader)` contract is not documented as "the reader must normalize whitespace", and returning `Text` for whitespace-only content is legal for an `XmlReader` — `XNodeReader` does exactly that. - One branch fails **silently** (0 rules, no exception), so users only see "highlighting doesn't work"; the other blames an innocent "main RuleSet" that *is* present in the file. - The outcome changes when an **XML comment** is added. A parser should be immune to that — this is what made the bug so expensive to find. ### Suggested fixes (any one closes it) 1. Only skip **element** nodes in the namespace check: ```csharp if (reader.NodeType == XmlNodeType.Element && reader.NamespaceURI != Namespace) { if (!reader.IsEmptyElement) reader.Skip(); continue; } ``` A non-element node then falls through the loop and costs only the single `Read()` it is supposed to cost, keeping alignment intact. 2. Or harden `GetValidatingReader`: filter whitespace-only `Text`/`SignificantWhitespace` nodes coming from the inner reader (a small delegating `XmlReader`). The parser only cares about significant content, so that is the safest place to normalize. 3. Or improve diagnostics: when no unnamed `RuleSet` is found, report the parsed element count and hint that the input reader must normalize whitespace (or fall back to the first `RuleSet` with a warning). The current message sends users hunting in the wrong place. ### Workaround for anyone hitting this ```csharp var ms = new MemoryStream(); doc.Save(ms); ms.Position = 0; using (var reader = new XmlTextReader(ms)) definition = HighlightingLoader.Load(reader, HighlightingManager.Instance); ``` Verified: 3 rules for the file above, 4 rules after appending a rule via `XElement.Add`. Happy to attach the standalone repro harness (a small `.cs` that dumps both node streams and runs the replicated state machine) and the raw logs. Thanks for the library — this is the only issue we have hit in years of using it.

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-icsharpcode/AvalonEdit#454

No description provided.

Rows
Columns