On 15/08/2023 14:24, Christopher Schultz wrote:
Mark,

On 8/9/23 10:13, Mark Thomas wrote:
On 09/08/2023 14:45, ma...@apache.org wrote:
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/main by this push:
      new c880673cff Update Servlet API for parameter error handling changes
c880673cff is described below

commit c880673cffad6b8884c1d2464e5a736e852e7eeb
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Thu Jun 15 12:15:08 2023 +0100

     Update Servlet API for parameter error handling changes

Hi all,

I have started work on implementing these changes but wanted to gather feedback before I go much further with the implementation work.

The key question is how much control do we want to provide over the behaviour if invalid parameters are encountered?

Question 1.
The default behavior will be to throw an exception. Do we want to use a single exception type or do we want to use specialized exceptions for each type of error? It is more (boilerplate) code but specialized exceptions allow applications to identify what went wrong without having to do things like parse error messages.

While I generally like specialized exception types, I think Tomcat would have to create these exception classes itself, meaning that applications sensitive to them would be non-portable.

Should we be encouraging such non-portability?

Good point. I have taken a couple of different approaches to implement this change and so far they have turned out to be too invasive for my liking. My current attempt (the 4th) is looking better and having a single exception type would further simplify things.

I guess there are three options:

a) use IllegalStateException

b) use a single new exception type that extends IllegalStateException

c) use multiple new exception types that extend IllegalStateException

The argument in favour of b) is it allows us to suppress the generation of stack traces which given that they are i) relatively expensive and ii) triggered by user input in this case


Question 2.
Do we want to support non-default behavior which, most likely, would be to ignore the invalid parameter(s) and continue processing? Assuming we do, do we want to make that configurable for each type of error or just have a single "ignoreInvalidParameters" option?

Ignoring parameter values is almost always NOT what the application is expecting, and may cause breakage. Definitely this should NOT be the default. I'm not sure if it even makes sense to provide a configuration option to ignore such parameter values (or names).

Option 1.
My current implementation is using specialized exceptions and per error type configuration although it is still in progress. I'm also logging all invalid parameters at debug level.

Option 2.
This is creating quite a lot of plumbing. I am wondering, given that this is essentially handling for invalid requests, whether it is worth it. The alternative approach I have in mind is:
- specialized exceptions for each type of error
- log errors at debug level
- no options to allow invalid parameters
- wait and see if users request configuration options as they start to
   migrate to Tomcat 11

At this point I am leaning towards option 2 while keeping the implementation I have so far for option 1 in a branch in case we decide to add it later.

Thoughts?

Option 2 seems more like "yes and" rather than "no but" -- an extension to Option 1, really. I guess except for the configuration option(s). I like option 2 because I'm not sure all that additional configuration will be worth it.

Thanks for the feedback. I'll hopefully have some code for folks to review later this week.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to