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

[joakime]

PROOF OF CONCEPT

This is likely incomplete and certainly not tested (yet).

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

The configuration in the WEB-INF/web.xml would look like this ...

<?xml version="1.0" encoding="UTF-8"?>
<web-app 
   xmlns="http://xmlns.jcp.org/xml/ns/javaee" 
   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
   xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/javaee http://xmlns.jcp.org/xml/ns/javaee/web-app_3_1.xsd"
   metadata-complete="false"
   version="3.1">

  <context-param>
    <param-name>org.eclipse.jetty.server.Request.maxFormContentSize</param-name>
    <param-value>1000000</param-value>
  </context-param>

  <context-param>
    <param-name>org.eclipse.jetty.server.Request.maxFormKeys</param-name>
    <param-value>2000</param-value>
  </context-param>
 
  <!-- ... other config ... -->
</web-app>

[janbartel]

This change would prevent the use of System properties being used, which would be removal of long released behaviour. You could just add in support for the context attributes, or we could punt this in favour of a different solution for jetty-web.xml.

[joakime]

The support for these two System properties was just moved to the doStart() location, not removed.

[janbartel]

I see it now in doStart(). However, I don't think we should be doing this development in jetty-10, if we're doing it, it should be in jetty-12.

[joakime]

This was a possible solution for what was brought up in Issue #11274, which meant it should be done in Jetty 10 thru Jetty 12.

But the OP (@vmassol) made some good points (see #11266 (comment)) and has convinced me that the approach in this PR isn't a great solution.

I'm totally OK with this being only for Jetty 12.
Want me to recreate this for Jetty 12 only? or drop it as a concept?

[janbartel]

IMHO we should do it for 12, and also update the doco for it with the new context attributes.

[joakime]

Will do, and I'll add unit tests!

[janbartel]

I see it now in doStart(). However, I don't think we should be doing this development in jetty-10, if we're doing it, it should be in jetty-12.

[joakime]

Closing, will be a feature introduced in Jetty 12 instead.

[vmassol]

@joakime Hello. Thanks again for working on this. Do you know if the feature has now been integrated in Jetty 12 or not yet? I checked the release notes but couldn't see it (but I could have missed it). For the context, we're still stuck with #11266 and any solution would work for us, even context attributes :) thx!

[vmassol]

@joakime gentle ping to know if this feature has been added in Jetty 12, see #11274 (comment) Thx!

[joakime]

@joakime gentle ping to know if this feature has been added in Jetty 12, see #11274 (comment) Thx!

See replacement PR #11898