Skip to content
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.

Sign up for GitHub

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

Jump to bottom

html/template: exponential memory usage in srcset processing #24731

Closed
rhysh opened this issue Apr 6, 2018 · 3 comments
Closed

html/template: exponential memory usage in srcset processing #24731

rhysh opened this issue Apr 6, 2018 · 3 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.11

Comments

@rhysh
Copy link
Contributor

rhysh commented Apr 6, 2018

What version of Go are you using (go version)?

Go 1.10, on the playground

Does this issue reproduce with the latest release?

Yes, it's also present in go version devel +542ea5ad91 Wed Apr 4 13:39:34 2018 -0700 darwin/amd64

What operating system and processor architecture are you using (go env)?

The problem reproduces on the playground

What did you do?

https://play.golang.org/p/-Emmk7seW-_6

package main

import (
	"html/template"
	"os"
)

func main() {
	data := "<r srcset={{\",,,,,,,,,,,,,,,,\"}}>"

	template.Must(template.New("").Parse(data)).Execute(os.Stdout, nil)
}

(Found via go-fuzz)

What did you expect to see?

<r srcset=,,,,,,,,,,,,,,,,>

What did you see instead?

<r srcset=panic: bytes.Buffer: too large [recovered]
	panic: bytes.Buffer: too large

goroutine 1 [running]:
text/template.errRecover(0x1052ff50, 0x10538070)
	/usr/local/go/src/text/template/exec.go:143 +0x220
panic(0x19a800, 0x1050c128)
	/usr/local/go/src/runtime/panic.go:505 +0x2c0
bytes.(*Buffer).grow(0x10552420, 0x3d942e20, 0x8ac60, 0x535a)
	/usr/local/go/src/bytes/buffer.go:141 +0x3c0
bytes.(*Buffer).Grow(0x10552420, 0x3d942e20)
	/usr/local/go/src/bytes/buffer.go:163 +0x40
html/template.processUrlOnto(0x1bc42f, 0x0, 0x1, 0x10552420, 0x0, 0x0)
	/usr/local/go/src/html/template/url.go:91 +0x40
html/template.filterSrcsetElement(0x1bc42f, 0x10, 0x10, 0x10, 0x10552420, 0x0)
	/usr/local/go/src/html/template/url.go:212 +0x2e0
html/template.srcsetFilterAndEscaper(0x1050c1e8, 0x1, 0x1, 0x0, 0x0, 0x0)
	/usr/local/go/src/html/template/url.go:171 +0x140
reflect.Value.call(0x194e80, 0x1d80c8, 0x13, 0x1b23d0, 0x4, 0x1050a120, 0x1, 0x1, 0x1, 0x535a, ...)
	/usr/local/go/src/reflect/value.go:447 +0xa60
reflect.Value.Call(0x194e80, 0x1d80c8, 0x13, 0x1050a120, 0x1, 0x1, 0x197d80, 0x1050c1e0, 0x98, 0x0)
	/usr/local/go/src/reflect/value.go:308 +0xc0
text/template.(*state).evalCall(0x1052ff0c, 0x0, 0x0, 0x0, 0x194e80, 0x1d80c8, 0x13, 0x1ebae0, 0x10546820, 0x1bb0f7, ...)
	/usr/local/go/src/text/template/exec.go:667 +0x600
text/template.(*state).evalFunction(0x1052ff0c, 0x0, 0x0, 0x0, 0x10546800, 0x1ebae0, 0x10546820, 0x1050c1d0, 0x1, 0x1, ...)
	/usr/local/go/src/text/template/exec.go:535 +0x160
text/template.(*state).evalCommand(0x1052ff0c, 0x0, 0x0, 0x0, 0x10546820, 0x197d80, 0x1050c1e0, 0x98, 0x197d80, 0x1050c1e0, ...)
	/usr/local/go/src/text/template/exec.go:432 +0x500
text/template.(*state).evalPipeline(0x1052ff0c, 0x0, 0x0, 0x0, 0x105381b0, 0x0, 0x0, 0x0, 0x199000, 0x10546480)
	/usr/local/go/src/text/template/exec.go:405 +0x100
text/template.(*state).walk(0x1052ff0c, 0x0, 0x0, 0x0, 0x1eb9e0, 0x105466a0)
	/usr/local/go/src/text/template/exec.go:231 +0x520
text/template.(*state).walk(0x1052ff0c, 0x0, 0x0, 0x0, 0x1ebc20, 0x10546620)
	/usr/local/go/src/text/template/exec.go:239 +0x140
text/template.(*Template).execute(0x10546540, 0x1eb430, 0x1050c108, 0x0, 0x0, 0x0, 0x0, 0x0)
	/usr/local/go/src/text/template/exec.go:194 +0x200
text/template.(*Template).Execute(0x10546540, 0x1eb430, 0x1050c108, 0x0, 0x0, 0x10546540, 0x0, 0x1050c1c8)
	/usr/local/go/src/text/template/exec.go:177 +0x60
html/template.(*Template).Execute(0x105465c0, 0x1eb430, 0x1050c108, 0x0, 0x0, 0x0, 0x0, 0x1053a070)
	/usr/local/go/src/html/template/template.go:122 +0xa0
main.main()
	/tmp/sandbox194177667/main.go:11 +0xc0

https://github.com/golang/go/blob/go1.10.1/src/html/template/url.go#L91

html/template.processUrlOnto calls bytes.Buffer.Grow with a desired target size, rather than with a desired growth amount, leading to memory usage doubling with each srcset element.

b.Grow(b.Cap() + len(s) + 16)
@mvdan mvdan added the NeedsFix The path to resolution is known, but the work has not been done. label Apr 6, 2018
@mvdan mvdan added this to the Go1.11 milestone Apr 6, 2018
@mvdan
Copy link
Member

mvdan commented Apr 6, 2018

Since you seem to have found the fix, I presume you'll send a CL?

@rhysh
Copy link
Contributor Author

rhysh commented Apr 6, 2018

Yes, I'll send one in a few minutes.

@gopherbot
Copy link

Change https://golang.org/cl/105155 mentions this issue: html/template: grow srcset buffer in proportion to need

@golang golang locked and limited conversation to collaborators Apr 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
3 participants
@rhysh @mvdan @gopherbot