System.NotSupportedException with specific svg file

[mrbean-bremen]

This is a spin-off of #59, as this part is not related to the original issue.

@dillanmann commented on 6 Sep 2017:
The version I am using is version 2.2.1.39233, the latest version available via NuGet.

The code that is throwing the error is:

        public static void RenderSvgStringToPng(XDocument xmlDoc, string outputFile)
        {
            var innerXml = xmlDoc.ToString();
            using (var xmlStream = new MemoryStream(Encoding.Default.GetBytes(innerXml)))
            {
                xmlStream.Position = 0;
                var doc = SvgDocument.Open<SvgDocument>(xmlStream); // <---- throwing here
                using (var image = doc.Draw())
                    image.Save(outputFile, ImageFormat.Png);
            }
        }

Note that it throws the error at SvgDocument.Open regardless of whether I just give it the plain SVG string (from xmlDoc.ToString() using SvgDocument.FromSvg) or whether I use the memory stream way.

The error I get is:

An exception of type 'System.NotSupportedException' occurred in Svg.dll but was not handled in user code

Additional information: Must call AddTerm AddSeparator in that order

I noticed some first chance exceptions logged:

A first chance exception of type 'System.ArgumentNullException' occurred in Svg.dll
iisexpress.exe Warning: 0 : Attribute 'd' cannot be set - type 'Svg.Pathing.SvgPathSegmentList' cannot convert from string ''.
A first chance exception of type 'System.NotSupportedException' occurred in Svg.dll

This is all the detail I could find, let me know anything else you need! :)

@gvheertum commented on 6 Sep 2017:
@dillanmann no worries :) Thanks for the details.

From what I can see it fails on something in the SVG rather than crashing on threading issues. Nevertheless, if there is something making the SVG invalid, it should fail with more grace than a nullpointerexceptions. From what I see now (after a quick glance at the problem) is it tries to make a path, but the property is empty (iisexpress.exe Warning: 0 : Attribute 'd' cannot be set - type 'Svg.Pathing.SvgPathSegmentList' cannot convert from string ''.).

I'm not at the machine where I can test this, but I'll try to look into this when I am getting back at my PC and see if I can reproduce this and try to pinpoint what exactly goes wrong.

@gvheertum commented on 6 Sep 2017:
Checking the SVG source I noticed:

<path class="plot-now" d="" style="opacity: 0;"></path>

Can you try what happens if you remove that element (or provide a d value)? I think this might be the culprit.

However, Chrome and other tools do accept this tag, so the SVG generator should not fail on this one. But maybe removing this element might be a temporary fix for your problem :)

@dillanmann commented on 6 Sep 2017:
Have tried removing the element but it unfortunately still gives me the same error, except now the first exception (iisexpress.exe Warning: 0 : Attribute 'd' cannot be set - type 'Svg.Pathing.SvgPathSegmentList' cannot convert from string ''.) is no longer present in the output log.

