Skip to content

Commit

Permalink
Fix: ensure that status codes are checked in any case
Browse files Browse the repository at this point in the history
If this check is missing then responses from the server with the graphql end-point  that are not compliant to the graphql spec will be just ignored (not treated as an error). 
One reason for this behavior of the server could be (e.g. springboot) that the request is already rejected before it reaches the graphql end-point due to missing authorization. In this case the security filter that does not know about graphql responds with an error that is not compliant to the GQL spec.
The same situation would be in case of a 404, 403 etc.
  • Loading branch information
ThomasObenaus authored and setnicka committed Feb 2, 2023
1 parent 3a92531 commit fabd9b3
Showing 1 changed file with 25 additions and 0 deletions.
25 changes: 25 additions & 0 deletions graphql.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ func (c *Client) runWithJSON(ctx context.Context, req *Request, resp interface{}
return errors.Wrap(err, "reading body")
}
c.logf("<< %s", buf.String())
bodyStr := buf.String()

if err := json.NewDecoder(&buf).Decode(&gr); err != nil {
if res.StatusCode != http.StatusOK {
return fmt.Errorf("graphql: server returned a non-200 status code: %v", res.StatusCode)
Expand All @@ -147,6 +149,17 @@ func (c *Client) runWithJSON(ctx context.Context, req *Request, resp interface{}
// return first error
return gr.Errors[0]
}

// Handle the http status codes before handling response from the graphql endpoint.
// If this is not done then !200 status codes will just be ignored without the caller even noticing, instead
// the caller just gets back an empty result set, suggesting that the query did not found any result.
// The reason for this is that for example in case of a 404,401,403 etc. the request is rejected before
// it even hits an graphql handler on the server side.
// As a result the response returned by this non graphql component is not compliant to the graphql spec.
if res.StatusCode != http.StatusOK {
return fmt.Errorf("graphql: server returned a non-200 status code: %v - %v", res.StatusCode, bodyStr)
}

return nil
}

Expand Down Expand Up @@ -208,6 +221,8 @@ func (c *Client) runWithPostFields(ctx context.Context, req *Request, resp inter
return errors.Wrap(err, "reading body")
}
c.logf("<< %s", buf.String())
bodyStr := buf.String()

if err := json.NewDecoder(&buf).Decode(&gr); err != nil {
if res.StatusCode != http.StatusOK {
return fmt.Errorf("graphql: server returned a non-200 status code: %v", res.StatusCode)
Expand All @@ -218,6 +233,16 @@ func (c *Client) runWithPostFields(ctx context.Context, req *Request, resp inter
// return first error
return gr.Errors[0]
}

// Handle the http status codes before handling response from the graphql endpoint.
// If this is not done then !200 status codes will just be ignored without the caller even noticing, instead
// the caller just gets back an empty result set, suggesting that the query did not found any result.
// The reason for this is that for example in case of a 404,401,403 etc. the request is rejected before
// it even hits an graphql handler on the server side.
// As a result the response returned by this non graphql component is not compliant to the graphql spec.
if res.StatusCode != http.StatusOK {
return fmt.Errorf("graphql: server returned a non-200 status code: %v - %v", res.StatusCode, bodyStr)
}
return nil
}

Expand Down

0 comments on commit fabd9b3

Please sign in to comment.