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
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
