From f97fb5e3daa8c973ed636cabde8ea2f1f6d5e3b3 Mon Sep 17 00:00:00 2001 From: Koniuszy <44303865+koniuszy@users.noreply.github.com> Date: Mon, 28 Nov 2022 12:06:00 -0500 Subject: [PATCH 1/6] safe http error when parsing body --- graphql/handler/transport/http_post.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/graphql/handler/transport/http_post.go b/graphql/handler/transport/http_post.go index 99c3157185..6530b0bbe5 100644 --- a/graphql/handler/transport/http_post.go +++ b/graphql/handler/transport/http_post.go @@ -3,7 +3,7 @@ package transport import ( "mime" "net/http" - + "log" "github.com/99designs/gqlgen/graphql" ) @@ -33,7 +33,8 @@ func (h POST) Do(w http.ResponseWriter, r *http.Request, exec graphql.GraphExecu start := graphql.Now() if err := jsonDecode(r.Body, ¶ms); err != nil { w.WriteHeader(http.StatusBadRequest) - writeJsonErrorf(w, "json body could not be decoded: "+err.Error()) + log.Printf("decoding error: %s", err.Error()) + writeJsonErrorf(w, "json body could not be decoded") return } From cd2ff3cb8adaff488377a6c0318ce6e5271890da Mon Sep 17 00:00:00 2001 From: Koniuszy <44303865+koniuszy@users.noreply.github.com> Date: Mon, 28 Nov 2022 12:09:17 -0500 Subject: [PATCH 2/6] fix tests --- graphql/handler/transport/http_post_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphql/handler/transport/http_post_test.go b/graphql/handler/transport/http_post_test.go index b27ffb3977..7a5cb6bdb6 100644 --- a/graphql/handler/transport/http_post_test.go +++ b/graphql/handler/transport/http_post_test.go @@ -26,7 +26,7 @@ func TestPOST(t *testing.T) { resp := doRequest(h, "POST", "/graphql", "notjson") assert.Equal(t, http.StatusBadRequest, resp.Code, resp.Body.String()) assert.Equal(t, resp.Header().Get("Content-Type"), "application/json") - assert.Equal(t, `{"errors":[{"message":"json body could not be decoded: invalid character 'o' in literal null (expecting 'u')"}],"data":null}`, resp.Body.String()) + assert.Equal(t, `{"errors":[{"message":"json body could not be decoded"}],"data":null}`, resp.Body.String()) }) t.Run("parse failure", func(t *testing.T) { From 5384d833ccf051339f17e44f05da8f6a8d0c5d52 Mon Sep 17 00:00:00 2001 From: Koniuszy <44303865+koniuszy@users.noreply.github.com> Date: Mon, 28 Nov 2022 14:08:17 -0500 Subject: [PATCH 3/6] fix linting --- graphql/handler/transport/http_post.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/graphql/handler/transport/http_post.go b/graphql/handler/transport/http_post.go index 6530b0bbe5..efefbf13b2 100644 --- a/graphql/handler/transport/http_post.go +++ b/graphql/handler/transport/http_post.go @@ -1,10 +1,10 @@ package transport import ( - "mime" - "net/http" - "log" - "github.com/99designs/gqlgen/graphql" + "github.com/99designs/gqlgen/graphql" + "log" + "mime" + "net/http" ) // POST implements the POST side of the default HTTP transport From efe80257efd22a08443ae8214447dfca1afe299b Mon Sep 17 00:00:00 2001 From: Koniuszy <44303865+koniuszy@users.noreply.github.com> Date: Thu, 1 Dec 2022 15:49:17 -0500 Subject: [PATCH 4/6] fix linting --- graphql/handler/transport/http_post.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/graphql/handler/transport/http_post.go b/graphql/handler/transport/http_post.go index efefbf13b2..dd129bd174 100644 --- a/graphql/handler/transport/http_post.go +++ b/graphql/handler/transport/http_post.go @@ -1,10 +1,10 @@ package transport import ( - "github.com/99designs/gqlgen/graphql" - "log" - "mime" - "net/http" + "github.com/99designs/gqlgen/graphql" + "log" + "mime" + "net/http" ) // POST implements the POST side of the default HTTP transport From 0ccd68f65ee4039a80ee51aa3792a9ade646241b Mon Sep 17 00:00:00 2001 From: Steve Coffman Date: Sat, 3 Dec 2022 14:40:08 -0500 Subject: [PATCH 5/6] Dispatch decoding errors so hook can present them Signed-off-by: Steve Coffman --- graphql/handler/transport/http_post.go | 63 ++++++++++++++++++++------ 1 file changed, 48 insertions(+), 15 deletions(-) diff --git a/graphql/handler/transport/http_post.go b/graphql/handler/transport/http_post.go index dd129bd174..28847754b9 100644 --- a/graphql/handler/transport/http_post.go +++ b/graphql/handler/transport/http_post.go @@ -1,10 +1,16 @@ package transport import ( - "github.com/99designs/gqlgen/graphql" + "fmt" + "io" "log" "mime" "net/http" + "strings" + + "github.com/vektah/gqlparser/v2/gqlerror" + + "github.com/99designs/gqlgen/graphql" ) // POST implements the POST side of the default HTTP transport @@ -26,32 +32,59 @@ func (h POST) Supports(r *http.Request) bool { return r.Method == "POST" && mediaType == "application/json" } +func getRequestBody(r *http.Request) (string, error) { + if r == nil || r.Body == nil { + return "", nil + } + body, err := io.ReadAll(r.Body) + if err != nil { + return "", fmt.Errorf("unable to get Request Body %w", err) + } + return string(body), nil +} + func (h POST) Do(w http.ResponseWriter, r *http.Request, exec graphql.GraphExecutor) { + ctx := r.Context() w.Header().Set("Content-Type", "application/json") - - var params *graphql.RawParams + params := &graphql.RawParams{} start := graphql.Now() - if err := jsonDecode(r.Body, ¶ms); err != nil { - w.WriteHeader(http.StatusBadRequest) - log.Printf("decoding error: %s", err.Error()) - writeJsonErrorf(w, "json body could not be decoded") - return - } - params.Headers = r.Header - params.ReadTime = graphql.TraceTiming{ Start: start, End: graphql.Now(), } - rc, err := exec.CreateOperationContext(r.Context(), params) + bodyString, err := getRequestBody(r) if err != nil { - w.WriteHeader(statusFor(err)) - resp := exec.DispatchError(graphql.WithOperationContext(r.Context(), rc), err) + gqlErr := gqlerror.Errorf("could not get json request body: %+v", err) + resp := exec.DispatchError(ctx, gqlerror.List{gqlErr}) + log.Printf("could not get json request body: %+v", err.Error()) + writeJson(w, resp) + } + + bodyReader := io.NopCloser(strings.NewReader(bodyString)) + if err = jsonDecode(bodyReader, ¶ms); err != nil { + w.WriteHeader(http.StatusBadRequest) + gqlErr := gqlerror.Errorf( + "json request body could not be decoded: %+v body:%s", + err, + bodyString, + ) + resp := exec.DispatchError(ctx, gqlerror.List{gqlErr}) + log.Printf("decoding error: %+v body:%s", err.Error(), bodyString) + writeJson(w, resp) + return + } + + rc, OpErr := exec.CreateOperationContext(ctx, params) + if OpErr != nil { + w.WriteHeader(statusFor(OpErr)) + resp := exec.DispatchError(graphql.WithOperationContext(ctx, rc), OpErr) writeJson(w, resp) return } - responses, ctx := exec.DispatchOperation(r.Context(), rc) + + var responses graphql.ResponseHandler + responses, ctx = exec.DispatchOperation(ctx, rc) writeJson(w, responses(ctx)) } From 142fc3b645d57629e0f7f0d0385f0e770d35090b Mon Sep 17 00:00:00 2001 From: Steve Coffman Date: Sat, 3 Dec 2022 14:50:44 -0500 Subject: [PATCH 6/6] Revert test expectation to original Signed-off-by: Steve Coffman --- graphql/handler/transport/http_post_test.go | 2 +- .../resolvergen/testdata/filetemplate/out/schema.custom.go | 7 ++++--- .../testdata/followschema/out/schema.resolvers.go | 7 ++++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/graphql/handler/transport/http_post_test.go b/graphql/handler/transport/http_post_test.go index 7a5cb6bdb6..b104118e6f 100644 --- a/graphql/handler/transport/http_post_test.go +++ b/graphql/handler/transport/http_post_test.go @@ -26,7 +26,7 @@ func TestPOST(t *testing.T) { resp := doRequest(h, "POST", "/graphql", "notjson") assert.Equal(t, http.StatusBadRequest, resp.Code, resp.Body.String()) assert.Equal(t, resp.Header().Get("Content-Type"), "application/json") - assert.Equal(t, `{"errors":[{"message":"json body could not be decoded"}],"data":null}`, resp.Body.String()) + assert.Equal(t, `{"errors":[{"message":"json request body could not be decoded: invalid character 'o' in literal null (expecting 'u') body:notjson"}],"data":null}`, resp.Body.String()) }) t.Run("parse failure", func(t *testing.T) { diff --git a/plugin/resolvergen/testdata/filetemplate/out/schema.custom.go b/plugin/resolvergen/testdata/filetemplate/out/schema.custom.go index c1412cb918..d7917ee998 100644 --- a/plugin/resolvergen/testdata/filetemplate/out/schema.custom.go +++ b/plugin/resolvergen/testdata/filetemplate/out/schema.custom.go @@ -2,6 +2,7 @@ package customresolver // This file will be automatically regenerated based on the schema, any resolver implementations // will be copied through when generating and any unknown code will be moved to the end. +// Code generated by github.com/99designs/gqlgen version v0.17.20-dev DO NOT EDIT. import ( "context" @@ -35,9 +36,9 @@ type resolverCustomResolverType struct{ *CustomResolverType } // !!! WARNING !!! // The code below was going to be deleted when updating resolvers. It has been copied here so you have // one last chance to move it out of harms way if you want. There are two reasons this happens: -// - When renaming or deleting a resolver the old code will be put in here. You can safely delete -// it when you're done. -// - You have helper methods in this file. Move them out to keep these resolver files clean. +// - When renaming or deleting a resolver the old code will be put in here. You can safely delete +// it when you're done. +// - You have helper methods in this file. Move them out to keep these resolver files clean. func AUserHelperFunction() { // AUserHelperFunction implementation } diff --git a/plugin/resolvergen/testdata/followschema/out/schema.resolvers.go b/plugin/resolvergen/testdata/followschema/out/schema.resolvers.go index c1412cb918..d7917ee998 100644 --- a/plugin/resolvergen/testdata/followschema/out/schema.resolvers.go +++ b/plugin/resolvergen/testdata/followschema/out/schema.resolvers.go @@ -2,6 +2,7 @@ package customresolver // This file will be automatically regenerated based on the schema, any resolver implementations // will be copied through when generating and any unknown code will be moved to the end. +// Code generated by github.com/99designs/gqlgen version v0.17.20-dev DO NOT EDIT. import ( "context" @@ -35,9 +36,9 @@ type resolverCustomResolverType struct{ *CustomResolverType } // !!! WARNING !!! // The code below was going to be deleted when updating resolvers. It has been copied here so you have // one last chance to move it out of harms way if you want. There are two reasons this happens: -// - When renaming or deleting a resolver the old code will be put in here. You can safely delete -// it when you're done. -// - You have helper methods in this file. Move them out to keep these resolver files clean. +// - When renaming or deleting a resolver the old code will be put in here. You can safely delete +// it when you're done. +// - You have helper methods in this file. Move them out to keep these resolver files clean. func AUserHelperFunction() { // AUserHelperFunction implementation }