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

Issue #11266 - allow context attributes to be used to configure max form (content size / keys) #11898

Closed

Conversation

joakime
Copy link
Contributor

@joakime joakime commented Jun 10, 2024

Replacement of PR #11274

  • Allows these common configurations to be set via WEB-INF/web.xml without the need for a WEB-INF/jetty-web.xml

…orm (content size / keys)

* Allows these common configurations to be set via WEB-INF/web.xml without the need for a WEB-INF/jetty-web.xml
@joakime joakime requested a review from janbartel June 10, 2024 12:52
@joakime joakime self-assigned this Jun 10, 2024
@joakime
Copy link
Contributor Author

joakime commented Jun 10, 2024

@janbartel is this approach acceptable?

This PR is currently draft, as I still need to write some documentation and new unit tests for these new techniques.

Copy link
Contributor

@janbartel janbartel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we need to do this now that we can accommodate all different environment versions of WEB-INF/jetty-web.xml and even a general way to apply an environment specific context xml file to every deployment to that environment.

@joakime
Copy link
Contributor Author

joakime commented Jun 13, 2024

I'm not sure we need to do this now that we can accommodate all different environment versions of WEB-INF/jetty-web.xml and even a general way to apply an environment specific context xml file to every deployment to that environment.

I have no idea what you are referencing.

The documentation, and the code do not show alternate jetty-web.xml behaviors on the jetty-12.0.x branch.

@vmassol
Copy link

vmassol commented Jun 13, 2024

I'm not sure we need to do this now that we can accommodate all different environment versions of WEB-INF/jetty-web.xml and even a general way to apply an environment specific context xml file to every deployment to that environment.

This was discussed at #11266 (comment) but I'm not sure it's been implemented already. If it has then that's awesome as I could use it on XWiki (we're still stuck ATM).

Thx

@janbartel
Copy link
Contributor

@joakime @vmassol it's been implemented in the jetty-12.1.x branch. I guess if there's interest it could be backported to jetty-12.0.x, but we will be switching to jetty-12.1.x in the nearish future anyway (because servlet 6.1).

@janbartel
Copy link
Contributor

I forgot we already had an issue for backporting the new environment specific jetty-web.xml and jetty-env.xml files: #11774

@vmassol
Copy link

vmassol commented Jun 27, 2024

@janbartel cool, thx. I hope that Jetty 12.1 will be released soon or that a 12.0.x release with #11774 will be done soon :) Right now we're stuck on Jetty 10 (as XWiki cannot move to jakarta EE yet), which is why we're keen to move to Jetty 12.x. Thx again

@olamy
Copy link
Member

olamy commented Jun 27, 2024

@vmassol 12.0.11 should be around soon (we hope mid/late next week)

@vmassol
Copy link

vmassol commented Jun 27, 2024

Great, thanks Olivier! :) (I hope it'll contain #11774 which is still open ATM)

@vmassol
Copy link

vmassol commented Jun 27, 2024

@janbartel @joakime @olamy Actually I'm not sure if this issue will help us since our problem is the ability to deploy the same XWiki WAR to work both in Jetty 10 and 12 (see #11266).

AFAIU the code from this issue just adds a new way to configure jetty through web.xml and thus this way is not present in Jetty 10.x, right?

OTOH implementing #11266 (comment) would work since it's backward-compatible (Jetty 10 would use jetty-web.xml and Jetty 12 would use jetty-web-xxx.xml).

Or do you plan to backport the ability to configure Jetty from web.xml also in Jetty 10?

Thx

@janbartel
Copy link
Contributor

@vmassol no, we're not backporting any changes like this to jetty-10, so you're correct that the way to go in jetty-12.0 and beyond is to use the features in #11966 to make an ee specific jetty-web-eeX.xml file (and just continue to use jetty-web.xml as-is in prior releases).

@janbartel janbartel closed this Jul 9, 2024
@janbartel janbartel deleted the fix/12.0.x/maxform-configs-as-context-attributes branch July 9, 2024 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants