Skip to content

internal/sanitizer: allow picture and source elements in readme #111

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/frontend/markdown.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ func processReadme(ctx context.Context, readme *internal.Readme, info *source.In
}, nil
}

// rewriteImgSrc rewrites the HTML in the markdown document to replace img
// src keys with a value that properly represents the source of the image
// rewriteImgSrc rewrites the HTML in the markdown document to replace img and source
// src and srcset keys with a value that properly represents the source of the image
// from the repo.
func rewriteImgSrc(doc *markdown.Document, info *source.Info, readme *internal.Readme) {
walkBlocks(doc.Blocks, func(b markdown.Block) error {
Expand Down
4 changes: 2 additions & 2 deletions internal/frontend/overview.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,10 @@ func translateHTML(htmlText []byte, info *source.Info, readme *internal.Readme)
// It reports whether it made a change.
func walkHTML(n *html.Node, info *source.Info, readme *internal.Readme) bool {
changed := false
if n.Type == html.ElementNode && n.DataAtom == atom.Img {
if n.Type == html.ElementNode && (n.DataAtom == atom.Img || n.DataAtom == atom.Source) {
var attrs []html.Attribute
for _, a := range n.Attr {
if a.Key == "src" {
if a.Key == "src" || a.Key == "srcset" {
if v := translateLink(a.Val, info, true, readme); v != "" {
a.Val = v
changed = true
Expand Down
36 changes: 36 additions & 0 deletions internal/frontend/readme_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,42 @@ func TestReadme(t *testing.T) {
wantHTML: `<p><strong><img src="https://github.com/gobuffalo/buffalo/raw/master/logo.svg" alt="alt"/></strong></p>`,
wantOutline: nil,
},
{
name: "picture and source elements with escaped image source",
unit: unit,
readme: &internal.Readme{
Filepath: "README.md",
Contents: `<picture>` + "\n" +
`<source media="(prefers-color-scheme: dark)" srcset=".images/dark.svg">` + "\n" +
`<source media="(prefers-color-scheme: light)" srcset=".images/light.svg">` + "\n" +
`<img alt="GoVector Logo" src=".images/light.svg">` + "\n" +
`</picture>`,
},
wantHTML: `<picture>` + "\n" +
`<source media="(prefers-color-scheme: dark)" srcset="https://github.com/valid/module_name/raw/v1.0.0/.images/dark.svg"/>` + "\n" +
`<source media="(prefers-color-scheme: light)" srcset="https://github.com/valid/module_name/raw/v1.0.0/.images/light.svg"/>` + "\n" +
`<img alt="GoVector Logo" src="https://github.com/valid/module_name/raw/v1.0.0/.images/light.svg"/>` + "\n" +
`</picture>`,
wantOutline: nil,
},
{
name: "picture and source elements with invalid media query",
unit: unit,
readme: &internal.Readme{
Filepath: "README.md",
Contents: `<picture>` + "\n" +
`<source media="<script>malicious code</script>" srcset=".images/dark.svg">` + "\n" +
`<source media="(prefers-color-scheme: light)" srcset=".images/light.svg">` + "\n" +
`<img alt="GoVector Logo" src=".images/light.svg">` + "\n" +
`</picture>`,
},
wantHTML: `<picture>` + "\n" +
`<source srcset="https://github.com/valid/module_name/raw/v1.0.0/.images/dark.svg"/>` + "\n" +
`<source media="(prefers-color-scheme: light)" srcset="https://github.com/valid/module_name/raw/v1.0.0/.images/light.svg"/>` + "\n" +
`<img alt="GoVector Logo" src="https://github.com/valid/module_name/raw/v1.0.0/.images/light.svg"/>` + "\n" +
`</picture>`,
wantOutline: nil,
},
} {
t.Run(test.name, func(t *testing.T) {
test.unit.Readme = test.readme
Expand Down
11 changes: 9 additions & 2 deletions internal/sanitizer/sanitizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ var allowElems = []string{
"mark",
"ol",
"p",
"picture",
"pre",
"q",
"rp",
Expand All @@ -234,6 +235,7 @@ var allowElems = []string{
"samp",
"section",
"small",
"source",
"span",
"strike",
"strong",
Expand Down Expand Up @@ -292,6 +294,8 @@ var allowAttrs = []allowAttr{
{"p", "width", flexiblewidth}, // pkgsite allows all values
{"q", "cite", validURL},
{"time", "datetime", iso8601},
{"source", "media", mediaQuery},
{"source", "srcset", validURL},
{"ol", "type", re(`(?i)^(circle|disc|square|a|A|i|I|1)$`)},
{"ul", "type", re(`(?i)^(circle|disc|square|a|A|i|I|1)$`)},
{"li", "type", re(`(?i)^(circle|disc|square|a|A|i|I|1)$`)},
Expand Down Expand Up @@ -352,8 +356,9 @@ var allowAttrs = []allowAttr{
// roundtripAttrs is a map from attribute keys which should be checked
// against roundtripURL to maps of tags which are allowed to have them.
var roundtripAttrs = map[string]map[string]bool{
"src": {"img": true},
"href": {"a": true},
"src": {"img": true},
"srcset": {"source": true},
"href": {"a": true},
"cite": {
"blockquote": true,
"del": true,
Expand Down Expand Up @@ -385,6 +390,8 @@ var integer = re(`^[0-9]+$`)
var iso8601 = re(`^[0-9]{4}(-[0-9]{2}(-[0-9]{2}([ T][0-9]{2}(:[0-9]{2}){1,2}(.[0-9]{1,6})` +
`?Z?([\+-][0-9]{2}:[0-9]{2})?)?)?)?$`)

var mediaQuery = re(`^[\(\)\s\w\-:./]+$`)

func re(rx string) func(string) bool {
return regexp.MustCompile(rx).MatchString
}
Expand Down
12 changes: 12 additions & 0 deletions internal/sanitizer/sanitizer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,18 @@ func TestSanitizeBytes(t *testing.T) {
},
{`<p><bad><bad2><bad3><bad4>hello<bad5><bad6><p> middle</p>goodbye`,
`<p>hello</p><p> middle</p>goodbye`},
{
`<picture>
<source media="(prefers-color-scheme: dark)" srcset="dark.svg">
<source media="(prefers-color-scheme: light)" srcset="light.svg">
<img src="light.svg" alt="Logo">
</picture>`,
`<picture>
<source media="(prefers-color-scheme: dark)" srcset="dark.svg"/>
<source media="(prefers-color-scheme: light)" srcset="light.svg"/>
<img src="light.svg" alt="Logo"/>
</picture>`,
},
}

for _, tc := range testCases {
Expand Down