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

jetty-http-spi: Filters in HttpContext/JettyHttpContext ignored, implementation missing #9125

Open
demonti opened this issue Jan 5, 2023 · 10 comments · May be fixed by #12764
Open

jetty-http-spi: Filters in HttpContext/JettyHttpContext ignored, implementation missing #9125

demonti opened this issue Jan 5, 2023 · 10 comments · May be fixed by #12764
Labels
Bug For general bugs on Jetty side Help Wanted Low Priority

Comments

@demonti
Copy link

demonti commented Jan 5, 2023

Jetty version(s)
Jetty 9.x is now at End of Community Support
I discovered this in Jetty 9.4, but the problem obviously exists also in 10.x, 11.x

Java version/vendor (use: java -version)
java version "1.8.0_271"
Java(TM) SE Runtime Environment (build 1.8.0_271-b09)
Java HotSpot(TM) 64-Bit Server VM (build 25.271-b09, mixed mode)

OS type/version
Linux mclane 5.19.0-26-generic #27-Ubuntu SMP PREEMPT_DYNAMIC Wed Nov 23 20:44:15 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Ubuntu 22.10

Description
I ported code that used Java's built-in HTTP-Server (com.sun.net.httpserver) to Jetty's adapter. The code uses HttpContext.getFilters() to add two filters, one dealing with authentication, another with input decompression (we are using HTTP POST with compressed data).

I discovered that both filters do not work. The code is not executed. I downloaded the code for 9.4.x and 10.x from Github and saw that the JettyHttpContext, which extends Java's HttpContext, only stores the filters, but does not process the filters on its own. It is merely a container holding all the data. The only access to the filters is via the getFilters() method. However, nowhere in the Jetty code this method is ever called. Filters are obviously not implemented. I guess this is not intentional. If it actually is, I would highly appreciate a hint on how filtering should be implemented instead.

How to reproduce?

Check the call hierarchy of the JettyHttpContext.getFilters() method and see that it is not called. Thus, any filters added by user code will be ignored.

@demonti demonti added the Bug For general bugs on Jetty side label Jan 5, 2023
@joakime
Copy link
Contributor

joakime commented Jan 5, 2023

Congrats, you are officially the first person (since 2011) to use Java's built-in HTTP-Server's Filter functionality with Jetty! (yes! really!)

Original code addition for jetty-http-spi to Jetty 9.0.0 ...

Current codebase for jetty-10.0.x ...

https://github.com/eclipse/jetty.project/blob/jetty-10.0.x/jetty-http-spi/src/main/java/org/eclipse/jetty/http/spi/HttpSpiContextHandler.java#L71

https://github.com/eclipse/jetty.project/blob/4ca148b2d250ac476b9395b4d679d59a240fb899/jetty-http-spi/src/main/java/org/eclipse/jetty/http/spi/HttpSpiContextHandler.java#L71

This was never implemented.

This would be a very low priority for us.

Your options:

  • Implement it yourself and submit a PR to the project
  • Don't use sun HttpServer Filter's, use Jetty Handlers
  • Don't use sun HttpServer, use Jetty properly (it's faster and more robust)

Typically, people that care about Authentication use the Servlet Layer in Jetty to accomplish this (for Jetty 12, this would be the "core" layer instead)
For Input Decompression, the Jetty GzipHandler is used (This is intentional as the changes to ServletInputStream as part of the Servlet Async I/O APIs prevent reliable filter based input decompression that manipulates the InputStream)

@demonti
Copy link
Author

demonti commented Jan 5, 2023

Wow!

This is a SOAP interface using the JAX-WS framework, which has the ability to work together with the built-in server. This is a convenient solution for reasonable traffic between internal services. I guess that there is an alternative with servlets, and with the directions you gave me, I likely will find a solution.

From my perspective, I don't mind if you close the issue as "won't fix", but on the other hand it is questionable whether to retain the submodule at all in future releases if it is only implemented halfhearted.

@joakime
Copy link
Contributor

joakime commented Jan 5, 2023

Jetty operates with many JAX-WS frameworks, none of them use Jetty via the com.sun.net.httpserver.HttpServer, but instead use Jetty directly.

@demonti
Copy link
Author

demonti commented Jan 5, 2023

I guess so. The whole effort was to solve a problem with the built-in server and was thought as a quick & dirty solution. We have used Tomcat standalone instances with web applications in the past for such purposes, so we miss some experience in respect to embedded Jetty and SOAP. However, I'm confident to solve it, thanks!

@joakime
Copy link
Contributor

joakime commented Jan 5, 2023

The reason this is Low Priority for us ...

Looking at maven download statistics for jetty-http-spi (all versions) we see that for the (whole) year 2022, it accounts for 0.00012% of all of our downloaded artifacts.
A mere 9,312 downloads over the entire year.

If we look a other artifacts for the same time period:

  • jetty-server had 177,678,050 downloads
  • jetty-util had 201,719,787 downloads

@strogiyotec
Copy link
Contributor

@demonti hello, may I ask you to give me a code example to reproduce the bug, I will work on the ticket.
Regards !

Copy link

This issue has been automatically marked as stale because it has been a
full year without activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@github-actions github-actions bot added the Stale For auto-closed stale issues and pull requests label Jun 12, 2024
Copy link

This issue has been closed due to it having no activity.

@sbordet sbordet removed the Stale For auto-closed stale issues and pull requests label Jul 14, 2024
@sbordet sbordet reopened this Jul 14, 2024
@SentryMan
Copy link
Contributor

I am also running into this. Will this get fixed?

@gregw
Copy link
Contributor

gregw commented Jan 19, 2025

I am also running into this. Will this get fixed?

It will eventually, but it is a very low priority. We have few users using the server this way and absolutely no commercial customers.

I expect that once 12.1 is released, we might have done more time to look at this. But we are very much open to community contributions to resolve this.

@SentryMan SentryMan linked a pull request Feb 4, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side Help Wanted Low Priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants