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

fix nil err panic in httpd WriteResponse #9353

Merged
merged 1 commit into from
Jan 24, 2018
Merged

fix nil err panic in httpd WriteResponse #9353

merged 1 commit into from
Jan 24, 2018

Conversation

phemmer
Copy link
Contributor

@phemmer phemmer commented Jan 22, 2018

Got the below panic from my client when it tried to do an invalid query. The problem and fix is obvious, so putting up a PR.

[panic:runtime error: invalid memory address or nil pointer dereference] goroutine 25614 [running]:
runtime/debug.Stack(0xc42068b420, 0xc420108600, 0xbe91793e8d120376)
	/usr/lib/go/src/runtime/debug/stack.go:24 +0xa7
github.com/influxdata/influxdb/services/httpd.(*Handler).recovery.func1.1(0xc42068b420, 0xc420108600, 0xbe91793e8d120376, 0x55582cbe9a, 0x12f00c0, 0xc4201f8a50, 0x12a57a0, 0xc421ab27e0)
	/tmp/.gdm_32513/src/github.com/influxdata/influxdb/services/httpd/handler.go:1537 +0xcd
panic(0xccd700, 0x12dc3b0)
	/usr/lib/go/src/runtime/panic.go:491 +0x283
github.com/influxdata/influxdb/services/httpd.(*msgpackFormatter).WriteResponse(0xc421b214e0, 0x0, 0x0, 0x0, 0x12945a0, 0xc421b21580, 0x0, 0x0, 0x0)
	/tmp/.gdm_32513/src/github.com/influxdata/influxdb/services/httpd/response_writer.go:211 +0x979
github.com/influxdata/influxdb/services/httpd.(*responseWriter).WriteResponse(0xc42068b460, 0x0, 0x0, 0x0, 0x12945a0, 0xc421b21580, 0x52, 0x52, 0x3d)
	/tmp/.gdm_32513/src/github.com/influxdata/influxdb/services/httpd/response_writer.go:59 +0x5a
github.com/influxdata/influxdb/services/httpd.(*Handler).httpError(0xc4201f8a50, 0x7f94723b5b68, 0xc42068b460, 0xc420428ae0, 0x52, 0x190)
	/tmp/.gdm_32513/src/github.com/influxdata/influxdb/services/httpd/handler.go:1265 +0x1bd
github.com/influxdata/influxdb/services/httpd.(*Handler).serveQuery(0xc4201f8a50, 0x7f94723b5b68, 0xc42068b460, 0xc420108600, 0x0, 0x0)
	/tmp/.gdm_32513/src/github.com/influxdata/influxdb/services/httpd/handler.go:387 +0x1bd4
github.com/influxdata/influxdb/services/httpd.(*Handler).(github.com/influxdata/influxdb/services/httpd.serveQuery)-fm(0x7f94723b5b68, 0xc42068b460, 0xc420108600, 0x0, 0x0)
	/tmp/.gdm_32513/src/github.com/influxdata/influxdb/services/httpd/handler.go:136 +0x5c
github.com/influxdata/influxdb/services/httpd.authenticate.func1(0x7f94723b5b68, 0xc42068b460, 0xc420108600)
	/tmp/.gdm_32513/src/github.com/influxdata/influxdb/services/httpd/handler.go:1337 +0x7d2
net/http.HandlerFunc.ServeHTTP(0xc4202e7b40, 0x7f94723b5b68, 0xc42068b460, 0xc420108600)
	/usr/lib/go/src/net/http/server.go:1918 +0x44
github.com/influxdata/influxdb/services/httpd.(*Handler).responseWriter.func1(0x12a2ca0, 0xc421c44e60, 0xc420108600)
	/tmp/.gdm_32513/src/github.com/influxdata/influxdb/services/httpd/handler.go:1514 +0xab
net/http.HandlerFunc.ServeHTTP(0xc4202e7b60, 0x12a2ca0, 0xc421c44e60, 0xc420108600)
	/usr/lib/go/src/net/http/server.go:1918 +0x44
github.com/influxdata/influxdb/services/httpd.gzipFilter.func1(0x12a2d20, 0xc42068b440, 0xc420108600)
	/tmp/.gdm_32513/src/github.com/influxdata/influxdb/services/httpd/gzip.go:39 +0x215
