Navigation Menu

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

Expose ServletHolder getter in ServletHandler$ChainEnd for auditing libraries to use #6487

Closed
joakime opened this issue Jun 30, 2021 · 5 comments · Fixed by #6488 or #6511
Closed

Expose ServletHolder getter in ServletHandler$ChainEnd for auditing libraries to use #6487

joakime opened this issue Jun 30, 2021 · 5 comments · Fixed by #6488 or #6511
Assignees
Labels
Enhancement Sponsored This issue affects a user with a commercial support agreement

Comments

@joakime
Copy link
Contributor

joakime commented Jun 30, 2021

Jetty version(s)
9.4.42

Java version/vendor (use: java -version)
All

OS type/version
All

Description
If an audit library wants to hook into the FilterChain of the ServletHandler, they use the new APIs such as ServletHandler.newFilterChain(FilterHolder filterHolder, FilterChain chain) introduced in Issue #5022

However accessing the ServletHolder isn't that easy, and you have to resort to reflection trickery to access it.

We should expose the ServletHolder as a formal API.

@joakime joakime self-assigned this Jun 30, 2021
@joakime joakime added the Sponsored This issue affects a user with a commercial support agreement label Jun 30, 2021
@joakime joakime added this to To do in Jetty 9.4.44 FROZEN via automation Jun 30, 2021
joakime added a commit that referenced this issue Jun 30, 2021
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Jul 8, 2021
…d-servletholder

Issue #6487 - expose ServletHolder in ChainEnd
joakime added a commit that referenced this issue Jul 8, 2021
Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
joakime added a commit that referenced this issue Jul 12, 2021
…nd-servletholder

Issue #6487 - expose ServletHolder in ChainEnd (Jetty 10)
@gregw
Copy link
Contributor

gregw commented Sep 20, 2021

@joakime I'm closing this as both PRs are merged.

@gregw gregw closed this as completed Sep 20, 2021
Jetty 9.4.44 FROZEN automation moved this from To do to Done Sep 20, 2021
@joakime
Copy link
Contributor Author

joakime commented Feb 1, 2022

@gregw any reason why Chain and ChainEnd are still private?

@gregw
Copy link
Contributor

gregw commented Feb 2, 2022

@joakime well the question really should be put the other way. What is the reason for not having them private?
I guess they could be made protected so they are available to extensions, but they probably need better names if they are to be exposed... maybe: FilterChainLink and FilterChainTarget

@joakime
Copy link
Contributor Author

joakime commented Feb 2, 2022

@gregw to allow people to implement the newFilterChain() properly, and actually be able to call this .getServletHolder() that the associated PRs were all about.

@gregw
Copy link
Contributor

gregw commented Feb 2, 2022

@joakime developers that implement newFilterChain() can do so without use of our utility classes for creating links in the chain and a termination. However, I have no issue with making our utility classes available as protected classes, so long as we think about the names a bit more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Sponsored This issue affects a user with a commercial support agreement
Projects
No open projects
2 participants