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

Inconsistent content type check for cloudevent response #966

Closed
denismakogon opened this issue May 1, 2018 · 1 comment · Fixed by #968
Closed

Inconsistent content type check for cloudevent response #966

denismakogon opened this issue May 1, 2018 · 1 comment · Fixed by #968

Comments

@denismakogon
Copy link
Member

Problem happens here https://github.com/fnproject/fn/blob/master/api/agent/protocol/cloudevent.go#L194
Error:

"Error: Unknown content type: application/json; charset=utf-8\n" 

Trace:

"Error: Unknown content type: application/json; charset=utf-8\n" id=01CCDG7QJFR2M004RZJ000000Z route=/test-pr-branch stack="goroutine 5813 [running]:\nruntime/debug.Stack(0xc42060fbd0, 0x4e80ac0, 0xc4200c34b0)\n\t/usr/local/go/src/runtime/debug/stack.go:24 +0xa7\ngithub.com/fnproject/fn/api/server.HandleErrorResponse(0x4e8cbc0, 0xc420514ff0, 0x7285ae8, 0xc42016e210, 0x4e80ac0, 0xc4200c34b0)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/api/server/error_response.go:42 +0x429\ngithub.com/fnproject/fn/api/server.handleErrorResponse(0xc42016e210, 0x4e80ac0, 0xc4200c34b0)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/api/server/error_response.go:24 +0xa9\ngithub.com/fnproject/fn/api/server.(*Server).handleFunctionCall(0xc4203b8000, 0xc42016e210)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/api/server/runner.go:24 +0x66\ngithub.com/fnproject/fn/api/server.(*Server).(github.com/fnproject/fn/api/server.handleFunctionCall)-fm(0xc42016e210)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/api/server/server.go:879 +0x34\ngithub.com/fnproject/fn/vendor/github.com/gin-gonic/gin.(*Context).Next(0xc42016e210)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/vendor/github.com/gin-gonic/gin/context.go:104 +0x43\ngithub.com/fnproject/fn/api/server.(*Server).checkAppPresenceByNameAtRunner.func1(0xc42016e210)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/api/server/gin_middlewares.go:113 +0x12d\ngithub.com/fnproject/fn/vendor/github.com/gin-gonic/gin.(*Context).Next(0xc42016e210)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/vendor/github.com/gin-gonic/gin/context.go:104 +0x43\ngithub.com/fnproject/fn/api/server.(*Server).runMiddleware(0xc4203b8000, 0xc42016e210, 0x0, 0x0, 0x0)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/api/server/middleware.go:73 +0x29f\ngithub.com/fnproject/fn/api/server.(*Server).rootMiddlewareWrapper.func1(0xc42016e210)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/api/server/middleware.go:63 +0x58\ngithub.com/fnproject/fn/vendor/github.com/gin-gonic/gin.(*Context).Next(0xc42016e210)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/vendor/github.com/gin-gonic/gin/context.go:104 +0x43\ngithub.com/fnproject/fn/api/server.panicWrap(0xc42016e210)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/api/server/gin_middlewares.go:77 +0x51\ngithub.com/fnproject/fn/vendor/github.com/gin-gonic/gin.(*Context).Next(0xc42016e210)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/vendor/github.com/gin-gonic/gin/context.go:104 +0x43\ngithub.com/fnproject/fn/api/server.traceWrap(0xc42016e210)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/api/server/gin_middlewares.go:63 +0x3c3\ngithub.com/fnproject/fn/vendor/github.com/gin-gonic/gin.(*Context).Next(0xc42016e210)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/vendor/github.com/gin-gonic/gin/context.go:104 +0x43\ngithub.com/fnproject/fn/api/server.loggerWrap(0xc42016e210)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/api/server/gin_middlewares.go:94 +0x18e\ngithub.com/fnproject/fn/vendor/github.com/gin-gonic/gin.(*Context).Next(0xc42016e210)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/vendor/github.com/gin-gonic/gin/context.go:104 +0x43\ngithub.com/fnproject/fn/vendor/github.com/gin-gonic/gin.(*Engine).handleHTTPRequest(0xc4200ef680, 0xc42016e210)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/vendor/github.com/gin-gonic/gin/gin.go:332 +0x585\ngithub.com/fnproject/fn/vendor/github.com/gin-gonic/gin.(*Engine).ServeHTTP(0xc4200ef680, 0x4e8b500, 0xc4208ffaa0, 0xc420807400)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/vendor/github.com/gin-gonic/gin/gin.go:296 +0x153\ngithub.com/fnproject/fn/vendor/go.opencensus.io/plugin/ochttp.(*Handler).ServeHTTP(0xc4204e16c0, 0x4e8b500, 0xc4208ffaa0, 0xc420807400)\n\t/Users/denismakogon/Documents/oracle/go/src/github.com/fnproject/fn/vendor/go.opencensus.io/plugin/ochttp/server.go:73 +0x109\nnet/http.serverHandler.ServeHTTP(0xc42038c9c0, 0x4e8bc40, 0xc420728a80, 0xc420807300)\n\t/usr/local/go/src/net/http/server.go:2694 +0xbc\nnet/http.(*conn).serve(0xc420507360, 0x4e8cb00, 0xc420900940)\n\t/usr/local/go/src/net/http/server.go:1830 +0x651\ncreated by net/http.(*Server).Serve\n\t/usr/local/go/src/net/http/server.go:2795 +0x27b\n"
@rdallman
Copy link
Contributor