net/http.HandlerFunc.ServeHTTP(0xc4202e7b80, 0x12a2d20, 0xc42068b440, 0xc420108600)
	/usr/lib/go/src/net/http/server.go:1918 +0x44
github.com/influxdata/influxdb/services/httpd.cors.func1(0x12a2d20, 0xc42068b440, 0xc420108600)
	/tmp/.gdm_32513/src/github.com/influxdata/influxdb/services/httpd/handler.go:1459 +0x102
net/http.HandlerFunc.ServeHTTP(0xc4202e7ba0, 0x12a2d20, 0xc42068b440, 0xc420108600)
	/usr/lib/go/src/net/http/server.go:1918 +0x44
github.com/influxdata/influxdb/services/httpd.requestID.func1(0x12a2d20, 0xc42068b440, 0xc420108600)
	/tmp/.gdm_32513/src/github.com/influxdata/influxdb/services/httpd/handler.go:1490 +0x179
net/http.HandlerFunc.ServeHTTP(0xc4202e7bc0, 0x12a2d20, 0xc42068b440, 0xc420108600)
	/usr/lib/go/src/net/http/server.go:1918 +0x44
github.com/influxdata/influxdb/services/httpd.(*Handler).logging.func1(0x12a2d20, 0xc42068b420, 0xc420108600)
	/tmp/.gdm_32513/src/github.com/influxdata/influxdb/services/httpd/handler.go:1498 +0xe1
net/http.HandlerFunc.ServeHTTP(0xc4202e7be0, 0x12a2d20, 0xc42068b420, 0xc420108600)
	/usr/lib/go/src/net/http/server.go:1918 +0x44
github.com/influxdata/influxdb/services/httpd.(*Handler).recovery.func1(0x12a57a0, 0xc421ab27e0, 0xc420108600)
	/tmp/.gdm_32513/src/github.com/influxdata/influxdb/services/httpd/handler.go:1551 +0x14c
net/http.HandlerFunc.ServeHTTP(0xc4202e7c00, 0x12a57a0, 0xc421ab27e0, 0xc420108600)
	/usr/lib/go/src/net/http/server.go:1918 +0x44
github.com/bmizerany/pat.(*PatternServeMux).ServeHTTP(0xc4202e7960, 0x12a57a0, 0xc421ab27e0, 0xc420108600)
	/tmp/.gdm_26020/src/github.com/bmizerany/pat/mux.go:117 +0x15b
github.com/influxdata/influxdb/services/httpd.(*Handler).ServeHTTP(0xc4201f8a50, 0x12a57a0, 0xc421ab27e0, 0xc420108600)
	/tmp/.gdm_32513/src/github.com/influxdata/influxdb/services/httpd/handler.go:288 +0x24d
net/http.serverHandler.ServeHTTP(0xc420c90270, 0x12a57a0, 0xc421ab27e0, 0xc420108600)
	/usr/lib/go/src/net/http/server.go:2619 +0xb4
net/http.(*conn).serve(0xc4225a4280, 0x12a6de0, 0xc421920740)
	/usr/lib/go/src/net/http/server.go:1801 +0x71d
created by net/http.(*Server).Serve
	/usr/lib/go/src/net/http/server.go:2720 +0x288
Required for all non-trivial PRs
  • Rebased/mergable
  • Tests pass
  • CHANGELOG.md updated
  • Sign CLA (if not already signed)

@ghost ghost added the proposed label Jan 22, 2018
Copy link
Contributor

@jsternberg jsternberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you update the changelog and give yourself credit?

I'll merge after that and I think this needs to be backported to 1.4.

@jsternberg
Copy link
Contributor

Can you also add a test case for this? Thanks.

@phemmer
Copy link
Contributor Author

phemmer commented Jan 24, 2018

@jsternberg updated

@jsternberg
Copy link
Contributor

Looks good to me. I'll push the merge button the minute that CI completes successfully.

Thanks for the help.

@jsternberg jsternberg merged commit a19d3c2 into influxdata:master Jan 24, 2018
@ghost ghost removed the proposed label Jan 24, 2018
jsternberg pushed a commit that referenced this pull request Jan 24, 2018
@jsternberg
Copy link
Contributor

This is backported now. I believe we're planning to release 1.4.3 very soon so you should see it within the next week or so assuming something unexpected doesn't happen.

@phemmer phemmer deleted the fix-err-panic branch January 24, 2018 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants