Graphql-go: Support query with GET

[steveazz]

Overview

Hello, first I would like to thank you for the awesome project and all the work that is put into it.

I was wondering is there any reason that GET is not supported for query as specified here as you can see the query is passed as a query string http://myapi/graphql?query={me{name}}

Benefits

If we allow GET requests it would be much easier to use caching mechanism such as varnish, cloudflare and other normal HTTP caching

Implementation

In the HTTP handler do a check we check if the request is a get request and only parse the the key query and we will populate only Query inside of the params struct. Would there be any limitations doing it like this?

func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    var params struct {
        Query         string                 `json:"query"`
        OperationName string                 `json:"operationName"`
        Variables     map[string]interface{} `json:"variables"`
    }

    if r.Method == http.MethodGet {
        params.Query = r.URL.Query().Get("query")
    } 

       // Skip this part
    if err := json.NewDecoder(r.Body).Decode(&params); err != nil {
        http.Error(w, err.Error(), http.StatusBadRequest)
        return
        }
    response := h.Schema.Exec(r.Context(), params.Query, params.OperationName, params.Variables)
    responseJSON, err := json.Marshal(response)
    if err != nil {
        http.Error(w, err.Error(), http.StatusInternalServerError)
        return
    }

    w.Header().Set("Content-Type", "application/json")
    w.Write(responseJSON)
}

[tonyghita]

Hey, I'm a bit confused by your question. This is possible! I implement both GET and POST handlers for my GraphQL endpoint.

In fact, that seems to be a best practice.

[nicksrandall]

In general, I think people would only want to allow “GET” requests for “Query” Graphql queries and not “Mutations”. Otherwise there is a pretty severe security risk. Maybe it makes sense to make a function that parses the query with operationName just to figure out if it’s a query or mutation?

[tonyghita]

@nicksrandall what would the security risks be? I'd imagine security is agnostic of HTTP method

[nicksrandall]

@tonyghita it totally depends on the domain of the software. An example could be a malicious user could send an innocent user an image with "src" of graphql query and then that query would be executed in context of innocent user without them knowing and potentially granting the malicous user access to something sensitive?

[steveazz]

@tonyghita

Hey, I'm a bit confused by your question. This is possible! I implement both GET and POST handlers for my GraphQL endpoint.

With master? When I tried to send a GET request I kept getting 400 Bad Request and the reason being the current handler only handles POST requests, it cerinatly can be modified, I was just asking if it simple as that or if I am missing something.

I can open a PR for this to add support for it.

In general, I think people would only want to allow “GET” requests for “Query” Graphql queries and not “Mutations”

I agree with you doesn't seem like it is best practice to add mutations in a GET request, even the article @tonyghita linked too only shows the query.

[tonyghita]

Ah @SteveAzz you were asking particularly about the included handler. I understand now!
@nicksrandall true... a co-worker mentioned you also open up possibility of MITM over HTTPS.

I wonder if the "best practice" should be re-visited.

[nicksrandall]

So, like I said before, this does bring up an interesting use case for a function to tell if request is a "query" or "mutation"?

[steveazz]

So, like I said before, this does bring up an interesting use case for a function to tell if the request is a "query" or "mutation"?

What do you mean by that? It doesn't seem like the current handler in master supports mutations unless I am missing something? Are we suppose to use that handler or is it best practice to use our own handler?

Anyways, it doesn't matter, query and mutation have to be under their respective parent object, meaning if it a query it will the request will look like http://myapi/graphql?query={me{name}} we can simply ignore the mutation key from the query string.

func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    var params struct {
        Query         string                 `json:"query"`
        OperationName string                 `json:"operationName"`
        Variables     map[string]interface{} `json:"variables"`
    }
    if err := json.NewDecoder(r.Body).Decode(&params); err != nil {
        http.Error(w, err.Error(), http.StatusBadRequest)
        return
    }

    response := h.Schema.Exec(r.Context(), params.Query, params.OperationName, params.Variables)
    responseJSON, err := json.Marshal(response)
    if err != nil {
        http.Error(w, err.Error(), http.StatusInternalServerError)
        return
    }

    w.Header().Set("Content-Type", "application/json")
    w.Write(responseJSON)
}

[pavelnikolov]

The caching example already show how to handle both GET and POST requests