[
https://issues.apache.org/jira/browse/WW-5665?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Lukasz Lenart updated WW-5665:
------------------------------
Fix Version/s: 7.5.0
(was: 7.4.0)
> 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
> Priority: Major
> Fix For: 7.5.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)
