Lukasz Lenart created WW-5665:
---------------------------------
Summary: Interceptor param validation errors point at the
interceptor definition instead of the offending interceptor-ref
Key: WW-5665
URL: https://issues.apache.org/jira/browse/WW-5665
Project: Struts 2
Issue Type: Improvement
Components: Core Interceptors
Reporter: Lukasz Lenart
Fix For: 7.4.0
h3. Problem
WW-5659 added configuration-time validation of {{WithLazyParams}} interceptor
parameters: a
param declared on an {{<interceptor-ref>}} that is not writable on the
interceptor's params
holder now fails startup with a {{ConfigurationException}} instead of being
ignored.
The check runs in {{DefaultInterceptorFactory.buildInterceptor}}, which
receives:
{code:java}
Interceptor buildInterceptor(InterceptorConfig interceptorConfig, Map<String,
String> interceptorRefParams)
{code}
{{InterceptorConfig extends Located}} and carries the location of the
{{<interceptor>}}
*definition*. The ref params arrive as a bare {{Map<String, String>}} with no
location at all.
So the exception can only be attributed to the definition, while the mistake is
in the ref.
h3. Why this is worse than it sounds
Interceptor definitions usually live in framework or plugin configuration, not
in the
application's. {{actionFileUpload}} is declared at {{struts-default.xml:60}},
inside the
{{struts2-core}} jar.
So an application with a typo in its own {{struts.xml}}:
{code:xml}
<interceptor-ref name="actionFileUpload">
<param name="allowedTypez">image/png</param>
</interceptor-ref>
{code}
fails to start with an error pointing at a line inside a Struts jar. The
message names the
interceptor, the param and the holder type, so it is actionable - but the file
and line send the
reader to framework configuration they did not write and cannot change, for a
mistake sitting in
their own file.
Startup failure is the intended behaviour (see WW-5659); the diagnostics are
what need work.
h3. Cause
The {{InterceptorFactory}} SPI has no parameter for the ref's {{Location}}.
{{InterceptorBuilder}} holds it - {{constructInterceptorReference}} takes a
{{Location}} and uses
it for its own errors - but does not pass it down to the factory.
h3. Options
# *Extend the SPI.* Add an overload of {{buildInterceptor}} taking the ref's
{{Location}},
defaulting to the existing method so third-party {{InterceptorFactory}}
implementations keep
compiling. Most direct; the location reaches the exception where it is thrown.
# *Validate in {{InterceptorBuilder}} instead.* It already has both the ref
params and their
{{Location}}. Moves the check away from the holder-type knowledge that lives
in the factory,
so it would need the holder instance passed back or the check split.
# *Improve the message without moving the location.* Name the referencing
package and action in
the text so the reader can find the ref themselves. Cheapest, and leaves a
misleading
file:line in place - a partial fix at best.
Option 1 looks cleanest. Worth checking whether other configuration errors
raised from factories
have the same weakness before settling on a shape, so this is fixed as a
pattern rather than one
call site.
h3. Suggested work
* thread the {{<interceptor-ref>}} location through to the factory
* assert in a test that the reported location is the ref's, not the definition's
* review other {{ConfigurationException}}s raised from
{{DefaultInterceptorFactory}} for the
same attribution problem
h3. References
*
{{core/src/main/java/org/apache/struts2/factory/DefaultInterceptorFactory.java}}
-
{{buildInterceptor}} and {{validateLazyParamNames}}
* {{core/src/main/java/org/apache/struts2/factory/InterceptorFactory.java}} -
the SPI without a
ref {{Location}}
*
{{core/src/main/java/org/apache/struts2/config/providers/InterceptorBuilder.java}}
-
{{constructInterceptorReference}}, which has the {{Location}}
* {{core/src/main/resources/struts-default.xml:60}} - where
{{actionFileUpload}} is defined
* WW-5659 - introduced the validation
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
