YamlDotNet icon indicating copy to clipboard operation
YamlDotNet copied to clipboard

Invalid key indicator format after upgrade to 12.0.0

Open lahma opened this issue 2 years ago • 2 comments

Describe the bug

Following YAML checks out as valid in online parsers:

es5id: 11.4.7-4-1
description: -"" should be zero

but after upgrade from 11.2.1 to 12.0.0 it started to fail with message:

YamlDotNet.Core.SemanticErrorException
Invalid key indicator format.
   at YamlDotNet.Core.Parser.ParseNode(Boolean isBlock, Boolean isIndentlessSequence) in C:\Work\YamlDotNet\YamlDotNet\Core\Parser.cs:line 462
   at YamlDotNet.Core.Parser.ParseBlockMappingValue() in C:\Work\YamlDotNet\YamlDotNet\Core\Parser.cs:line 833
   at YamlDotNet.Core.Parser.StateMachine() in C:\Work\YamlDotNet\YamlDotNet\Core\Parser.cs:line 158
   at YamlDotNet.Core.Parser.MoveNext() in C:\Work\YamlDotNet\YamlDotNet\Core\Parser.cs:line 107
   at YamlDotNet.Core.ParserExtensions.Consume[T](IParser parser) in C:\Work\YamlDotNet\YamlDotNet\Core\ParserExtensions.cs:line 43

To Reproduce

[Fact]
public void InvalidKeyIndicatorFormat()
{
    var yamlStream = new YamlStream();
    yamlStream.Load(new StringReader("description: -\"\" should be zero"));
}

Possible fix

Removing \" from below check seems to fix the issue, but not sure something else will be broken...

https://github.com/aaubry/YamlDotNet/blob/fd6731d4ab6bb4f3bec79a47b9ecb60f0d8e415f/YamlDotNet/Core/Scanner.cs#L401-L404

lahma avatar Jul 30 '22 10:07 lahma

Created a fix candidate: https://github.com/aaubry/YamlDotNet/pull/710

lahma avatar Jul 30 '22 12:07 lahma

Sorry for the delay. I’m not at a computer right now, but I think a better fix would be to only fall into the block if statement is if it is followed by a space or line break or null and then remove this check altogether. Because I suspect using single quotes, or any of those characters, would also result in the same error. I’ll try and look at that in a couple of hours.

EdwardCooke avatar Aug 07 '22 13:08 EdwardCooke

I appologize for the really long delay, had some family stuff to deal with. Anyways, the check should only be for the following characters ,[]{}. If you update your PR with that then I'll merge it in.

EdwardCooke avatar Sep 08 '22 16:09 EdwardCooke

Hey no sweat, I get what I'm paying for 😉 Thanks for checking, I'll add this to my todo list.

lahma avatar Sep 08 '22 17:09 lahma