Client preset uses a different hash than apollo server

[maclockard]

Which packages are impacted by your issue?

@graphql-codegen/client-preset

Describe the bug

Currently client preset uses sha1 while Apollo Server uses sha256 with no plans to make it configurable.

Ideally the exact hashing behavior here would be configurable without me needing to use unstable_onExecutableDocumentNode or rewrite my own version of ClientSideBaseVisitor.

Ideally, I would like to write my own plugin similar to graphql-codegen-persisted-query-ids that could operate on the document nodes generated by upstream plugins. Currently, the documents passed into a plugin are the 'raw' documents from source, but ideally the could be the documents generated by 'upstream' plugins.

The benefit of allowing me access to the generated documents in a plugin is it would allow me to use whatever framework specific hashing I need to do without y'all needing to support every single possible target out there.

Your Example Website or App

N/A linked directly to relevant code above

Steps to Reproduce the Bug or Issue

N/A linked directly to relevant code above

Expected behavior

At a minimum the hash would match what Apollo Server is using since you refer to the "APQ Specification of Apollo" in your current docs: https://the-guild.dev/graphql/yoga-server/docs/features/automatic-persisted-queries.

Best case, I would be able to write a plugin that hashes the document nodes myself without using unstable APIs or effectively duplicating core code.

Screenshots or Videos

No response

Platform

• OS: macOS
• NodeJS: 16.19.0
• graphql version: 16.6.0
• @graphql-codegen/* version(s): 3.2.1

Codegen Config File

No response

Additional context

No response

[alanquigley-toast]

You should check that the server and the client use the same version of graphql.

[maclockard]

They do, I'm forcing a resolution to 16.6.0 for both server and client using yarn resolutions.

If you view my links above, its pretty clear that client preset is generating the hash using sha1 while apollo server uses sha256. AFAIK these hashing algorithms will produce different output on the same input.

Currently client preset uses sha1 while Apollo Server uses sha256 with no plans to make it configurable.

[maclockard]

Again, ideally, this hashing would be possible in a plugin so y'all don't need to maintain compatability with every single possible GraphQL server. While Apollo Server has a pretty strict requirement with sha256, I have no clue what other servers require.

For this hashing to be done in a plugin, either the 'executable document' node needs to be passed as a plugin argument or some sort of stable version of unstable_onExecutableDocumentNode should be exposed. Otherwise, you can't guarantee the hash generated at build time will use the exact same document content as what is used by the client at runtime.

[n1ru4l]

Does apollo server complain about the hashes (e.g. raise an error)? In the end it is just a hashtable lookup so the actual hashing algorithm should be irrelevant for the server.

[maclockard]

Yes, apollo server complains about the hashes. It does server side verification to make sure folks are not passing mismatched hashes and queries here: https://github.com/apollographql/apollo-server/blob/8cbc61406229653454e50ea98f11dbe834e036b5/packages/server/src/requestPipeline.ts#L158-L164

I think this is a good idea since it prevents potential foot guns of folks incorrectly caching queries that are missing fields etc. It would not surprise me if other server implementations were to also do this.

[maclockard]

Also, FWIW I've been trying to use unstable_onExecutableDocumentNode myself, but ran into a couple issues that I think will lead to correctness issues with the client preset. Specifically:

1. ClientSideBaseVisitor does not respect the passed nonOptionalTypename config option. The typescript plugin does, resulting in the generated hash not accounting for the __typename field being included in the generated query documents. This leads to apollo server (correctly) rejecting the request since the hash does not match.
2. ClientSideBaseVisitor orders the fragments included in the document in a different order than the typescript plugin, leading to the hash not matching the query as well.

Part of this seems to be due the fact that each plugin executes independently rather than as a pipeline (like webpack for example). This makes it really easy for there to be disagreement between plugins about what the 'final' executable document should look like. This disagreement makes hashing very error prone and could cause all sorts of problems down the road.

I realize that it would not be a trivial change, but conceptually a pipeline architecture would both be more performant and easier to extend via plugins rather than having each plugin recompute/regenerate everything themselves.

[n1ru4l]

As the option unstable_onExecutableDocumentNode says, you should not use it. It is only there as an escape hatch for us to implement persisted operations extraction.

If you want to alter the document's nonOptionalTypename never intend that the document is modified, it is only intended for the generated TypeScript types. If you want to transform/modify the documents, you need to use a document transform.

I don't get why Apollo Server needs to verify the hash and the document string, there is no security issue as the server owner needs to provide the persisted operation hash map?

Reading what you are writing I think you are talking about Automatic Persisted Queries (APQ), which are not the same as persisted documents and are not related to this codegen option.

Aside from that, I am happy to accept a PR that allows passing a custom hashing function, e.g. like the following:

import { type CodegenConfig } from '@graphql-codegen/cli'

const hashFn = (document: DocumentNode): string => {
  // whatever u wanna do here
}
 
const config: CodegenConfig = {
  schema: 'schema.graphql',
  documents: ['src/**/*.tsx'],
  generates: {
    './src/gql/': {
      preset: 'client',
      presetConfig: {
        persistedDocuments: {
          hashFn,
        }
      }
    }
  }
}

I realize that it would not be a trivial change, but conceptually a pipeline architecture would both be more performant and easier to extend via plugins rather than having each plugin recompute/regenerate everything themselves.

Soon the client preset will no longer use the typescript and typescript-operations underneath. As we are planning to re-architecture the code generation.

[maclockard]

Reading what you are writing I think you are talking about Automatic Persisted Queries (APQ), which are not the same as persisted documents and are not related to this codegen option.

I'm a bit confused then considering your docs refers to them as the same thing: https://the-guild.dev/graphql/codegen/plugins/presets/preset-client#persisted-documents

Persisted documents (often also referred to as persisted queries or persisted documents) is a technique for reducing client to server upstream traffic by sending a unique identifier instead of the full GraphQL document. It is also commonly used to reduce the size of the client bundle as well as to improve security by preventing the client from sending and executing arbitrary GraphQL operations.

The above docs even link to these docs: https://the-guild.dev/graphql/yoga-server/docs/features/persisted-operations

Persisted operations is a mechanism for preventing the execution of arbitrary GraphQL operation documents. By default, the persisted operations plugin follows the the APQ Specification for SENDING hashes to the server.

Which links to docs on APQ.

[maclockard]

I don't get why Apollo Server needs to verify the hash and the document string, there is no security issue as the server owner needs to provide the persisted operation hash map?

Its not a security issue, its a correctness issue and prevents a common footgun. Lets say my hashing is done on this query:

query GetMyThing {
  getThing {
    id
    name
    __typename
  }
}

but the query I send to server is actually:

query GetMyThing {
  getThing {
    id
    name
  }
}

Now the server has an incorrect value in its cache and needs to be cleared before the query hashes work correctly. This is even more problematic if the server is a 3rd party server.

[n1ru4l]

You will never send an operation document to the server when using persisted documents. You will always ONLY send the hash.

In Yoga we clearly dofferenciate between these two implementations:

• https://the-guild.dev/graphql/yoga-server/docs/features/persisted-operations
• https://the-guild.dev/graphql/yoga-server/docs/features/automatic-persisted-queries

[n1ru4l]

If you want to use APQ the persisted operations feature in the client preset is completly irrelevant for you.