tdewolff / minify

Go minifiers for web formats
https://go.tacodewolff.nl/minify
MIT License
3.7k stars 218 forks source link

HTML: panic with intensive workload #402

Closed Alexfilus closed 3 years ago

Alexfilus commented 3 years ago

Hi! I'm using github.com/tdewolff/minify/v2/html with https://docs.gofiber.io/ When I just refresh browser page manually, everything is fine, but when I tried to test app performance with wrk, I got panic:

panic: runtime error: index out of range [8925] with length 8925

goroutine 234 [running]:
github.com/tdewolff/parse/v2.(*Input).Peek(...)
        /Users/aleksandrfilippenko/go/pkg/mod/github.com/tdewolff/parse/v2@v2.5.14/input.go:105
github.com/tdewolff/parse/v2/html.(*Lexer).shiftAttribute(0xc0003d4230, 0xc0003462a0, 0x23, 0x4457045)
        /Users/aleksandrfilippenko/go/pkg/mod/github.com/tdewolff/parse/v2@v2.5.14/html/lex.go:376 +0x505
github.com/tdewolff/parse/v2/html.(*Lexer).Next(0xc0003d4230, 0x1b79, 0xc000392678, 0xc000014500, 0xc0003e91a0)
        /Users/aleksandrfilippenko/go/pkg/mod/github.com/tdewolff/parse/v2@v2.5.14/html/lex.go:112 +0xe8
github.com/tdewolff/minify/v2/html.(*TokenBuffer).read(0xc0003e94f0, 0xc00047e000)
        /Users/aleksandrfilippenko/go/pkg/mod/github.com/tdewolff/minify/v2@v2.9.16/html/buffer.go:41 +0x45
github.com/tdewolff/minify/v2/html.(*TokenBuffer).Shift(0xc0003e94f0, 0xc0003462a0)
        /Users/aleksandrfilippenko/go/pkg/mod/github.com/tdewolff/minify/v2@v2.9.16/html/buffer.go:102 +0x59
github.com/tdewolff/minify/v2/html.(*Minifier).Minify(0x4c44554, 0xc000578240, 0x48723c0, 0xc00054a000, 0x4871d60, 0xc0002b3140, 0x0, 0x0, 0x0)
        /Users/aleksandrfilippenko/go/pkg/mod/github.com/tdewolff/minify/v2@v2.9.16/html/html.go:374 +0x1065
github.com/tdewolff/minify/v2/html.Minify(0xc000578240, 0x48723c0, 0xc00054a000, 0x4871d60, 0xc0002b3140, 0x0, 0xc00041c5a0, 0x100000000000001)
        /Users/aleksandrfilippenko/go/pkg/mod/github.com/tdewolff/minify/v2@v2.9.16/html/html.go:60 +0x70
github.com/tdewolff/minify/v2.MinifierFunc.Minify(0x47c8840, 0xc000578240, 0x48723c0, 0xc00054a000, 0x4871d60, 0xc0002b3140, 0x0, 0xc0003e9738, 0x40539f8)
        /Users/aleksandrfilippenko/go/pkg/mod/github.com/tdewolff/minify/v2@v2.9.16/minify.go:38 +0x65
github.com/tdewolff/minify/v2.(*M).MinifyMimetype(0xc000578240, 0xc00002a790, 0x9, 0x10, 0x48723c0, 0xc00054a000, 0x4871d60, 0xc0002b3140, 0x0, 0x0, ...)
        /Users/aleksandrfilippenko/go/pkg/mod/github.com/tdewolff/minify/v2@v2.9.16/minify.go:207 +0x2f9
github.com/tdewolff/minify/v2.(*M).Minify(0xc000578240, 0x47a04a0, 0x9, 0x48723c0, 0xc00054a000, 0x4871d60, 0xc0002b3140, 0xc0003d1898, 0x1)
        /Users/aleksandrfilippenko/go/pkg/mod/github.com/tdewolff/minify/v2@v2.9.16/minify.go:196 +0xd1
main.main.func4(0xc00054a000, 0xc00002a5f8, 0x1)

Here is handler code:

buf := new(bytes.Buffer)
err = engine.Render(
    buf,
    "views/index",
    fiber.Map{
        "Categories": categories,
        "Subs":       subs,
        "Title":      "Главная",
        "Styles":     string(styles),
    },
    "views/layouts/main",
)
if err != nil {
    log.Fatal(err)
}

m := minify.New()
m.AddFunc("text/html", minifyHtml.Minify)
err = m.Minify("text/html", c, buf)
if err != nil {
    log.Fatal(err)
}

c.Set("content-type", fiber.MIMETextHTMLCharsetUTF8)

return nil

len which triggers panic is different for every try, and looks like some of request works fine before app fails

Alexfilus commented 3 years ago

looks like it falls down after 100-200 requests. Every time numbers are different.

Alexfilus commented 3 years ago

Little update: error reproduced only with concurrent request handling. wrk -t 1 -c 2 http://127.0.0.1/ - got panic index of range, and that index always larger than content size wrk -t 1 -c 1 http://127.0.0.1/ - no panic, everything works

tdewolff commented 3 years ago

That looks like a race condition, can you please check if this goes away when you copy the input buffer before passing to the minifier? E.g.:

buf2 := &bytes.Buffer{}
io.Copy(buf2, buf)
err = m.Minify("text/html", c, buf2)
// ...

Additionally, can you check what happens if you run it with the race condition detector enabled? E.g.: go install -race [...]

Alexfilus commented 3 years ago

Now it's also panic, but in template render... go install -race worked about a minute but found nothing.

tdewolff commented 3 years ago

If you skip minifying, does it still happen? To me it sounds like you have a race condition elsewhere. The minify library isn't using concurrency, but it does modify the input byte stream in-place and requires it to be untouched by others while minifying. If anything in your code is changing that input byte-slice or e.g. reusing it for another request, that may be the problem.

If this doesn't happen when you disable the minifier, could you please make a small piece of code that reproduces the error?

Alexfilus commented 3 years ago

I made some experiments. Looks like it's Minify and Fiber's template/html are incompatible somehow. My code works with template/html and without minify, if I turn on them both, program panics. Also I tried standard go template/html with standard net/http server, and with fasthttp. It works fine. I change template executor to jet. Panic is gone now! I don't find particular reason why it happend, but now my app works fine. Thank you, for attention.

gaby commented 1 year ago

@Alexfilus Even got this to work with Fiber/Fasthttp?