Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Cleaning up request attributes after websocket upgrade in Jetty 10 #5868

Closed
SerCeMan opened this issue Jan 9, 2021 · 4 comments · Fixed by #5935
Closed

Cleaning up request attributes after websocket upgrade in Jetty 10 #5868

SerCeMan opened this issue Jan 9, 2021 · 4 comments · Fixed by #5935
Assignees

Comments

@SerCeMan
Copy link
Contributor

SerCeMan commented Jan 9, 2021

Jetty version
10.0.0

Description

Hi, Jetty maintainers!

I'm currently in the progress of migrating from 9.4.x to 10.0.0, and I stumbled across a difference in the behaviour of the JettyWebSocketCreator. In Jetty 9.4.x, the upgrade request allows for removing servlet attributes:
https://github.com/eclipse/jetty.project/blob/403d5ec318ffaa20f1f2c0b62df217cfc99ebbe0/jetty-websocket/websocket-servlet/src/main/java/org/eclipse/jetty/websocket/servlet/UpgradeHttpServletRequest.java#L407-L414

However, 10.0.0 throws an exception when an attempt to modify the request attributes is made:
https://github.com/eclipse/jetty.project/blob/7dfb44f7ad7c70d5dda509587629b825bacafce4/jetty-websocket/websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/UpgradeHttpServletRequest.java#L406-L410

The issue with not being able to remove the attributes is that the WebSocket applications often maintain a large number of connections and additional attributes on the request lead to a significant increase in memory usage. In 9.4.x, it would be possible to read the attributes within JettyWebSocketCreator, act upon them, and then establish a socket and completely remove the attributes freeing up a significant amount of memory. Is there a way to achieve similar behaviour in Jetty 10 where attributes are removed after establishing the WebSocket connection?

@SerCeMan
Copy link
Contributor Author

Hey, @lachlan-roberts! I'm just wondering if you could suggest a workaround for this issue. I'm thinking of adding another filter that re-packs all attributes into a holder attribute which can later nullify all of the references, however, the workaround feels incredibly hackish.

@gregw
Copy link
Contributor

gregw commented Feb 1, 2021

@SerCeMan that approach would work, but is a bit hackish. Best is that we allow you to remove those attributes. I've flagged this to be fixed in the next release.

@SerCeMan
Copy link
Contributor Author

SerCeMan commented Feb 1, 2021

Thanks, @gregw! Let me know if you'd prefer me to submit a pull request, I could try to give it a go.

lachlan-roberts added a commit that referenced this issue Feb 2, 2021
…dshake

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@lachlan-roberts
Copy link
Contributor

@SerCeMan I have opened PR #5935 to fix this, feel free to add your review. With the PR you should now be able to set request attributes in the JettyWebSocketCreator.

lachlan-roberts added a commit that referenced this issue Feb 8, 2021
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit that referenced this issue Feb 9, 2021
…estResponse

Issue #5868 - allow request attributes to be set in websocket upgrade
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants