Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Stricter JSON body validation; run graphql-over-HTTP test suite #7171

Merged
merged 3 commits into from Nov 21, 2022
Merged

Stricter JSON body validation; run graphql-over-HTTP test suite #7171

merged 3 commits into from Nov 21, 2022

Commits on Nov 21, 2022

  1. Stricter JSON body validation; run graphql-over-HTTP test suite 

    This PR adds the spec audit suite from `graphql-http` (designed to
validate compatibility with the "GraphQL over HTTP" specification) to
the integration test suite. It expects all required (MUST) audits to
pass, and for all optional (MAY/SHOULD) audits to pass that we
haven't explicitly excepted.

All required audits already passed. Failing optional audits fell into
three categories:

- Ignoring `operationName`, `variables`, and `extensions` when provided
  with incorrect types instead of returning a 400 error. The SHOULD here
  seemed reasonable and so this PR changes our POST handler to return
  400s if incorrect types are provided here. As described in the
  changeset, this is backwards incompatible but we are comfortable
  making this choice today.
- Returning 400 errors for various error conditions like GraphQL parse
  errors, for the original `application/json` response type. The one
  aspect of the GoH spec that is prescriptive rather than descriptive is
  the invention of the `application/graphql-response+json` response MIME
  type; the theory is that you can't really "trust" that a non-2xx
  response with MIME type `application/json` is actually generated by
  the GraphQL server rather than by some other proxy, so it has you use
  this new type along with 4xx for these errors, and then SHOULDs that
  these errors come with 200 for the original MIME type. The main reason
  that this is a SHOULD is because many servers like Apollo Server
  already return 400s in this case, and we're not interested in changing
  that. So we ignore these particular errors.
- There's a sorta buggy audit that has opinions about how bad JSON
  errors should look. See
  graphql/graphql-http#25

Note that we deliberately make a strict dependency on a single version
of graphql-http, so that the tests run by a given version of
`@apollo/server-integration-test-suite` are well defined.

Fixes #7158.
    @glasser
    glasser committed Nov 21, 2022
    Configuration menu
    Copy the full SHA
    f1296a0 View commit details
    Browse the repository at this point in the history

  2. graphql-http audit url option can be a function (#7180) 

    graphql/graphql-http#24
    @enisdenjo @glasser
    enisdenjo authored and glasser committed Nov 21, 2022
    Configuration menu
    Copy the full SHA
    69b2d1e View commit details
    Browse the repository at this point in the history

  3. Simplify declaration 

    To be honest I'm not sure why this is valid (TS has no way of knowing
that the beforeAll callback gets called before the others?) but hey, it
works and if it's ever not valid tests would fail.
    @glasser
    glasser committed Nov 21, 2022
    Configuration menu
    Copy the full SHA
    56684d9 View commit details
    Browse the repository at this point in the history