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

Breaking change in class DosFilter from 9.4.53 to 9.4.54 #11618

Closed
trnguyencflt opened this issue Apr 2, 2024 · 8 comments
Closed

Breaking change in class DosFilter from 9.4.53 to 9.4.54 #11618

trnguyencflt opened this issue Apr 2, 2024 · 8 comments
Labels

Comments

@trnguyencflt
Copy link

Jetty Version
9.4.54

Jetty Environment
N/A

Java Version
8+

Question
The commit that made into 9.4.54 contains a breaking change to DosFilter class because this function extractUserId(ServletRequest request) is now completely ignored in the class. This is breaking the behaviour of any subclass of this class that implements extractUserId. In addition, marking it as @Deprecated IMO isn't right because deprecated means it is still working while offering projects that depend on this to work on new alternative.

Is there a way to make it work, i.e keeping the behaviour of extractUserId while achieving the goal of the commit?

Many thanks,
Truc

@msn-tldr
Copy link

msn-tldr commented Apr 2, 2024

+1 as this broke our dependency on DosFilter that used extractUserId.

  • What is the way forward?
  • How can such breaking changes be avoided in future?

@joakime
Copy link
Contributor

joakime commented Apr 2, 2024

Jetty 9.x is now at End of Community Support.

The change in DosFilter was done as Session (and auth/user id) tracking is no longer supported.
The extractUserId in turn is no longer called.

The way forward is to not use an unsupported version of Jetty, you should be using Jetty 12 at this point in time (even if you are using javax.servlet.* classes, you can use the ee8 environment on Jetty 12)

@joakime
Copy link
Contributor

joakime commented Apr 2, 2024

Is there a way to make it work, i.e keeping the behaviour of extractUserId while achieving the goal of the commit?

No, as user/auth based tracking breaks the fundamental contract of Rate Tracking that is used at the server level (to accomplish DosFilter based filtering on userId, it needed to remove arbitrary server level Rate Tracking to do it. Which breaks server level Rate Tracking).
There is also the fact that session, auth, and user_id based entries are also exposed across different requests breaking the Servlet request contract.
And the session and auth entries accumulate improperly and are not cleaned up in a timely manner.

Note that this was removed from all versions of Jetty, from 9.4.x thru to 12.0.x

@msn-tldr
Copy link

msn-tldr commented Apr 2, 2024

@joakime thanks for looking into this!

The way forward is to not use an unsupported version of Jetty, you should be using Jetty 12 at this point in time (even if you are using javax.servlet.* classes, you can use the ee8 environment on Jetty 12)

Does jetty 12 support any way of user-id based rate-limiting? Or jetty users have to roll their own implementation of this.

@sbordet
Copy link
Contributor

sbordet commented Apr 2, 2024

Does jetty 12 support any way of user-id based rate-limiting? Or jetty users have to roll their own implementation of this.

Jetty 12 will not support user-id based rate limiting, because it is very often too application specific, and may put the server under pressure.

I recommend that you carefully review your current usage, and make sure that any user-id you are using is a valid one (for example, after authentication).

@trnguyencflt
Copy link
Author

thanks all for the explanation 👍

@msn-tldr
Copy link

msn-tldr commented Apr 3, 2024

Thanks @joakime & @sbordet.

@janbartel
Copy link
Contributor

Closing this issue as it appears to have been answered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants