ср, 22 мар. 2023 г. в 14:38, Mark Thomas <ma...@apache.org>:
>
> Any more thoughts on this?
>

1. If we cannot agree on the required behaviour, it is one more reason
to make it configurable.

As I said, it would be more useful to configure it at a Context.

2. Regarding the default behaviour,

Throwing an exception was also my first thought,
and it seems more natural.

Regarding implementation, I thought that in
org.apache.catalina.connector.Request all lines

        if (!parametersParsed) {
            parseParameters();
        }
could be amended with "if (parseFailed) throw new
IllegalStateException(parseFailedReason)", or maybe put this "throw"
into parseParameters().

BTW, Spring Framework has a feature that routing of requests can be
configured with annotations.
https://docs.spring.io/spring-framework/docs/5.3.25/reference/html/web.html#mvc-ann-requestmapping-params-and-headers

In this case parameters parsing is hidden from the caller (done by
framework), and also a Request may be omitted from method signature
(so one wouldn't check its attributes to check for failed parsing). In
this case it makes sense to throw an exception to report a failure.

3. Regarding UserDataHelper,

1) If we rely on it, it means being too late.

At the time one considers reading the logs, data loss has already happened.

2) If you look at my mail regarding code paths (7th email in this thread),
if I have read the code correctly, I think that in case of

"d) Request.getParameter() was called, and request was  "multipart/form-data"."

there is no logging.

4. Regarding the value for maxParameterCount

500 parameters may mean 100 rows of 5 values each;
100 rows may mean daily values for 3 months.

1000 parameters may mean a year of daily data with 3 values each day.
It is not what one would frequently see in practice, but it could happen.


> There hasn't been much movement from the spec EG on this, so my current
> thinking is to revert this change for 10.1.x and earlier to wait and see
> what the Servlet EG decides.
>

5. If someone is thinking about improved API,

a) I wonder whether ExtendedAccessLogValve calling of getParameter()
could be improved,
so that it does not trigger parameter parsing that includes reading
the body of a POST.

Or maybe do reading, but with a lower timeout. Essentially in the same
way as when skipping a body of a failed request.

It is not a job for an AccessLogValve to spend time on parameter parsing.

b) I wonder whether parameter parsing could be done asynchronously.


Best regards,
Konstantin Kolinko

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

Reply via email to