sanitizehtml: add a package to sanitize HTML

common/data/text/sanitizehtml/sanitize.go

Index: common/data/text/sanitizehtml/sanitize.go
diff --git a/common/data/text/sanitizehtml/sanitize.go b/common/data/text/sanitizehtml/sanitize.go
new file mode 100644
index 0000000000000000000000000000000000000000..28461aca7c03b0c653f544137cb4562a92221e36
--- /dev/null
+++ b/common/data/text/sanitizehtml/sanitize.go
@@ -0,0 +1,207 @@
+// Copyright 2017 The LUCI Authors. All rights reserved.
+// Use of this source code is governed under the Apache License, Version 2.0
+// that can be found in the LICENSE file.
+
+// Package sanitizehtml implements a sanitizer of a very limited HTML.
+// See Sanitize comment.
+package sanitizehtml
+
+import (
+	"bufio"
+	"io"
+	"net/url"
+	"strings"
+
+	"golang.org/x/net/html"
+	"golang.org/x/net/html/atom"
+)
+
+// attrValueSanitizer sanitizes an attribute value.
+type attrValueSanitizer func(string) string
+
+func alwaysSafe(s string) string {
+	return s
+}
+
+func sanitizeURL(s string) string {
+	u, err := url.Parse(s)
+	invalidityReason := ""
+	switch {
+	case err != nil:
+		invalidityReason = "url-is-malformed"

[xtof, 2017/05/09 16:06:21]

Nit: After the transformation, the URL is no longer malformed.  Maybe just
remove the "is"?
 about:invalid#malformed-url
 about:invalid#disallowed-scheme

Or you could be more verbose
 about:invalid#sanitized&reason=malformed-url

[nodir, 2017/05/10 06:48:48]

Done.

+
+	case u.Scheme != "http" && u.Scheme != "https":
+		invalidityReason = "url-is-not-http-or-https"

[xtof, 2017/05/09 16:06:21]

Instead of the indirection via invalidityReason, it might be easier to read to
just return directly (perhaps with a const to avoid typos)
                  return aboutInvalid + "disallowed-scheme"

[nodir, 2017/05/10 06:48:48]

yeah, i like this more. done

+
+	case u.Host == "":
+		invalidityReason = "url-is-relative"
+	}
+	if invalidityReason != "" {
+		return "about:invalid#" + invalidityReason
+	}
+
+	// re-serialize the URL to ensure that what we return is what we think
+	// we parsed.
+	return u.String()
+}
+
+type attrMap map[string]attrValueSanitizer
+
+var (
+	anchorAttrs = attrMap{
+		"alt":  alwaysSafe,
+		"href": sanitizeURL,
+	}
+	trAttrs = attrMap{
+		"rowspan": alwaysSafe,
+		"colspan": alwaysSafe,
+	}
+	tdAttrs = attrMap{
+		"rowspan": alwaysSafe,
+		"colspan": alwaysSafe,
+	}
+)
+
+type stringWriter interface {
+	WriteString(string) (int, error)
+}
+
+type sanitizer struct {
+	sw  stringWriter
+	err error
+}
+
+// p prints the text, unless there was an error before.
+func (s *sanitizer) p(safeMarkup string) {
+	if s.err == nil {
+		_, s.err = s.sw.WriteString(safeMarkup)
+	}
+}
+
+// printAttrs sanitizes and prints a whitelist of attributes in el
+func (s *sanitizer) printAttrs(el *html.Node, whitelist attrMap) {
+	for _, a := range el.Attr {
+		key := strings.ToLower(a.Key)
+		if sanitizer, ok := whitelist[key]; a.Namespace == "" && ok {
+			s.p(" ")
+			s.p(key)
+			s.p("=\"")
+			s.p(html.EscapeString(sanitizer(a.Val)))
+			s.p("\"")
+		}
+	}
+}
+
+// printElem prints the safe element with a whitelist of attributes.
+// If allowedAttrs is nil, all attributes are omitted.
+//
+// Do not call for unsafe elements.
+func (s *sanitizer) printElem(safeElement *html.Node, allowedAttrs attrMap) {
+	tag := safeElement.DataAtom.String()
+	s.p("<")
+	s.p(tag)
+	if allowedAttrs == nil {
+		// ignore attributes
+	} else {
+		s.printAttrs(safeElement, allowedAttrs)
+	}
+	s.p(">")
+
+	s.visitChildren(safeElement)
+
+	s.p("</")
+	s.p(tag)
+	s.p(">")
+}
+
+func (s *sanitizer) visit(n *html.Node) {
+	switch n.Type {
+	case html.TextNode:
+		// print it escaped.
+		s.p(html.EscapeString(n.Data))
+
+	case html.ElementNode:
+		// This switch statement defines what HTML elements we allow.
+		switch n.DataAtom {
+		case atom.Br:
+			// br is allowed and it should not be closed
+			s.p("<br>")
+
+		case atom.Script, atom.Style:
+			// ignore entirely
+			// do not visit children so we don't print inner text
+
+		case atom.A:
+			s.p(`<a rel="noopener" target="_blank"`)
+			s.printAttrs(n, anchorAttrs)
+			s.p(">")
+			s.visitChildren(n)
+			s.p("</a>")
+
+		case atom.P, atom.Ol, atom.Ul, atom.Li, atom.Table, atom.Strong, atom.Em:
+			// print without attributes
+			s.printElem(n, nil)
+
+		case atom.Tr:
+			s.printElem(n, trAttrs)
+
+		case atom.Td:
+			s.printElem(n, tdAttrs)
+
+		default:
+			// ignore the element, but visit children.
+			s.visitChildren(n)
+		}
+
+	default:
+		// ignore the node, but visit children.
+		s.visitChildren(n)
+	}
+}
+
+func (s *sanitizer) visitChildren(n *html.Node) {
+	for c := n.FirstChild; c != nil; c = c.NextSibling {
+		s.visit(c)
+	}
+}
+
+// Sanitize strips all HTML nodes except allowed ones.
+//
+// Unless explicitly specified, attributes are stripped.
+// Allowed elements:
+//  - p, br
+//  - strong, em
+//  - a
+//    - if href attribute is not a valid absolute HTTP(s) link, it is replaced
+//      with a innocuous fragment-only link.
+//    - alt attribute is allowed
+//  - ul, ol, li
+//  - table
+//  - tr, td. Attributes rowspan/colspan are allowed, but if a value contains a
+//    non-digit character, the character and the rest of the value is stripped.
+//
+// Elements <script> and <style> are ignored entirely.
+// For all other HTML nodes, Sanitize ignores the node, but visits its children.
+func Sanitize(r io.Reader, w io.Writer) (err error) {

[nigeltao1, 2017/05/10 02:27:20]

The general Go style is destination args before source args. For example, the
io.Copy function is Copy(Writer, Reader), not Copy(Reader, Writer).

[nodir, 2017/05/10 06:48:48]

Done.

+	var root *html.Node
+	root, err = html.Parse(r)
+	if err != nil {
+		return err
+	}
+
+	sw, ok := w.(stringWriter)
+	if !ok {
+		bw := bufio.NewWriter(w)
+		defer func() {
+			ferr := bw.Flush()
+			if err == nil {
+				err = ferr
+			}
+		}()
+		sw = bw
+	}
+
+	s := sanitizer{sw: sw}
+	s.visit(root)
+	return s.err
+}