fix: safe http error response

[koniuszy]

I cannot intercept the error of an invalid request using the hooks. I always get:

{
  "errors": [
    {
      "message": "json body could not be decoded: json: cannot unmarshal array into Go value of type graphql.RawParams"
    }
  ],
  "data": null
}

This change removes from the response the details of a decoding error, and instead returns a StatusBadRequest with a more generic decoding error. The more specific error is instead logged.

Before:

{"errors":[{"message":"json body could not be decoded: invalid character 'o' in literal null (expecting 'u')"}],"data":null}

After:

{"errors":[{"message":"json body could not be decoded"}],"data":null}

I have:

• Added tests covering the bug / feature (see testing)
• Updated any relevant documentation (see docs)

Solved:
#2430

[coveralls]

Coverage decreased (-0.04%) to 75.395% when pulling 142fc3b on koniuszy:patch-1 into f28ffcc on 99designs:master.

[StevenACoffman]

Hi. Rather than remove the displaying information from those who want it, can we instead look for a way to avoid having the hook be unable to parse it?

Do you have a small repository where you can reproduce the original problem?

[koniuszy]

hey @StevenACoffman, thanks for the reply,

Initially I tried to control the error with the hooks but to no avail.
This could be controlled by a config flag or some sort of an error hook. At the moment it's a security leak and in my opinion it should be disabled by default asap.

If it does not persuade you, please point the direction so I could at least try implementing what's requested.

[crossworth]

Hello @koniuszy, maybe another way to solve this would to implement a custom graphql.Transport and use it instead of the default transport.POST.

srv.AddTransport(transport.MyCustomPOST{})

[StevenACoffman]

Thanks! So there were actually multiple problems that this uncovered.

The main one was that decoding errors were not dispatched, so the hooks had no ability to intercept errors to present the error as you wished. There was also a potential nil pointer dereference which would cause panic and bypass the error presenter hook (although the recover panic would catch that one).

I modified your example repository to add a hook. With that change and using this updated PR to gqlgen it will not present the details you wanted to be obscured:

const defaultPort = "8080"

func main() {
	port := os.Getenv("PORT")
	if port == "" {
		port = defaultPort
	}

	srv := handler.NewDefaultServer(MakeExecutableSchema(&graph.Resolver{}))
	srv.SetErrorPresenter(func(ctx context.Context, e error) *gqlerror.Error {
		fmt.Printf("Got error here: %+v", e)
		err := graphql.DefaultErrorPresenter(ctx, e)
		err.Message = "Eeek!"

		return err
	})

	http.Handle("/", playground.Handler("GraphQL playground", "/query"))
	http.Handle("/query", srv)

	log.Printf("connect to http://localhost:%s/ for GraphQL playground", port)
	log.Fatal(http.ListenAndServe(":"+port, nil))
}

func MakeExecutableSchema(resolvers *graph.Resolver) graphql.ExecutableSchema {
	return generated.NewExecutableSchema(generated.Config{Resolvers: resolvers})
}

➜  ~ curl --compressed -i 'http://localhost:8080/query' -X POST -H 'Content-Type: application/json' --data-raw '[]'
HTTP/1.1 400 Bad Request
Content-Type: application/json
Date: Sat, 03 Dec 2022 18:43:52 GMT
Content-Length: 44

{"errors":[{"message":"Eeek!"}],"data":null}