XmlException in Message

[kane-armstrong]

Screenshot of error attached.

Links.htmlMessage.Href:

%3chtml%3e%0d%0a%0d%0a%3chead%3e%0d%0a%0d%0a%0d%0a%3cstyle%3e%0d%0ap.MsoNormal%2c+li.MsoNormal%2c+div.MsoNormal+%7b%0amargin-top%3a0cm%3b%0amargin-right%3a0cm%3b%0amargin-bottom%3a8.0pt%3b%0amargin-left%3a0cm%3b%0aline-height%3a107%25%3b%0afont-size%3a11.0pt%3b%0afont-family%3a%22Calibri%22%2csans-serif%3b%0a%7d%0a%0a.MsoChpDefault+%7b%0afont-family%3a%22Calibri%22%2csans-serif%3b%0a%7d%0a%0a.MsoPapDefault+%7b%0amargin-bottom%3a8.0pt%3b%0aline-height%3a107%25%3b%0a%7d%0a%0adiv.WordSection1+%7b%0a%7d%0a%0d%0a%3c%2fstyle%3e%0d%0a%0d%0a%3c%2fhead%3e%0d%0a%0d%0a%3cbody+lang%3dEN-NZ+style%3d%22%22%3e%0d%0a%0d%0a%3cdiv+class%3dWordSection1%3e%0d%0a%0d%0a%3cp+class%3dMsoNormal+style%3d%22margin-bottom%3a0cm%3bmargin-bottom%3a.0001pt%3bline-height%3anormal%3btext-autospace%3anone%3b%22%3e%3cspan+lang%3dEN-AU+style%3d%22font-size%3a10.0pt%3bfont-family%3a%26quot%3bSegoe+UI%26quot%3b%2csans-serif%3bcolor%3ablack%3b%22%3ehello%3c%2fspan%3e%3c%2fp%3e%0d%0a%0d%0a%3c%2fdiv%3e%0d%0a%0d%0a%3c%2fbody%3e%0d%0a%0d%0a%3c%2fhtml%3e%0d%0a

[kane-armstrong]

I fixed this by parsing using regex instead of XDocument - see below. I would PR but I don't know how you feel about splitting this out into a utility class - I did it this way for testing purposes.

    public static class ParsingUtility
    {
        private static string SpanElementRegex = "<span[^>]*?>(.*?)</span>";

        /// <summary>
        /// Retrieves the content within an HTML span element. 
        /// </summary>
        /// <param name="element">A Url-encoded span element</param>
        /// <returns>The content within the element if matched, null otherwise.</returns>
        public static string GetSpanElementContent(string element)
        {
            if (string.IsNullOrEmpty(element))
            {
                return null;
            }

            var cleaned = WebUtility.UrlDecode(element);
            var regex = new Regex(SpanElementRegex);
            var match = regex.Match(cleaned);
            if (match.Success)
            {
                return match.Groups[1].Value;
            }
            return null;
        }
    }

Message.cs:

        [JsonIgnore]
        public string Text
        {
            get
            {
                if (!string.IsNullOrEmpty(text))
                {
                    return text;
                }
                if (Links.htmlMessage != null)
                {
                    var messageText = ParsingUtility.GetSpanElementContent(Links.htmlMessage?.Href.Split(',')[1]);
                    if (messageText == null)
                    {
                        throw new InvalidOperationException($"Message received not in expected format. Received: {Links.htmlMessage?.Href}. See https://msdn.microsoft.com/en-us/skype/ucwa/message_ref for more.");
                    }

                    return messageText;
                }
                if (Links.plainMessage != null)
                {
                    return WebUtility.UrlDecode(Links.plainMessage?.Href?.Split(',')[1]);
                }
                return "";
            }
            set => text = value;
        }

[baywet]

Hi @kane-armstrong Sorry for the delay and thanks for investigating. I think that's a valuable fix and it should be PR'ed, and yes I think a utility class at this point is ok, I don't know whether or not we'll need it somewhere else.
A couple of remarks:

• switch the Utility to internal
• make the private string for the regex, a regex right away, this way it'll be compiled by the regex engine only once and cached.
• in message.cs maybe instead of leveraging a local variable messageText, assign text right away implementing this way a lazy loading pattern
• simplifying the messageText to text will allow you to also leverage the ?? operator for the exception, simlifying the code a bit. Those are all the remarks I have so far but we can keep the discussion going on the PR :)

[kenakamu]

great point. I just want to avoid using the name 'utility' as much as possible. But other than that, totally agree.

[kane-armstrong]

Hi @baywet, thanks for the feedback. Sorry its a bit messy, I should have refactored it before posting!

Hi @kenakamu, I agree the name smells, I'll think of something cleaner when I PR.

[baywet]

@kenakamu what was the outcome of this one? Did you implement it yourself?

[kane-armstrong]

Oops, forgot about this one. Do you guys still want me to do it? @baywet Message.cs hasn't changed - I checked (here)

[baywet]

Yeah implement a PR with the recommendation we provided and submit it if you have time. Thanks!