[
https://issues.apache.org/jira/browse/WW-5691?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Lukasz Lenart updated WW-5691:
------------------------------
Fix Version/s: 8.0.0
(was: 7.4.0)
Description:
Part of the WW-5689 lean-core work. Retargeted from 7.4.0 to 8.0.0 to pair with
WW-5693: both remove an internal-but-overridable template from core, so they
belong in the major together, under one migration-guide entry.
Scope note: this concerns _core's own_ FreeMarker dependency only; tag-library
users still require FreeMarker via the default FTL themes.
h2. Current behaviour
When an action is still running and the package defines no {{wait}} result,
{{ExecuteAndWaitInterceptor}} builds one itself:
{code}FreemarkerResult waitResult = new FreemarkerResult();
container.inject(waitResult);
waitResult.setLocation("/org/apache/struts2/interceptor/wait.ftl");
waitResult.execute(actionInvocation);
{code}
This dates to WW-3068 — the configuration became unmodifiable, so the earlier
trick of registering a fake {{wait}} result was no longer possible and the
interceptor started instantiating the result directly. The consequence is a
hard {{import org.apache.struts2.views.freemarker.FreemarkerResult}} in an
interceptor that has nothing to do with the view layer.
Note there is no startup cost to reclaim here, unlike WW-5690: the result is
constructed only when the fallback actually fires. Removing the import is the
entire value of this ticket, so the rewrite is the whole job.
h2. What the template actually needs
{{wait.ftl}} is ten lines of content, but one of them is a tag, not just
FreeMarker:
{code}meta http-equiv="refresh" content="5;url=<@s.url includeParams="none"/>"
{code}
With neither {{value}} nor {{action}} set, that resolves to the *mapped action
URL*, not the request URI. Traced through {{ServletUrlRenderer.renderUrl}} (the
{{value == null && action == null}} branch) into
{{Component.determineActionURL}}, it reduces to two collaborators that are both
core-owned injectable beans, so no {{components}} import is needed:
{code}String method = proxy.isMethodSpecified() ? proxy.getMethod() : null;
ActionMapping mapping = new ActionMapping(proxy.getActionName(),
proxy.getNamespace(), method, null);
String uri = actionMapper.getUriFromActionMapping(mapping);
String url = urlHelper.buildUrl(uri, request, response, null,
request.getScheme(), true, true, false, false);
{code}
({{includeContext}} true, {{encode}} true, {{forceAddSchemeHostAndPort}} false;
{{escapeAmp}} is moot because {{includeParams="none"}} means there are no
parameters.)
h2. Escaping is mandatory, and it is currently free
{{FreemarkerManager}} configures {{HTMLOutputFormat.INSTANCE}} with
{{ENABLE_IF_DEFAULT_AUTO_ESCAPING_POLICY}}, so the interpolated URL is
HTML-escaped today without anyone thinking about it. A plain-Java renderer
loses that. The value goes straight into an HTML attribute
({{content="5;url=..."}}) and is built from the context path and action
mapping, so it must be escaped deliberately. Same trap as WW-5693, and sharper
here.
h2. Work
# Write a characterisation test pinning the currently rendered wait page. *No
test covers this fallback path at all today* — grep for {{wait.ftl}} across
{{core/src/test}} returns nothing — so this is new coverage, not a refactor of
existing coverage.
# Render the fallback wait page in plain Java, escaping the URL into the
attribute.
# Delete {{wait.ftl}} and the {{FreemarkerResult}} import.
An application that defines its own {{wait}} result never reaches this code and
is unaffected throughout.
h2. Compatibility
The fallback only fires when no {{wait}} result is configured, and that case
already logs a WARN recommending one be added. The affected population is an
application in that state which _also_ overrides
{{/org/apache/struts2/interceptor/wait.ftl}} on the classpath. Small, but a
real override break — hence 8.0.0 and a migration-guide entry shared with
WW-5693.
was:
Part of the WW-5689 lean-core work, but self-contained and shippable in 7.x.
See WW-5690 for the shared scope note — this concerns _core's own_ FreeMarker
dependency only; tag-library users still require FreeMarker via the default FTL
themes.
h2. Current behaviour
When an action is still running and the package defines no {{wait}} result,
{{ExecuteAndWaitInterceptor}} builds one itself:
{code}FreemarkerResult waitResult = new FreemarkerResult();
container.inject(waitResult);
waitResult.setLocation("/org/apache/struts2/interceptor/wait.ftl");
waitResult.execute(actionInvocation);
{code}
This dates to WW-3068 — the configuration became unmodifiable, so the earlier
trick of registering a fake {{wait}} result was no longer possible and the
interceptor started instantiating the result directly. The consequence is a
hard {{import org.apache.struts2.views.freemarker.FreemarkerResult}} in an
interceptor that has nothing to do with the view layer.
h2. Proposal
The built-in fallback wait page must not hard-reference a view technology.
# Introduce a small pluggable strategy for rendering the fallback wait page,
defaulting to a template-free renderer that emits the same meta-refresh markup
{{wait.ftl}} produces today.
# Keep the FreeMarker path available as an alternative implementation for
anyone who has overridden {{wait.ftl}}.
An application that defines its own {{wait}} result is unaffected either way —
that path never reaches this code.
h2. Compatibility
{{wait.ftl}} is internal. Behaviour of the rendered page (meta refresh, delay,
title) must stay identical; worth a test that pins the emitted markup before
the swap.
> Remove core's FreeMarker dependency from ExecuteAndWaitInterceptor's fallback
> wait page
> ---------------------------------------------------------------------------------------
>
> Key: WW-5691
> URL: https://issues.apache.org/jira/browse/WW-5691
> Project: Struts 2
> Issue Type: Task
> Components: Core
> Reporter: Lukasz Lenart
> Priority: Major
> Fix For: 8.0.0
>
>
> Part of the WW-5689 lean-core work. Retargeted from 7.4.0 to 8.0.0 to pair
> with WW-5693: both remove an internal-but-overridable template from core, so
> they belong in the major together, under one migration-guide entry.
> Scope note: this concerns _core's own_ FreeMarker dependency only;
> tag-library users still require FreeMarker via the default FTL themes.
> h2. Current behaviour
> When an action is still running and the package defines no {{wait}} result,
> {{ExecuteAndWaitInterceptor}} builds one itself:
> {code}FreemarkerResult waitResult = new FreemarkerResult();
> container.inject(waitResult);
> waitResult.setLocation("/org/apache/struts2/interceptor/wait.ftl");
> waitResult.execute(actionInvocation);
> {code}
> This dates to WW-3068 — the configuration became unmodifiable, so the earlier
> trick of registering a fake {{wait}} result was no longer possible and the
> interceptor started instantiating the result directly. The consequence is a
> hard {{import org.apache.struts2.views.freemarker.FreemarkerResult}} in an
> interceptor that has nothing to do with the view layer.
> Note there is no startup cost to reclaim here, unlike WW-5690: the result is
> constructed only when the fallback actually fires. Removing the import is the
> entire value of this ticket, so the rewrite is the whole job.
> h2. What the template actually needs
> {{wait.ftl}} is ten lines of content, but one of them is a tag, not just
> FreeMarker:
> {code}meta http-equiv="refresh" content="5;url=<@s.url includeParams="none"/>"
> {code}
> With neither {{value}} nor {{action}} set, that resolves to the *mapped
> action URL*, not the request URI. Traced through
> {{ServletUrlRenderer.renderUrl}} (the {{value == null && action == null}}
> branch) into {{Component.determineActionURL}}, it reduces to two
> collaborators that are both core-owned injectable beans, so no {{components}}
> import is needed:
> {code}String method = proxy.isMethodSpecified() ? proxy.getMethod() : null;
> ActionMapping mapping = new ActionMapping(proxy.getActionName(),
> proxy.getNamespace(), method, null);
> String uri = actionMapper.getUriFromActionMapping(mapping);
> String url = urlHelper.buildUrl(uri, request, response, null,
> request.getScheme(), true, true, false, false);
> {code}
> ({{includeContext}} true, {{encode}} true, {{forceAddSchemeHostAndPort}}
> false; {{escapeAmp}} is moot because {{includeParams="none"}} means there are
> no parameters.)
> h2. Escaping is mandatory, and it is currently free
> {{FreemarkerManager}} configures {{HTMLOutputFormat.INSTANCE}} with
> {{ENABLE_IF_DEFAULT_AUTO_ESCAPING_POLICY}}, so the interpolated URL is
> HTML-escaped today without anyone thinking about it. A plain-Java renderer
> loses that. The value goes straight into an HTML attribute
> ({{content="5;url=..."}}) and is built from the context path and action
> mapping, so it must be escaped deliberately. Same trap as WW-5693, and
> sharper here.
> h2. Work
> # Write a characterisation test pinning the currently rendered wait page. *No
> test covers this fallback path at all today* — grep for {{wait.ftl}} across
> {{core/src/test}} returns nothing — so this is new coverage, not a refactor
> of existing coverage.
> # Render the fallback wait page in plain Java, escaping the URL into the
> attribute.
> # Delete {{wait.ftl}} and the {{FreemarkerResult}} import.
> An application that defines its own {{wait}} result never reaches this code
> and is unaffected throughout.
> h2. Compatibility
> The fallback only fires when no {{wait}} result is configured, and that case
> already logs a WARN recommending one be added. The affected population is an
> application in that state which _also_ overrides
> {{/org/apache/struts2/interceptor/wait.ftl}} on the classpath. Small, but a
> real override break — hence 8.0.0 and a migration-guide entry shared with
> WW-5693.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