rdallman commented May 1, 2018

there's code to use https://golang.org/src/mime/mediatype.go?s=2954:3039#L102 laying around fn, we just need to more consistently use it.

rdallman pushed a commit that referenced this issue May 2, 2018
related: fnproject/fdk-go#26

adds a test for the protocol dumping of a request to the container stdin.
there are a number of vectors to test for a cloud event, but since we're going
to change that behavior soon it's probably a waste of time to go about doing
so. in any event, this was pretty broken. my understanding of the cloud event
spec is deepening and the json stuff overall seems like a pretty poor choice
so far.

* fixes content type issue around json checking (since a string is also a json
value, we can just decode it, even though it's wasteful it's more easily
correct)
* doesn't force all json values to be map[string]interface{} and lets them be
whoever they want to be. maybe their dads are still proud.

closes #966
rdallman pushed a commit that referenced this issue May 7, 2018
related: fnproject/fdk-go#26

adds a test for the protocol dumping of a request to the container stdin.
there are a number of vectors to test for a cloud event, but since we're going
to change that behavior soon it's probably a waste of time to go about doing
so. in any event, this was pretty broken. my understanding of the cloud event
spec is deepening and the json stuff overall seems like a pretty poor choice
so far.

* fixes content type issue around json checking (since a string is also a json
value, we can just decode it, even though it's wasteful it's more easily
correct)
* doesn't force all json values to be map[string]interface{} and lets them be
whoever they want to be. maybe their dads are still proud.

closes #966
rdallman added a commit that referenced this issue May 7, 2018
adds a test for the protocol dumping of a request to the containert stdin.
there are a number of vectors to test for a cloud event, but since we're going
to change that behavior soon it's probably a waste of time to go about doing
so. in any event, this was pretty broken. my understanding of the cloud event
spec is deepening and the json stuff overall seems a little weird.

* fixes content type issue around json checking (since a string is also a json
value, we can just decode it, even though it's wasteful it's more easily
correct)
* doesn't force all json values to be map[string]interface{} and lets them be
whoever they want to be. maybe their dads are still proud.

closes #966
rdallman added a commit that referenced this issue May 8, 2018
adds a test for the protocol dumping of a request to the containert stdin.
there are a number of vectors to test for a cloud event, but since we're going
to change that behavior soon it's probably a waste of time to go about doing
so. in any event, this was pretty broken. my understanding of the cloud event
spec is deepening and the json stuff overall seems a little weird.

* fixes content type issue around json checking (since a string is also a json
value, we can just decode it, even though it's wasteful it's more easily
correct)
* doesn't force all json values to be map[string]interface{} and lets them be
whoever they want to be. maybe their dads are still proud.

closes #966
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 a pull request may close this issue.

2 participants