Repository: gohugoio/hugo (language: go)

Issue:
RSS XML can contain invalid characters
I found that RSS feeds that Hugo generates can contain characters that are invalid for XML.

The XML 1.0 spec defines valid characters: https://www.w3.org/TR/2006/REC-xml-20060816/#charsets

One I encountered in the wild in a blog using Hugo is U+000b (\v, vertical tab). (It was this blog, if you're interested: https://blog.hypriot.com/)

Trying to parse such XML raises an error with Go's decoder (which is how I noticed this in the first place):

`XML syntax error on line 10: illegal character code U+000B`

My environment:
  * Hugo Static Site Generator v0.20-DEV linux/amd64 BuildDate: 2017-04-02T17:42:16-07:00
  * Debian Linux (testing) 64bit

Here are two small sample Go programs to help demonstrate the problem:

Create a post with an invalid character:

```
package main

import "fmt"

func main() {
	post := `+++
date = "2017-04-02T16:11:58+05:30"
draft = false
title = "New post"

+++

Hi there
`

	post += "\u000bsudo apt-get update\u000b"

	fmt.Println(post)
}
```

Use like this: `$ ./create-problem-post  > ~/t/bookshelf/content/post/newpost.md`

Then re-generate the site: `$ hugo`

Then try to decode the RSS feed with this program:

```
package main

import (
	"encoding/xml"
	"io/ioutil"
	"log"
	"os"
)

func main() {
	buf, err := ioutil.ReadAll(os.Stdin)
	if err != nil {
		log.Fatalf("Reading from stdin: %s", err)
	}

	type TestStruct struct {
		Blah string
	}

	t := TestStruct{}

	if err := xml.Unmarshal(buf, &t); err != nil {
		log.Fatalf("Unmarshal XML: %s", err)
	}
}
```

Like so:

```
$ cat ~/t/bookshelf/public/index.xml | ./read-problem-post 
2017/04/02 21:27:43 Unmarshal XML: XML syntax error on line 22: illegal character code U+000B
```

Thank you!


Discussion / hints:
This issue has been automatically marked as stale because it has not had recent activity. The resources of the Hugo team are limited, and so we are asking for your help.
If this is a **bug** and you can still reproduce this error on the <code>master</code> branch, please reply with all of the information you have about it in order to keep the issue open.
If this is a **feature request**, and you feel that it is still relevant and valuable, please tell us why.
This issue will automatically be closed in the near future if no further activity occurs. Thank you for all your contributions.

The problem still exists. I just tested with the latest master branch:

```
$ hugo version
Hugo Static Site Generator v0.32-DEV linux/amd64 BuildDate: 2017-12-10T12:38:16-08:00
```

I had to change the sample program I provided that creates the problem post slightly to account for front matter changes:

```
$ cat create-problem-post/main.go 
package main

import "fmt"

func main() {
	post := `---
title: "New post"
date: "2017-04-02T16:11:58+05:30"
draft: false
---

Hi there
`

	post += "\u000bsudo apt-get update\u000b"

	fmt.Println(post)
}
```
It is also generating `&ldquo` and `&rdquo` which no browser or validator accepts as valid.
This issue has been automatically marked as stale because it has not had recent activity. The resources of the Hugo team are limited, and so we are asking for your help.
If this is a **bug** and you can still reproduce this error on the <code>master</code> branch, please reply with all of the information you have about it in order to keep the issue open.
If this is a **feature request**, and you feel that it is still relevant and valuable, please tell us why.
This issue will automatically be closed in the near future if no further activity occurs. Thank you for all your contributions.

The problem still exists with master as of this moment:

```
will@snorri:~/t/hugosite$ hugo version
Hugo Static Site Generator v0.42-DEV linux/amd64
will@snorri:~/t/hugosite$ ~/go/src/github.com/horgh/hugo-rss-test/read-problem-post/read-problem-post < public/index.xml
2018/06/09 09:01:41 Unmarshal XML: XML syntax error on line 20: illegal character code U+000B
```
This issue has been automatically marked as stale because it has not had recent activity. The resources of the Hugo team are limited, and so we are asking for your help.
If this is a **bug** and you can still reproduce this error on the <code>master</code> branch, please reply with all of the information you have about it in order to keep the issue open.
If this is a **feature request**, and you feel that it is still relevant and valuable, please tell us why.
This issue will automatically be closed in the near future if no further activity occurs. Thank you for all your contributions.

This is still a problem with current master:

```
will@snorri:~/t/hugosite$ hugo version
Hugo Static Site Generator v0.50-DEV linux/amd64 BuildDate: unknown
will@snorri:~/t/hugosite$ rm public/index.xml
will@snorri:~/t/hugosite$ hugo
[snip]
will@snorri:~/t/hugosite$ ~/go/src/github.com/horgh/hugo-rss-test/read-problem-post/read-problem-post < public/index.xml
2018/10/07 10:28:55 Unmarshal XML: XML syntax error on line 20: illegal character code U+000B
```
Notes from a short investigation:

I attempted to use `<!CDATA[ ... ]]>` around the `<description>` contents, but that didn't fix the issue.  The illegal character within the CDATA block still [violates the XML spec](https://www.w3.org/TR/xml/#sec-cdata-sect). (Additionally, we use `html/template` for the RSS feed, so the `<!CDATA` gets escaped if we try to use that in the RSS template, anyway.)

I then added a `transform.XMLEscape` template function that essentially calls [`xml.EscapeText`](https://golang.org/pkg/encoding/xml/#EscapeText).  That doesn't work by itself because `xml.EscapeText` will bail out when it finds an illegal character.

So, it looks like we'd need to add a `sanitizeXML` function to strip illegal characters prior to escaping.
Thanks for looking at this! I was actually going to take a stab at fixing it too.

Your XMLEscape idea seems great! Regarding the error from xml.EscapeText: I tested with U+000B and it output the Unicode replacement character for it rather than erroring. What character did you test with that caused an error?

A sanitizeXML function seems okay too if there are indeed errors.
I looked at the xml.EscapeText result rather quickly at the end, so you may be right.  I'll take another look.
I have a branch that uses the EscapeText template method: https://github.com/gohugoio/hugo/compare/master...horgh:horgh/rss-invalid-chars?expand=1

I had some trouble with the tests. For some reason in the tests the vertical tab disappears all together. If I build and run a test against a hugo directory it works fine though. Any ideas? Or maybe you're making a branch anyway!

Edit: And I don't understand that Travis failure!

Fix patch:
diff --git a/docs/content/en/functions/transform/XMLEscape.md b/docs/content/en/functions/transform/XMLEscape.md
new file mode 100644
index 00000000000..17ed2a13d11
--- /dev/null
+++ b/docs/content/en/functions/transform/XMLEscape.md
@@ -0,0 +1,38 @@
+---
+title: transform.XMLEscape
+description: Returns the given string, removing disallowed characters then escaping the result to its XML equivalent.
+categories: []
+keywords: []
+action:
+  aliases: []
+  related: []
+  returnType: string
+  signatures: [transform.XMLEscape INPUT]
+---
+
+The `transform.XMLEscape` function removes [disallowed characters] as defined in the XML specification, then escapes the result by replacing the following characters with [HTML entities]:
+
+- `"` → `&#34;`
+- `'` → `&#39;`
+- `&` → `&amp;`
+- `<` → `&lt;`
+- `>` → `&gt;`
+- `\t` → `&#x9;`
+- `\n` → `&#xA;`
+- `\r` → `&#xD;`
+
+For example:
+
+```go-html-template
+transform.XMLEscape "<p>abc</p>" → &lt;p&gt;abc&lt;/p&gt;
+```
+
+When using `transform.XMLEscape` in a template rendered by Go's [html/template] package, declare the string to be safe HTML to avoid double escaping. For example, in an RSS template:
+
+{{< code file="layouts/_default/rss.xml" >}}
+<description>{{ .Summary | transform.XMLEscape | safeHTML }}</description>
+{{< /code >}}
+
+[disallowed characters]: https://www.w3.org/TR/xml/#charsets
+[html entities]: https://developer.mozilla.org/en-us/docs/glossary/entity
+[html/template]: https://pkg.go.dev/html/template
diff --git a/tpl/tplimpl/embedded/templates/_default/rss.xml b/tpl/tplimpl/embedded/templates/_default/rss.xml
index a4cdd2383a2..6818d31ec16 100644
--- a/tpl/tplimpl/embedded/templates/_default/rss.xml
+++ b/tpl/tplimpl/embedded/templates/_default/rss.xml
@@ -64,7 +64,7 @@
       <pubDate>{{ .Date.Format "Mon, 02 Jan 2006 15:04:05 -0700" | safeHTML }}</pubDate>
       {{- with $authorEmail }}<author>{{ . }}{{ with $authorName }} ({{ . }}){{ end }}</author>{{ end }}
       <guid>{{ .Permalink }}</guid>
-      <description>{{ .Summary | html }}</description>
+      <description>{{ .Summary | transform.XMLEscape | safeHTML }}</description>
     </item>
     {{- end }}
   </channel>
diff --git a/tpl/transform/init.go b/tpl/transform/init.go
index 00ae8f89d04..e439604277a 100644
--- a/tpl/transform/init.go
+++ b/tpl/transform/init.go
@@ -112,6 +112,16 @@ func init() {
 			},
 		)
 
+		ns.AddMethodMapping(ctx.XMLEscape,
+			nil,
+			[][2]string{
+				{
+					`{{ transform.XMLEscape "<p>abc</p>" }}`,
+					`&lt;p&gt;abc&lt;/p&gt;`,
+				},
+			},
+		)
+
 		return ns
 	}
 
diff --git a/tpl/transform/transform.go b/tpl/transform/transform.go
index d943b0b5752..178db84e412 100644
--- a/tpl/transform/transform.go
+++ b/tpl/transform/transform.go
@@ -15,9 +15,12 @@
 package transform
 
 import (
+	"bytes"
 	"context"
+	"encoding/xml"
 	"html"
 	"html/template"
+	"strings"
 
 	"github.com/gohugoio/hugo/cache/namedmemcache"
 	"github.com/gohugoio/hugo/markup/converter/hooks"
@@ -118,6 +121,34 @@ func (ns *Namespace) HTMLUnescape(s any) (string, error) {
 	return html.UnescapeString(ss), nil
 }
 
+// XMLEscape returns the given string, removing disallowed characters then
+// escaping the result to its XML equivalent.
+func (ns *Namespace) XMLEscape(s any) (string, error) {
+	ss, err := cast.ToStringE(s)
+	if err != nil {
+		return "", err
+	}
+
+	// https://www.w3.org/TR/xml/#NT-Char
+	cleaned := strings.Map(func(r rune) rune {
+		if r == 0x9 || r == 0xA || r == 0xD ||
+			(r >= 0x20 && r <= 0xD7FF) ||
+			(r >= 0xE000 && r <= 0xFFFD) ||
+			(r >= 0x10000 && r <= 0x10FFFF) {
+			return r
+		}
+		return -1
+	}, ss)
+
+	var buf bytes.Buffer
+	err = xml.EscapeText(&buf, []byte(cleaned))
+	if err != nil {
+		return "", err
+	}
+
+	return buf.String(), nil
+}
+
 // Markdownify renders s from Markdown to HTML.
 func (ns *Namespace) Markdownify(ctx context.Context, s any) (template.HTML, error) {
 


Test patch:
diff --git a/tpl/tplimpl/template_funcs_test.go b/tpl/tplimpl/template_funcs_test.go
index cb1aa6febd4..9cc84934b9f 100644
--- a/tpl/tplimpl/template_funcs_test.go
+++ b/tpl/tplimpl/template_funcs_test.go
@@ -37,7 +37,7 @@ home=["HTML"]
 -- files/README.txt --
 Hugo Rocks!
 -- content/blog/hugo-rocks.md --
---- 
+---
 title: "**BatMan**"
 ---
 `
@@ -65,6 +65,10 @@ title: "**BatMan**"
 					// This will fail the build, so skip for now.
 					continue
 				}
+				if strings.Contains(example[0], "transform.XMLEscape") {
+					// This will fail the build, so skip for now.
+					continue
+				}
 				templates = append(templates, example[0])
 				expected = append(expected, example[1])
 			}
diff --git a/tpl/transform/integration_test.go b/tpl/transform/integration_test.go
index 17348928db6..9a68b7ff2c3 100644
--- a/tpl/transform/integration_test.go
+++ b/tpl/transform/integration_test.go
@@ -65,3 +65,23 @@ foo
 	b.AssertFileContent("public/p3/index.html", "_<h2 id=\"foo\">foo</h2>\n<p>bar</p>\n_")
 	b.AssertFileContent("public/p4/index.html", "_<p id=\"bar\">foo</p>\n_")
 }
+
+func TestXMLEscape(t *testing.T) {
+	t.Parallel()
+
+	files := `
+-- config.toml --
+disableKinds = ['section','sitemap','taxonomy','term']
+-- content/p1.md --
+---
+title: p1
+---
+a **b**  c
+<!--more-->
+  `
+	b := hugolib.Test(t, files)
+
+	b.AssertFileContent("public/index.xml", `
+	<description>&lt;p&gt;a &lt;strong&gt;b&lt;/strong&gt;  c&lt;/p&gt;</description>
+	`)
+}