Skip to content

Commit

Permalink
Fix nasty bug in spine recovery
Browse files Browse the repository at this point in the history
The tests were accidentally "cheating" by recovering from
the position before the actual target and catching up, so
did not trigger this bug.

Add a couple more granualar recovery tests to make it easier
to debug these kinds of issues.
  • Loading branch information
mhutch committed Sep 4, 2024
1 parent b4137f8 commit c7b108f
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 9 deletions.
15 changes: 12 additions & 3 deletions Core.Tests/Parser/ParsingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,13 @@ public void SpineParserRecoveryXhtmlStrictSchema ()
using var sr = new StreamReader (ResourceManager.GetXhtmlStrictSchema ());
var docTxt = sr.ReadToEnd ();

SpineParserRecovery (docTxt, 1127391);
}

[TestCase("<eee>foo\\</eee>", 25)]
[TestCase("<eee bar=\"1\" baz=\"hello\" />", 130)]
public void SpineParserRecovery (string docTxt, int expectedDelta)
{
var rootState = CreateRootState ();
var treeParser = new XmlTreeParser (rootState);
foreach (char c in docTxt) {
Expand All @@ -530,7 +537,7 @@ public void SpineParserRecoveryXhtmlStrictSchema ()
char c = docTxt[i];
spineParser.Push (c);

var recoveredParser = XmlSpineParser.FromDocumentPosition (rootState, doc, i).AssertNotNull ();
var recoveredParser = XmlSpineParser.FromDocumentPosition (rootState, doc, spineParser.Position).AssertNotNull ();
var delta = i - recoveredParser.Position;
totalNotRecovered += delta;

Expand All @@ -542,12 +549,14 @@ public void SpineParserRecoveryXhtmlStrictSchema ()
AssertEqual (spineParser.GetContext (), recoveredParser.GetContext ());
}

/*
int total = docTxt.Length * docTxt.Length / 2;
float recoveryRate = 1f - totalNotRecovered / (float)total;
TestContext.WriteLine ($"Recovered {(recoveryRate * 100f):F2}%");
*/

// check it never regresses
Assert.LessOrEqual (totalNotRecovered, 1118088);
Assert.LessOrEqual (totalNotRecovered, expectedDelta);
}

[TestCase ("<r>\r\n<a e='v' />\r\n</r>", "<r>\r\n<a e='v' /\r\n</r>")]
Expand All @@ -574,7 +583,7 @@ public void SpineParserRecoverFromError (string docTxt, string recoverFromDoc)
char c = docTxt[i];
spineParser.Push (c);

var recoveredParser = XmlSpineParser.FromDocumentPosition (rootState, doc, Math.Min (i, maxCompat)).AssertNotNull ();
var recoveredParser = XmlSpineParser.FromDocumentPosition (rootState, doc, Math.Min (i, maxCompat));

var end = Math.Min (i + 1, docTxt.Length);
for (int j = recoveredParser.Position; j < end; j++) {
Expand Down
5 changes: 5 additions & 0 deletions Core/Parser/XmlProcessingInstructionState.cs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ public class XmlProcessingInstructionState : XmlParserState
parents.Push (new XProcessingInstruction (pi.Span.Start));
}

// still in RootState
if (length == 1) {
return null;
}

return new (
currentState: this,
position: position,
Expand Down
15 changes: 10 additions & 5 deletions Core/Parser/XmlSpineParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,15 @@ public XmlSpineParser (XmlParserContext context, XmlRootState rootState) : base
/// the parser is not guaranteed but will not exceed <paramref name="maximumPosition" />.
/// </summary>
/// <returns></returns>
public static XmlSpineParser? FromDocumentPosition (XmlRootState stateMachine, XDocument xdocument, int maximumPosition)
=> xdocument.FindAtOrBeforeOffset (maximumPosition) is XObject obj
&& stateMachine.TryRecreateState (ref obj, maximumPosition) is XmlParserContext ctx
? new XmlSpineParser (ctx, stateMachine)
: null;
public static XmlSpineParser FromDocumentPosition (XmlRootState stateMachine, XDocument xdocument, int maximumPosition)
{
// Recovery must be based on the node before the target position.
// The state for the node at the position won't be entered until the character at (i.e. after) the position is processed.
var node = maximumPosition == 0? xdocument : xdocument.FindAtOrBeforeOffset (maximumPosition - 1);
if (node is XObject obj && stateMachine.TryRecreateState (ref obj, maximumPosition) is XmlParserContext ctx) {
return new XmlSpineParser (ctx, stateMachine);
}
return new XmlSpineParser (stateMachine); ;
}
}
}
2 changes: 1 addition & 1 deletion Core/Parser/XmlTagState.cs
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ public XmlTagState (XmlAttributeState attributeState, XmlNameState nameState)
newEl.Attributes.AddAttribute ((XAttribute)att.ShallowCopy ());
continue;
}
if (att.Span.End > position) {
if (att.Span.End >= position) {
foundPosition = Math.Min (position, att.Span.Start);
break;
}
Expand Down

0 comments on commit c7b108f

Please sign in to comment.