2
\$\begingroup\$

I have the following code which I have written due to the fact the Microsoft's Sanitizer is now to aggressive.

What I'm trying to do is as follows.

  1. Create a whitelist of HTML tags I want to keep
  2. After the tags have been converted, run the text through the sanitizer and remove any tags I do not require.
  3. After that, remove any tags that have either id's or classes inside them
  4. Finally convert the tags back.

My code is below, and it would be much appreciated if someone with more experience can have a look over it and give me some feedback.

string txt = "<h1>Keep as sanitizer removes</h1><p>keep</p><p class=\"remove\">remove class</p><br/><script>remove</script>";
            var whiteList = new List<Word>();
            whiteList.Add(new Word() { SearchWord = "<p>", ReplaceWord = "&lt;p&gt;" });
            whiteList.Add(new Word() { SearchWord = "</p>", ReplaceWord = "&lt;/p&gt;" });
            whiteList.Add(new Word() { SearchWord = "<h1>", ReplaceWord = "&lt;h1&gt;" });
            whiteList.Add(new Word() { SearchWord = "</h1>", ReplaceWord = "&lt;/h1&gt;" });
            whiteList.Add(new Word() { SearchWord = "<br/>", ReplaceWord = "&lt;br/&gt;" });

            whiteList.ForEach(w => txt = txt.Replace(w.SearchWord, w.ReplaceWord));

            var remove = Sanitizer.GetSafeHtmlFragment(txt);

            whiteList.ForEach(w => remove = remove.Replace(w.ReplaceWord, w.SearchWord));

            var again = RegexHelpers.StripHtmlAttributes(remove);

            var tt = again;



public static string StripHtmlAttributes(string s)
    {
        const string pattern = @"\s.+?=[""'].+?[""']";
        var result = Regex.Replace(s, pattern, string.Empty);
    return result;
}
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

You could simplify the declaration of the tags you want to keep, as it is very structured:

var whitelist = new List<string>(new string[] { "p", "h1", "br" });

foreach (var w in whitelist) {
  txt = txt.Replace("<" + w + ">", "&lt;" + w + "&gt;")
           .Replace("<" + w + "/>", "&lt;" + w + "/&gt;"));
}

var remove = Sanitizer.GetSafeHtmlFragment(txt);

foreach (var w in whitelist) {
  remove = remove.Replace("&lt;" + w + "&gt;", "<" + w + ">")
                 .Replace("&lt;" + w + "/&gt;", "<" + w + "/>"));
}

var again = RegexHelpers.StripHtmlAttributes(remove);

var tt = again;
\$\endgroup\$
0
3
\$\begingroup\$

Apparently, in general a regex isn't the right tool to parse HTML. I'm not sure why that is, but the fact that ...

<a href="foo" title="5>3"> Oops </a>

... is a valid tag might have something to do with it.

Therefore, to parse HTML, some people recommend you use a tool like Html Agility Pack.


Your code looks safe to me because of the way it's white-listing.

But some specific problems with it include:

  • You don't whitelist tags which have any attributes, e.g. <h1 id="foo">
  • Any &gt;h1&lt; in the original text will be converted to <h1>

To guard against the second problem, you might try to convert & to &amp; first, and then convert it back again.

\$\endgroup\$
4
  • \$\begingroup\$ Hi ChrisW thanks for reply, this section of code StripHtmlAttributes(string s) removes all elements from tags so if <h1 id="foo"> was entered, only <h1> would be returned Thanks \$\endgroup\$ Feb 10, 2014 at 14:42
  • \$\begingroup\$ It seems to me that you're not calling StripHtmlAttributes until after you call whiteList.ForEach(w => txt = txt.Replace ... and therefore your replace will fail to replace any whitelisted elements which contain an attribute. \$\endgroup\$
    – ChrisW
    Feb 10, 2014 at 14:47
  • \$\begingroup\$ Hi ChrisW thanks just tested again and I see what you mean, thanks \$\endgroup\$ Feb 10, 2014 at 16:57
  • \$\begingroup\$ Actually, parsing code like that using regular expressions is not a problem. The reason why HTML is not a regular language is nesting: you can't parse something like <i><b><i>oops</i></b></i> using regular expressions. \$\endgroup\$
    – svick
    Feb 14, 2014 at 20:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.