Not sure how much use it'll be but the stack trace for the error (beyond my code) is:

   ExCSS.TermList.AddSeparator(GrammarSegment termSepertor) in C:\Dev\vvvv\SVG\Source\External\ExCSS\Model\Values\TermList.cs:62
   ExCSS.Parser.ParseSingleValue(Block token) in C:\Dev\vvvv\SVG\Source\External\ExCSS\Parser.Blocks.cs:389
   ExCSS.Parser.ParseHexValue(Block token) in C:\Dev\vvvv\SVG\Source\External\ExCSS\Parser.Blocks.cs:485
   ExCSS.Parser.ParseTokenBlock(Block token) in C:\Dev\vvvv\SVG\Source\External\ExCSS\Parser.Blocks.cs:103
   ExCSS.Parser.Parse(String css) in C:\Dev\vvvv\SVG\Source\External\ExCSS\Parser.cs:64
   Svg.SvgDocument.Open(XmlReader reader) in C:\Dev\vvvv\SVG\Source\SvgDocument.cs:335
   Svg.SvgDocument.Open(Stream stream, Dictionary`2 entities) in C:\Dev\vvvv\SVG\Source\SvgDocument.cs:237
   Svg.SvgDocument.Open(Stream stream) in C:\Dev\vvvv\SVG\Source\SvgDocument.cs:196

@dillanmann commented on 6 Sep 2017:
Sorry for the reply spam - From some testing if I remove the entire 'style' element it will work, but obviously the styling is lost.

I then tried adding the style back in but removing the comments (in case something from the SASS -> CSS generation was causing the issue) but this didn't work.

At least I've managed to tie this down somewhere but don't really understand why the style would be causing the issue. Will keep debugging and let you know, but would appreciate any input you've got too!

@gvheertum commented on 6 Sep 2017:
I don't mind the messages (they are not spammy to me), this particular issue caught my interest so the more info the better. Also, the SVG library helped me quite a bit in the past, I would love to give back to the community :)

Probably the error regarding the empty string was thrown, but handled somewhere in the code (first chance exceptions are not always that bad it seems). It could have been an indicator, but for now it doesn't seem to be the cause of the issues you are experiencing. The trace you posted shows the error occurs in the parsing of the CSS, this makes sense, because you said that removing the style stuff gets rid of the error and bringing it back also returns the error.

I noticed in the style-tag that there are > selectors used in the CSS, which are escaped to > tags in the output, Chrome doesn't seem to care, but perhaps these are causing issues for the SVG library. If I find and replace these in the raw SVG you provided (> -> >), the SVG still is rendered correctly in Chrome. So perhaps trying that can solve the issue you are experiencing.

I am wondering if the > notation in styling is something that is allowed by the standard (CSS and/or SVG) or that Chrome is just being friendly/helpful (Chrome sometimes allows/fixes stuff that is out-of-standard, which is helpful when you are having files with this, but really unhelpful if you want to validate behaviour). If it's allowed by the standards, it might be worth fixing it.

@dillanmann commented on 6 Sep 2017:
I really appreciate all the effort you're going to here :)

In regards to the escaped > characters, I've actually got some unit tests to try and make it a bit easier to replicate this because I had the same idea. I essentially stripped out all of the css and just placed in potentially erroneous parts but the error was not thrown when I added in some css with the escaped characters in it.

@gvheertum commented on 6 Sep 2017:
I went a bit deeper and I think I found something. There are some elements in the styling that cause issues. I also added some test cases and found that indeed switching > and > doesn't matter and indeed removing all styling yields no exception.

Upon closer inspection of the error I found that the Message: Test method Svg.UnitTests.CssParsingIssue_Issue59.LoadSvg_TestComplexDocumentWithRawChars_YieldsResult threw exception: System.NotSupportedException: Must call AddTerm AddSeparator in that order is caused by the following statement in #footer:

border-top: 1px solid #009c46 !important;

The problem seems to be the !important element, if I remove this, the above error is no longer shown, however it will fail on other issues, being that pseudo-selectors like :hover and :before are not allowed.

That the !important causes issues is a bug of some sort, since in other elements the !important tag is not causing errors, but in the border-top situation is seems to fail when parsing the value. I'm going to check if I can roll a small fix for this. For the psuedo selectors I'm not sure if we should allow them or how to handle them.

You seem to attach the whole CSS of a website to the image, this makes the CSS to parse quite big, but also introduces elements in the CSS which are not really needed by the SVG (or better said, might even confuse the parser). Regardless of the fix, I would suggest splitting the CSS for the image from the other CSS and server that to the parser if possible.

@dillanmann commented on 6 Sep 2017:
Nice that we've found a resolution! :)

I was speaking with the front end dev on my team about this today, the reason the whole site css is includes is because we recently moved to using SASS/LESS and so all of the individual SCSS files are compiled down into a single css file rather than having a site wide and a plot specific one like we had before. Thankfully, I can rectify this fairly easily!

Thanks so much for your help, always nice to see folks willing and able to help out.

I have another issue with text not rendering as expected, but will report this in the appropriate issue / create one if it doesn't exist.

Again, thanks for your help :)

@gvheertum commented on 7 Sep 2017:
Good to hear you are happy and seem to have fixed the issue.

There seems to be something really awkward with the parsing of the border properties. I played around with it a bit more and found that

border-top: 1px solid  #009c46  ;

failes with the same error Svg.UnitTests.CssParsingIssue_Issue59.LoadSvg_CssParserCanHandleDoubleWhitespace_YieldsResult threw exception: System.NotSupportedException: Must call AddTerm AddSeparator in that order while

border-top: 1px solid #009c46;

doesn't seem to cause issues.

So the whitespacing seems to cause issues here. My best guess is that the !important is ignored and results in a floating whitespace in the CSS definition, which in his turn borks the parser... I did dive into the parser/lexer (result) of the css and found that indeed some additional whitespace is inserted in the list of tokens for this tag (in case of the extra whitespace and in case of the !important). This behavior does not seem to apply for all tags/definitions (since !important in other parts of your css does not yield any issues.

If I look closely at the result of the lexer in the error case, it seemed to only have parsed the 1px solid and not (yet) the color part. I'm going to look a bit deeper into it, because I am really curious why this happens.

[gvheertum]

I found the SVG we tested with back then (was buried somewhere in #59). Dillan posted the XML to pastebin and I did some testing on that file and was able to reproduce the issue. The original pastebin can be found here.

I'll try to isolate the issue into a unittest and try adding it to the testsuite (and of-course I'll try to find a fix 😉).

[mrbean-bremen]

That was fast - thanks! A test (and a fix of course) would be great!

[gvheertum]

I added 2 tests in a local branch to test both issues (crash on d="" and the crash of the style parser on the SVG file provided by Dillan).

Did some testing and playing around and found that the d="" is no longer an issue (not sure if I fixed it back then or whether the issue was resolved by something else), but the parser still seems to fail on the example file provided by Dillan. Since I am able to reproduce this issue in a test, this means I can try to find a fix.

[mrbean-bremen]

Thanks - even if you don't find a fix, the test file and your findings will be helpful.

[gvheertum]

I think I found what's happening. Somehow the !important is not seen/counted as a term (which makes sense), but the separating space is counted in the Term/Separator list (validated when adding Separators and Terms). This means that (according to the system) there is a Separator too many, which causes the fail with the AddTerm AddSeparator message. I did some testing and found some interesting results:

.footer { border-top: 1px solid #333333 !important; /*Fails*/ border-top: 1px solid #333333 ; /*Fails*/ }

.footer { border-top: 1px solid red !important; /*Success*/ border-top: 1px solid #333333; /*Success*/ border-top: 1px solid #333333; /*Success*/ }

There is some difference between the handling of the hex and the non hex-values (the hex elements seem to work with a buffer and keeping their context seems to cause the observed behaviour on whitespace (or Non-Term) elements after a hex code.

I did some poking around and don't have a correct fix yet, I had a fix for that made my tests pass, but other tests are failing (output wise), so I need to do some additional testing/coding.

For now the conclusion is, that using the !important and/or whitespace characters after a hex value will cause issues in the css parsing, resulting in the behaviour described in the initial problem.

[mrbean-bremen]

Ok, thanks, that doesn't sound like an easy fix, but you definitely have narrowed the problem down.
Are the tests you are writing about unit tests or image comparison tests? We definitely could do with more tests, even if you don't find a fix.

[gvheertum]

For now the tests I added are unit-tests (parse an example XML and expect no error) to see if the exception is triggered and they are quite quick and dirty (so I need to do some refactoring). I'm also using the existing set of image comparison tests to see if my changes break any of the existing tests (I did some changes in the hex parsing and that broke a large amount of the SVG compatibility checks).

If I encounter any testable stuff in my path to success I'll try adding test-cases (for example, I'm thinking about a test to check behaviour if the hex color code is invalid).

[mrbean-bremen]

@gvheertum - any luck with this one?

[gvheertum]

@mrbean-bremen Sadly, not really in light of a fix, but I was able to get it isolated and have some testing around it. I created a PR for those changes, so if anybody feels adventurous they can have a go at it ;)

I did some tinkering in the lexer, but with some of the changes that solved my case I often broke something else (so glad there are many tests also on the outputted files). So I did not find a solution I was comfortable with yet (so no fix in the PR sadly). I really hope to get some time to do some additional work, but unfortunately I'm a bit swamped.

[mrbean-bremen]

@gvheertum - thanks a lot for that! That will help to fix the issue - maybe I will have a go later...
I added a few comments to the PR, so you can adapt the code, or I can do this, if you don't have the time - just let me know.

[Lassesen]

As a FYI, I got the same error message and tracked down the issue to an issue in the CSS

I had
stroke-width: 0.1.5px;

So if you encounter this issue, start swapping out the css contents until you find the offender(s)