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.

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

Invalid Accept-Encoding header causes panic #914

Closed
1lann opened this issue Mar 29, 2015 · 6 comments
Closed

Invalid Accept-Encoding header causes panic #914

1lann opened this issue Mar 29, 2015 · 6 comments
Assignees
Labels
effort-minutes Will take up to 60 minutes to implement priority-should sooner rather than later topic-controller topic-filter type-bug
Milestone

Comments

@1lann
Copy link

1lann commented Mar 29, 2015

If you have results.compressed turned on, and you try to request a page with invalid Accept-Encoding headers (with a semicolon). Ex: curl --header "Accept-Encoding: foobar;" 127.0.0.1:9000. It results in a panic of

ERROR 2015/03/29 12:13:23 panic.go:29: runtime error: slice bounds out of range
/Users/jason/Workspace/Go/src/cruncher/app/init.go:54 (0xa1d6f)
    func.001: fc[0](c, fc[1:]) // Execute the next filter stage.
/Users/jason/Workspace/Go/src/github.com/revel/revel/i18n.go:155 (0x73237)
    I18nFilter: fc[0](c, fc[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/validation.go:191 (0x8d024)
    ValidationFilter: fc[0](c, fc[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/flash.go:45 (0x6fdcc)
    FlashFilter: fc[0](c, fc[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/session.go:148 (0x85408)
    SessionFilter: fc[0](c, fc[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/params.go:133 (0x770e9)
    ParamsFilter: fc[0](c, fc[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/filterconfig.go:208 (0x6f952)
    FilterConfiguringFilter: fc[0](c, fc[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/router.go:472 (0x82b3c)
    RouterFilter: fc[0](c, fc[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/panic.go:15 (0x75626)
    PanicFilter: fc[0](c, fc[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/watcher.go:232 (0x93765)
    func.031: fc[0](c, fc[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/server.go:43 (0x833de)
    handleInternal: Filters[0](c, Filters[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/server.go:31 (0x82fde)
    handle: handleInternal(w, r, nil)
/usr/local/go/src/net/http/server.go:1265 (0x128071)
    HandlerFunc.ServeHTTP: f(w, r)
/usr/local/go/src/net/http/server.go:1703 (0x12a0fa)
    serverHandler.ServeHTTP: handler.ServeHTTP(rw, req)
/usr/local/go/src/net/http/server.go:1204 (0x127bc7)
    (*conn).serve: serverHandler{c.server}.ServeHTTP(w, w.req)
/usr/local/go/src/runtime/asm_amd64.s:2232 (0x40211)
    goexit: 

and gives a 500 response to the client with RenderError.

Line 54 of init.go is the standard HeaderFilter included in init.go by default. By commenting out the HeaderFilter in the filters list, you get a different error instead:

go/src/runtime/asm_amd64.s:401 (0x3d945)
    call16: CALLFN(·call16, 16)
/usr/local/go/src/runtime/panic.go:387 (0x15958)
    gopanic: reflectcall(unsafe.Pointer(d.fn), deferArgs(d), uint32(d.siz), uint32(d.siz))
/Users/jason/Workspace/Go/src/github.com/revel/revel/intercept.go:93 (0x903cd)
    func.040: panic(err)
/usr/local/go/src/runtime/asm_amd64.s:401 (0x3d945)
    call16: CALLFN(·call16, 16)
/usr/local/go/src/runtime/panic.go:387 (0x15958)
    gopanic: reflectcall(unsafe.Pointer(d.fn), deferArgs(d), uint32(d.siz), uint32(d.siz))
/usr/local/go/src/runtime/panic.go:18 (0x14b3e)
    panicslice: panic(sliceError)
/Users/jason/Workspace/Go/src/github.com/revel/revel/compress.go:156 (0x66950)
    (*CompressResponseWriter).DetectCompressionType: num, err := strconv.ParseFloat(strings.TrimSpace(encodingParts[1])[2:], 32)
/Users/jason/Workspace/Go/src/github.com/revel/revel/compress.go:51 (0x6582a)
    CompressFilter: writer.DetectCompressionType(c.Request, c.Response)
/Users/jason/Workspace/Go/src/github.com/revel/revel/intercept.go:103 (0x73534)
    InterceptorFilter: fc[0](c, fc[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/i18n.go:155 (0x72907)
    I18nFilter: fc[0](c, fc[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/validation.go:191 (0x889e4)
    ValidationFilter: fc[0](c, fc[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/flash.go:45 (0x6f49c)
    FlashFilter: fc[0](c, fc[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/session.go:148 (0x84ad8)
    SessionFilter: fc[0](c, fc[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/params.go:133 (0x767b9)
    ParamsFilter: fc[0](c, fc[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/filterconfig.go:208 (0x6f022)
    FilterConfiguringFilter: fc[0](c, fc[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/router.go:472 (0x8220c)
    RouterFilter: fc[0](c, fc[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/panic.go:15 (0x74cf6)
    PanicFilter: fc[0](c, fc[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/server.go:43 (0x82aae)
    handleInternal: Filters[0](c, Filters[1:])
/Users/jason/Workspace/Go/src/github.com/revel/revel/server.go:31 (0x826ae)
    handle: handleInternal(w, r, nil)
/usr/local/go/src/net/http/server.go:1265 (0x119b11)
    HandlerFunc.ServeHTTP: f(w, r)
/usr/local/go/src/net/http/server.go:1703 (0x11bb9a)
    serverHandler.ServeHTTP: handler.ServeHTTP(rw, req)
/usr/local/go/src/net/http/server.go:1204 (0x119667)
    (*conn).serve: serverHandler{c.server}.ServeHTTP(w, w.req)
/usr/local/go/src/runtime/asm_amd64.s:2232 (0x3fa61)
    goexit: 

Which also results in multiple writes in the response, with the panic being on top of the actual page you requested for, not a server error page (RenderError). The stack trace is also returned in the response when running in production mode (Not sure if this is intended?) EDIT: I realised that this part of the issue has already been resolved in 0.12 #831. I would expect there to be a bad request response, or there to not be a panic/ignoring the bad header.

I'm using revel 0.11.3 with Go 1.4 darwin/amd64

@brendensoares
Copy link
Member

@1lann thanks for taking the time to detail this issue. The next step is to confirm this by testing independently to confirm your issue.

@ghost
Copy link

ghost commented Apr 4, 2015

I've tried to reproduce but cannot confirm the bug. revel/revel, revel/cmd, and revel/modules are the latest master branches, newly created app is used. Both revel runed and revel builded results work as expected, no panics or errors. Incorrect headers are ignored.

@1lann
Copy link
Author

1lann commented Apr 5, 2015

Still happens for me with the latest release. It only happens when you have results.compressed = true in your app.conf file, otherwise it does ignore it.

@ghost
Copy link

ghost commented Apr 5, 2015

@1lann Oh, I didn't pay attention to results.compressed part. So, can confirm, there is a bug. I have identified its cause and it should be fixed in v0.13. Thank you.

@ghost ghost added this to the v0.13 milestone Apr 5, 2015
@ghost ghost added topic-filter effort-minutes Will take up to 60 minutes to implement priority-should sooner rather than later and removed needs testing labels Apr 5, 2015
@ghost ghost self-assigned this Apr 5, 2015
@jeevatkm
Copy link
Contributor

@alkchr you have mentioned

I have identified its cause

Can you please share the details?

@jeevatkm jeevatkm assigned jeevatkm and unassigned ghost May 25, 2016
jeevatkm added a commit that referenced this issue May 25, 2016
@jeevatkm
Copy link
Contributor

One edge case is addressed, however it's better cover all possible cases or adapt to https://github.com/golang/gddo/blob/master/httputil/header/header.go#L172

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort-minutes Will take up to 60 minutes to implement priority-should sooner rather than later topic-controller topic-filter type-bug
Projects
None yet
Development

No branches or pull requests

4 participants