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.

Sign up for GitHub

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

Jump to bottom

Review GzipHandler inside ServletContextHandler #4765

Closed
sbordet opened this issue Apr 10, 2020 · 1 comment · Fixed by #4767
Closed

Review GzipHandler inside ServletContextHandler #4765

sbordet opened this issue Apr 10, 2020 · 1 comment · Fixed by #4767

Comments

@sbordet
Copy link
Contributor

sbordet commented Apr 10, 2020

Jetty version
10.0.x

Description
ServletContextHandler has direct support for GzipHandler, apparently for the only reason to organize the internal Handlers in this way: session -> security -> gzip -> servlet.

However, there is no apparent reason to have GzipHandler part of ServletContextHandler.

A configuration like this: GzipHandler -> ServletContextHandler seems to work perfectly fine, and it's more in line with how you would use GzipHandler with others ContextHandlers.

@gregw @joakime what am I missing?

Can GzipHandler be removed from ServletContextHandler?
sbordet added a commit that referenced this issue Apr 13, 2020
@sbordet
Issue #4765 - Review GzipHandler inside ServletContextHandler.
1a234dc 
Removed GzipHandler from ServletContextHandler.
Updated ServletContextHandlerTest.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@sbordet sbordet mentioned this issue Apr 13, 2020
Merged
sbordet added a commit that referenced this issue Apr 13, 2020
@sbordet
Issue #4765 - Review GzipHandler inside ServletContextHandler.
e17d0f4 
Fixed test failures.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
@gregw
Copy link
Contributor

gregw commented Apr 14, 2020

I believe this is a hangover from before we had the interceptor mechanism.

sbordet added a commit that referenced this issue Apr 14, 2020
@sbordet
Issue #4765 - Review GzipHandler inside ServletContextHandler.
013acca 
Fixed XML files.
Restored ServletContextHandler.setGzipHandler() and
deprecated it in case it's used from user's XML files.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet added a commit that referenced this issue Apr 14, 2020
@sbordet
Issue #4765 - Review GzipHandler inside ServletContextHandler.
509f579 
Restored IncludedGzipTest.
Fixed ServletContextHandler.relinkHandlers(), now excluding
the checks for GzipHandler.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
sbordet added a commit that referenced this issue Apr 14, 2020
@sbordet
Merge pull request #4767 from eclipse/jetty-10.0.x-4765-gziphandler_s…
5bad85b 
…ervletcontexthandler

Issue #4765 - Review GzipHandler inside ServletContextHandler.
viv added a commit to viv/Openfire that referenced this issue May 16, 2023
@viv
WIP: Upgrades Jetty 9.4.43.v20210629 to 10.0.15
2c0f741 
Remaining Issues:
- JSP compilation failure caused by:

apache/tomcat@224fb6c#diff-ef02fd9c3f90ca4838d3cdaa051489556eca75d537fc396022e0ed456c24d895R329

- `WebSocketClientConnectionHandler.onConnect` assumes that `session.getRemoteAddress()` supplies an `InetSocketAddress` which might not always be true. What's the best approach in our context?

Fixed in this commit:
- Websocket server Maven artifact renamed from `websocket-server` to `websocket-jetty-server`
- `WebSocketServlet` renamed to `JettyWebSocketServlet`
- `WebSocketServletFactory` renamed to `JettyWebSocketServletFactory`

See: https://www.eclipse.org/jetty/documentation/jetty-10/programming-guide/index.html#pg-migration-94-to-10

- `GzipHandler` is no longer part of `ServletContextHandler`
jetty/jetty.project#4765

- Renamed `setWebInfClassesDirs` to `setWebInfClassesResources`
jetty/jetty.project#4506

- Split `SslContextFactory` into Client and Server
jetty/jetty.project#3464
@viv viv mentioned this issue May 16, 2023
Merged
viv added a commit to viv/Openfire that referenced this issue Sep 4, 2023
@viv
WIP: Upgrades Jetty 9.4.43.v20210629 to 10.0.15
5e9befe 
Remaining Issues:
- JSP compilation failure caused by:

apache/tomcat@224fb6c#diff-ef02fd9c3f90ca4838d3cdaa051489556eca75d537fc396022e0ed456c24d895R329

- `WebSocketClientConnectionHandler.onConnect` assumes that `session.getRemoteAddress()` supplies an `InetSocketAddress` which might not always be true. What's the best approach in our context?

Fixed in this commit:
- Websocket server Maven artifact renamed from `websocket-server` to `websocket-jetty-server`
- `WebSocketServlet` renamed to `JettyWebSocketServlet`
- `WebSocketServletFactory` renamed to `JettyWebSocketServletFactory`

See: https://www.eclipse.org/jetty/documentation/jetty-10/programming-guide/index.html#pg-migration-94-to-10

- `GzipHandler` is no longer part of `ServletContextHandler`
jetty/jetty.project#4765

- Renamed `setWebInfClassesDirs` to `setWebInfClassesResources`
jetty/jetty.project#4506

- Split `SslContextFactory` into Client and Server
jetty/jetty.project#3464
guusdk pushed a commit to igniterealtime/Openfire that referenced this issue Sep 7, 2023
@viv @guusdk
WIP: Upgrades Jetty 9.4.43.v20210629 to 10.0.15
09c12d2 
Remaining Issues:
- JSP compilation failure caused by:

apache/tomcat@224fb6c#diff-ef02fd9c3f90ca4838d3cdaa051489556eca75d537fc396022e0ed456c24d895R329

- `WebSocketClientConnectionHandler.onConnect` assumes that `session.getRemoteAddress()` supplies an `InetSocketAddress` which might not always be true. What's the best approach in our context?

Fixed in this commit:
- Websocket server Maven artifact renamed from `websocket-server` to `websocket-jetty-server`
- `WebSocketServlet` renamed to `JettyWebSocketServlet`
- `WebSocketServletFactory` renamed to `JettyWebSocketServletFactory`

See: https://www.eclipse.org/jetty/documentation/jetty-10/programming-guide/index.html#pg-migration-94-to-10

- `GzipHandler` is no longer part of `ServletContextHandler`
jetty/jetty.project#4765

- Renamed `setWebInfClassesDirs` to `setWebInfClassesResources`
jetty/jetty.project#4506

- Split `SslContextFactory` into Client and Server
jetty/jetty.project#3464
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Issue #4765 - Review GzipHandler inside ServletContextHandler. jetty/jetty.project
2 participants
@gregw @sbordet