Skip to content

New option to disable GraphQL schema token limit validation#1384

Merged
kobylynskyi merged 5 commits into
kobylynskyi:mainfrom
yholkamp:configurable-token-limit
Nov 10, 2023
Merged

New option to disable GraphQL schema token limit validation#1384
kobylynskyi merged 5 commits into
kobylynskyi:mainfrom
yholkamp:configurable-token-limit

Conversation

@yholkamp

@yholkamp yholkamp commented Nov 8, 2023

Copy link
Copy Markdown
Contributor

Description

Related to #1265: This PR adds a way to configure the maximum number of GraphQL grammar tokens the parser will process before throwing a 'denial of service' prevention warning. While Gradle users can easily override this, with Maven this does not appear to be feasible.


Changes were made to:

  • Codegen library - Java
  • Codegen library - Kotlin
  • Codegen library - Scala
  • Maven plugin
  • Gradle plugin
  • SBT plugin

… may be exceeded.

Since these token limits are included as a precaution, a single setting may suffice.
@kobylynskyi

Copy link
Copy Markdown
Owner

@yholkamp thanks for creating a PR. Can you please add the same option to Gradle and SBT plugins? Thank you!

@yholkamp yholkamp force-pushed the configurable-token-limit branch 3 times, most recently from a9e0e32 to 85737bd Compare November 10, 2023 11:27
…ion to SBT, Maven and Gradle plugins.

This removes the previous tokenLimit as there will be a number of separate settings to configure.
@yholkamp yholkamp force-pushed the configurable-token-limit branch from 85737bd to 7ecffa2 Compare November 10, 2023 11:29
@yholkamp

Copy link
Copy Markdown
Contributor Author

Apologies for the multiple pushes, the linter didn't quite agree with me.

@kobylynskyi thanks for your reply, after closer inspection of the GraphQL library, my initial approach seems to be too much of a shortcut. There are 4 distinct limits that can be configured on the parser and these require different values to be useful. The library imposes these to avoid scenarios where its used to process GraphQL queries on a server, which is very different from the purpose here.

In order to make the plugin easy to use for the typical user, I've adjusted my changes to add a setting that skips this validation, while allowing it to be enabled when that's considered to be relevant. I'm hoping you'll agree with that approach.

@kobylynskyi kobylynskyi left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yholkamp thanks for the changes! Agree with your approach to disable the validation completely as too many users of this plugin really want this feature to be disabled.

@kobylynskyi kobylynskyi added this to the 5.9.0 milestone Nov 10, 2023
@kobylynskyi kobylynskyi changed the title Adding Maven configuration setting to change the GraphQL grammar token limit New option to disable GraphQL schema token limit validation Nov 10, 2023
@kobylynskyi

Copy link
Copy Markdown
Owner

@yholkamp thanks for working on this! Will be included in 5.9.0 release

@kobylynskyi kobylynskyi merged commit 1f99631 into kobylynskyi:main Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants