rx-angular icon indicating copy to clipboard operation
rx-angular copied to clipboard

ISR: Avoiding immense cache-keys generation (Adding the possibility of defining a custom cacheKey)

Open sihu opened this issue 1 year ago • 1 comments

Problem Solved By The Feature

This is related to https://github.com/rx-angular/rx-angular/issues/1720 but would be a bit more generic This is related to https://github.com/rx-angular/rx-angular/issues/1604 but the fix done there is not fixing the issue described here

We experienced problems using the ISR library since attackers tried to access a lot of urls. Since getCacheKey uses the req.url this will lead to a separate cache-entry for each request. Within short time it filled our cache with more than 15GB of data. We had to patch the library in order to use req.path for now.

E.g.

  • Changing parameters: /my-route?1, /my-route?something=1, , /my-route?something=3 ...
  • Calling 404 pages: /something-404 that would look exactly the same but are not

I'm aware that this might be the wanted behaviour because of pagination etc. But i want to have the control of modifying this and i am not sure if it's the best default. For simple pages req.path could be an alternative. Even with pagination one could simply iterate over pages and generate an immense amount of entries.

For the 404-Pages I found the solution to not cache it (revalidate: null). I will gladly add this as a hint to the documentation to protect other users. So i cannot cache the 404 page, but i also don't create a new entry for every 404-page called.

Solution

I would suggest a configuration that lets you specifying your cacheKey-Function. Ideally the interface would allow to pass in the req and then users could still allow certain params.

For the 404-problem i was first thinking of having a beforeAddingToCache-hook where i can decide from the given response-code if i want to cache it or not.

Alternatives Considered

I did not come up with any other ideas, but happy to discuss it here!

My current workaround:

sihu avatar Jul 03 '24 10:07 sihu

@eneajaho I just push a commit to my own fork to handle this, since my company has the same issue. (We used to use our own library to handle SSR, similar with your ISR actually)

The idea of the commited change is basically like

  1. create an array for allowed query params, so when url comes in, it will only preserve the allowed query params and store as cache key.
  2. if it is null or undefined, the behavior remain the same
  3. if it is empty array, the behavior is like using req.path

I add a bit of unit test for it to ensure the behavior works as it should be.

Let me know what you think, I am fine with different approach.

maxisam avatar Aug 22 '24 16:08 maxisam

Hi @sihu I guess the fix in #1757 should get the job done.

eneajaho avatar Aug 31 '24 17:08 eneajaho