Skip to content

Commit a825e2e

Browse files
authored
Fix panics on non-GET/non-POST requests (#186)
Returns 405 Method Not Allowed instead of panicking. See the working draft spec: https://github.com/graphql/graphql-over-http/blob/e4540976487b77bc04f1b2ce5cc48a9beea49381/spec/GraphQLOverHTTP.md?plain=1#L205-L206
1 parent c26b382 commit a825e2e

File tree

2 files changed

+50
-17
lines changed

2 files changed

+50
-17
lines changed

http.go

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,12 @@ func formatErrorsWithCode(data map[string]interface{}, err error, code string) m
5454
// a single object with { query, variables, operationName } or a list
5555
// of that object.
5656
func (g *Gateway) GraphQLHandler(w http.ResponseWriter, r *http.Request) {
57-
operations, batchMode, payloadErr := parseRequest(r)
57+
operations, batchMode, parseStatusCode, payloadErr := parseRequest(r)
5858

5959
// if there was an error retrieving the payload
6060
if payloadErr != nil {
6161
response := formatErrors(payloadErr)
62-
w.WriteHeader(http.StatusUnprocessableEntity)
62+
w.WriteHeader(parseStatusCode)
6363
err := json.NewEncoder(w).Encode(response)
6464
if err != nil {
6565
g.logger.Warn("Failed to encode error response:", err.Error())
@@ -165,26 +165,24 @@ func (g *Gateway) GraphQLHandler(w http.ResponseWriter, r *http.Request) {
165165
emitResponse(w, statusCode, string(response))
166166
}
167167

168-
// Parses request to operations (single or batch mode)
169-
func parseRequest(r *http.Request) (operations []*HTTPOperation, batchMode bool, payloadErr error) {
168+
// Parses request to operations (single or batch mode).
169+
// Returns an error and an error status code if the request is invalid.
170+
func parseRequest(r *http.Request) (operations []*HTTPOperation, batchMode bool, errStatusCode int, payloadErr error) {
170171
// this handler can handle multiple operations sent in the same query. Internally,
171-
// it modules a single operation as a list of one.
172+
// it models a single operation as a list of one.
172173
operations = []*HTTPOperation{}
173-
174-
// the error we have encountered when extracting query input
175-
176-
// make our lives easier. track if we're in batch mode
177-
batchMode = false
178-
179-
if r.Method == http.MethodGet {
180-
// if we got a GET request
174+
switch r.Method {
175+
case http.MethodGet:
181176
operations, payloadErr = parseGetRequest(r)
182-
183-
} else if r.Method == http.MethodPost {
184-
// or we got a POST request
177+
case http.MethodPost:
185178
operations, batchMode, payloadErr = parsePostRequest(r)
179+
default:
180+
errStatusCode = http.StatusMethodNotAllowed
181+
payloadErr = errors.New(http.StatusText(http.StatusMethodNotAllowed))
182+
}
183+
if errStatusCode == 0 && payloadErr != nil { // ensure an error always results in a failed status code
184+
errStatusCode = http.StatusUnprocessableEntity
186185
}
187-
188186
return
189187
}
190188

http_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1448,3 +1448,38 @@ func TestStaticPlaygroundHandler(t *testing.T) {
14481448
assert.Equal(t, http.StatusMethodNotAllowed, result.StatusCode)
14491449
})
14501450
}
1451+
1452+
// Conforms to working draft spec for GraphQL over HTTP: https://github.com/graphql/graphql-over-http/blob/e4540976487b77bc04f1b2ce5cc48a9beea49381/spec/GraphQLOverHTTP.md?plain=1#L205-L206
1453+
func TestGraphQLHandler_OptionsMethod(t *testing.T) {
1454+
t.Parallel()
1455+
planner := &MockErrPlanner{Err: errors.New("Planning error")}
1456+
schema, err := graphql.LoadSchema(`
1457+
type Query {
1458+
allUsers: [String!]!
1459+
}
1460+
`)
1461+
assert.NoError(t, err)
1462+
schemas := []*graphql.RemoteSchema{{Schema: schema, URL: "url1"}}
1463+
gateway, err := New(schemas, WithPlanner(planner))
1464+
if err != nil {
1465+
t.Error(err.Error())
1466+
return
1467+
}
1468+
request := httptest.NewRequest(http.MethodOptions, "/graphql", nil)
1469+
response := httptest.NewRecorder()
1470+
1471+
gateway.GraphQLHandler(response, request)
1472+
assert.Equal(t, http.StatusMethodNotAllowed, response.Code, "Unhandled HTTP method should return 405 Method Not Allowed, got:", response.Code)
1473+
assert.JSONEq(t, `
1474+
{
1475+
"data": null,
1476+
"errors": [
1477+
{
1478+
"extensions": {
1479+
"code": "UNKNOWN_ERROR"
1480+
},
1481+
"message": "Method Not Allowed"
1482+
}
1483+
]
1484+
}`, response.Body.String())
1485+
}

0 commit comments

Comments
 (0)