Lukasz Lenart created WW-5664:
---------------------------------
Summary: TextParser gives callers no way to tell an unresolved
expression from one that resolved to empty
Key: WW-5664
URL: https://issues.apache.org/jira/browse/WW-5664
Project: Struts 2
Issue Type: Improvement
Components: Core
Reporter: Lukasz Lenart
Fix For: 8.0.0
h3. Problem
{{OgnlTextParser.evaluate}} substitutes empty text when a variable does not
resolve, and
returns that substitution indistinguishably from a variable that legitimately
resolved to an
empty value:
{code:java}
// OgnlTextParser.java, inside evaluate(...)
Object o = evaluator.evaluate(var);
...
if (o != null) {
middle = o.toString();
if (StringUtils.isEmpty(left)) {
result = o; // legitimately empty value -> result
is ""
}
...
} else {
// the variable doesn't exist, so don't display anything
expression = left.concat(right);
result = expression; // unresolvable -> result is also ""
}
{code}
For a bare {{${x}}} both branches yield {{""}}. The parser discards the one
piece of
information only it holds - whether resolution actually succeeded - so no
caller can recover it.
h3. Why it matters beyond one caller
{{translateVariables}} and the underlying parser are used in 18 files across
core and the
plugins, including {{ActionChainResult}}, {{StrutsResultSupport}},
{{HttpHeaderResult}},
{{ValidatorSupport}}, {{UIBean}} and {{StrutsUtil}}. In each of them a typo'd
or unresolvable
expression is silently replaced with empty text. That is long-standing and
mostly benign for
display, but it means no consumer can choose to treat a broken expression
differently from an
intentionally empty one.
WW-5659 hit this directly. {{WithLazyParams.LazyParamInjector}} resolves
{{${...}}} interceptor
params into a per-invocation holder, and needed to fail closed when an upload
validation policy
could not be resolved - an empty allowlist read as "no restriction" would
silently switch off
validation. With no signal available it has to infer:
{code:java}
private boolean isUnresolved(String rawValue, Object paramValue) {
return rawValue != null
&& rawValue.contains("${")
&& (paramValue == null || paramValue.toString().isEmpty());
}
{code}
Two known consequences of the inference, both documented in that class:
* a param that *legitimately* resolves to empty is treated as unresolved. For a
security
control that is the safe direction, so it was accepted - but it is a guess,
not a fact.
* *partial resolution escapes entirely*. For {{${a},${b}}} where only {{${b}}}
fails, the
concatenation is non-empty, so the value is treated as fully resolved and a
**truncated**
value is written. For an allowlist that narrows rather than widens, so it is
not a security
hole today, but nothing detects it and nothing warns.
The second case is the one no amount of care at the call site can fix. It needs
the parser.
h3. Proposal
Give callers access to the resolution outcome. Shape to be decided in design;
the constraint is
that {{TextParseUtil.translateVariables}} is widely used public API, so the
existing signature
and its substitution behaviour must keep working unchanged.
Options worth weighing:
# an additional {{evaluate}} variant returning a small result object carrying
the value plus
which variables failed to resolve
# an optional callback on the parser, notified per unresolved variable,
defaulting to no-op
# extending {{ParsedValueEvaluator}} so the evaluator itself learns resolution
failed
Option 1 or 2 could be purely additive, in which case this need not wait for
8.0.0 - the fix
version above is conservative rather than required.
Once available, {{WithLazyParams.isUnresolved}} should be replaced with the
real signal, and its
javadoc caveats about legitimately-empty values and partial resolution removed.
h3. Non-goals
* *Not* a change to the default substitution behaviour. Replacing an
unresolvable expression
with empty text is long-standing, and applications depend on it for display.
This issue adds a
way to *observe* the outcome, not a way to change it.
* Not a rewrite of OGNL expression evaluation.
h3. References
* {{core/src/main/java/org/apache/struts2/util/OgnlTextParser.java}} - the two
branches that
produce identical output
* {{core/src/main/java/org/apache/struts2/util/TextParseUtil.java}} -
{{translateVariables}}
* {{core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java}} -
{{isUnresolved}},
the inference this would replace
* WW-5659 - the issue that surfaced this
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
