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.

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

Add cache: "bounded" configuration option #6536

Merged
merged 6 commits into from
Jun 8, 2022

Conversation

trevor-scheer
Copy link
Member

@trevor-scheer trevor-scheer commented Jun 7, 2022

Right now, when the APQ cache isn't configured, the APQ feature uses Apollo Server's cache which is unbounded by default.

  • We intend to not break this existing quirk in AS3, but instead provide a simple and sane configuration option for users to opt into (similar to how we did with csrfPrevention).
  • Document cache: "bounded" as typical usage in all docs examples
  • We've already fixed this in AS4. The migration path from cache: "bounded" in AS4 will be to simply delete that configuration since it will be the default.

@trevor-scheer trevor-scheer requested a review from glasser June 7, 2022 22:34
@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 7, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 5c299de:

Sandbox Source
Apollo Server Typescript Configuration
Apollo Server Configuration

@trevor-scheer trevor-scheer force-pushed the trevor/bounded-cache branch from 0e5ec97 to e67c2ae Compare June 7, 2022 22:37
@trevor-scheer trevor-scheer marked this pull request as ready for review June 7, 2022 22:58
Copy link
Member

@glasser glasser left a comment

Choose a reason for hiding this comment

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

looks good otherwise

</td>
<td>

It is recommended that you set this option to `"bounded"` if you otherwise would not have it configured. By default, the cache is unbounded. The APQ feature will use this cache by default (if you don't configure it otherwise). The default bounded cache is an [`InMemoryLRUCache`](https://www.npmjs.com/package/@apollo/utils.keyvaluecache) with a default size of roughly 30MiB. This can be configured by passing in your own cache instance.
Copy link
Member

Choose a reason for hiding this comment

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

I think I would start by saying what this does. This is the way you configure a cache backend that can be used by many features: APQs, full response cache, Data Sources, etc. Notably, it is made available to all plugins.

Then after that say that the default is an unbounded cache; this default is not recommended because clients can cause servers to crash by filling memory with APQs. If you don't want to choose a specific implementation, we recommend "bounded".

(And link to the new cache backend page but that's another PR right? So this can be left til that PR as long as it happens...)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good feedback, that's much better

@trevor-scheer trevor-scheer merged commit 9039418 into release-3.9.0 Jun 8, 2022
@trevor-scheer trevor-scheer deleted the trevor/bounded-cache branch June 8, 2022 00:54
trevor-scheer added a commit that referenced this pull request Jun 8, 2022
Issue a warning in production mode if neither the cache nor the APQ cache
(persistedQueries.cache) are configured.

We've provided a simple path to using a bounded cache via: #6536

The current default for AS3 is an unbounded in memory cache, which is
susceptible to a DOS attack since APQs can fill up the server's memory with no
limit. This warning provides an actionable recommendation to update their
configuration in order to prevent this.
@trevor-scheer trevor-scheer mentioned this pull request Jun 13, 2022
1 task
trevor-scheer added a commit that referenced this pull request Jun 15, 2022
This commit introduces the `cache: "bounded"` option. AS3 has an unbounded cache
by default, which means that a malicious client can take an open-ended amount of
memory in the cache, crashing the server.

Rather than breaking what has been the status quo since the beginning of the
project, we've chosen to add an opt-in option in order to use a bounded cache
with very little configuration. Similar to `csrfPrevention`, we've updated all
examples in our docs to use this `bounded` option.

This option will go away in AS4 when a bounded cache becomes the default.
trevor-scheer added a commit that referenced this pull request Jun 15, 2022
Issue a warning in production mode if neither the cache nor the APQ cache
(persistedQueries.cache) are configured.

We've provided a simple path to using a bounded cache via: #6536

The current default for AS3 is an unbounded in memory cache, which is
susceptible to a DOS attack since APQs can fill up the server's memory with no
limit. This warning provides an actionable recommendation to update their
configuration in order to prevent this.
@ttmotly
Copy link

ttmotly commented Aug 23, 2022

@trevor-scheer if this issue only affects apollo-server-core v3.x? or is the v1.x v2.x version also affected?

@glasser
Copy link
Member

glasser commented Aug 23, 2022

@ttmotly The issue exists in Apollo Server 2 as well. If you are still using Apollo Server 2, you can make your own InMemoryLRUCache with a chosen size ('bounded' is just a shorthand) or disable persisted queries.

None of the relevant features existed in Apollo Server 1.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants