[jira] [Created] (WW-5702) Polish HTML5 constraint derivation: scope gaps and attribute hygiene found reviewing WW-5695

["Lukasz Lenart (Jira)" <ji...@apache.org>]

Lukasz Lenart created WW-5702:
---------------------------------

             Summary: Polish HTML5 constraint derivation: scope gaps and attribute hygiene found reviewing WW-5695
                 Key: WW-5702
                 URL: https://issues.apache.org/jira/browse/WW-5702
             Project: Struts 2
          Issue Type: Task
            Reporter: Lukasz Lenart


Follow-up to WW-5695 (PR #1865), collecting review findings that are real but not blocking. None of these is a false-reject; the false-reject class was fixed on the WW-5695 branch itself.

The governing rule for the feature is unchanged: emit a constraint only when the browser cannot reject input the server would accept, and never set or change an input's {{type}}.

h3. 1. Derivation is not gated on the theme

{{UIBean.addConstraintAttributes}} guards on {{struts.ui.html5.constraints}} but not on the resolved theme, while only {{template/html5/common-attributes.ftl}} renders the map. Enabling the constant for a few html5 forms makes every xhtml/simple/css\_xhtml form in the application resolve its action's validators and call {{getMessage()}} per field to build a map nothing renders. Gate on the resolved theme as well as the constant. Note {{ConstraintAttributesTest}} never calls {{setTheme}}, so it currently asserts derivation under the default xhtml theme.

h3. 2. data-msg-* is emitted regardless of control type

{{StrutsHtmlConstraintProvider.constraintsFor}} calls {{addMessage}} for every validator independently of whether {{addConstraints}} derived anything and independently of the control. A checkbox whose {{required}} constraint was deliberately suppressed still renders {{data-msg-required}}, and {{s:label}} renders validation text onto an element that never submits. Gate {{addMessage}} on the control supporting at least one constraint.

The test that should catch this is vacuous: {{StrutsHtmlConstraintProviderTest}}'s helper passes a null action, and {{addMessage}} returns immediately on null, so no assertion in that class exercises the message path at all. Fix the helper to pass a real action.

h3. 3. Forms with no action attribute resolve validators under an empty context

{{Form.resolveActionValidators}} calls {{findString(action)}}, but {{action}} is frequently null: {{ServletUrlRenderer.renderFormUrl}} resolves the name from the current invocation into {{attributes.actionName}} without writing it back to the field. {{findString(null)}} does not throw - {{OgnlTextParser.evaluate}} coerces null to the empty string - and {{DefaultActionMapper.getMappingFromActionName("")}} returns a non-null mapping, so the {{attributes.actionName}} fallback is unreachable and the context becomes the empty string. Alias-scoped {{ActionClass-alias-validation.xml}} validators are then silently skipped, for the most common form usage.

This defect is inherited from the deprecated {{getValidators(String)}}, not introduced by WW-5695; what is new is that the path now runs for every field of every form. Fall back to {{attributes.actionName}} when {{findString(action)}} is blank. No test covers an action-less form.

h3. 4. Visitor-validated nested fields get a message and never a constraint

{{Form.findFieldValidators}} wraps prefixed-field validators in {{Form.FieldVisitorValidatorWrapper}}, which implements only {{FieldValidator}}. {{addConstraints}} dispatches on concrete validator types, so for the common nested-bean case - a textfield named {{user.name}} behind a {{visitor}} validator on {{user}} - no constraint is ever derived, while the wrapper's {{getValidatorType()}} returns the literal {{field-visitor}} and {{getMessage()}} delegates, so the field renders {{data-msg-field-visitor}} and nothing else. Unwrap via {{getFieldValidator()}} before dispatching, or skip wrapped validators entirely. Same visitor blind spot recorded against WW-3530.

h3. 5. max is emitted without the type guard applied to min

{{addRange}} guards {{min}} through {{isIntegral}} but passes {{max}} straight to {{putIfPresent}}, which stringifies whatever it is given. A date-typed or string-typed range validator on a control the developer declared numeric emits a nonsense {{max}}. {{addDoubleRange}} has the same asymmetry for NaN and infinite upper bounds. Browsers ignore an unparseable bound, so this is invalid markup rather than a false reject.

h3. 6. isIntegral rounds through double

{{isIntegral}} tests {{doubleValue()}} while {{putIfPresent}} renders the original object, so a custom range validator carrying a BigDecimal such as 1.0000000000000000001 passes the guard and then renders that fractional value as {{min}}, shifting the HTML step base. No shipped range validator is affected - only Short, Int, Long and Date subclasses exist - so this needs a custom validator to reach. Determine integrality from the decimal representation instead.

h3. 7. CHECKBOX and HIDDEN are unreachable control types

The enum declares both but only six components override {{getControlType()}}, so {{Checkbox}} and {{Hidden}} inherit {{OTHER}} and a replacement provider cannot distinguish them. Returning their real types is behaviourally inert with the default provider, which ignores both, so this is purely about the extension contract being honest.

h3. 8. The provider javadoc advertises a type override that cannot work

{{HtmlConstraintProvider}}'s javadoc suggests a replacement provider could map an email validator to {{type="email"}}. It cannot: {{TextField}} only puts {{type}} into the attribute map when the developer set it, so {{isAlreadyRendered}} does not filter it, and {{html5/text.ftl}} has already emitted a hardcoded {{type="text"}} by the time the constraint map renders - producing a duplicate attribute of which the browser keeps the first. The example also contradicts the never-change-type rule stated in {{StrutsHtmlConstraintProvider}}. Correct the javadoc, and consider dropping {{type}} from the derived map outright.

h3. 9. Dynamic attribute names are compared case-sensitively

{{isAlreadyRendered}} does an exact-key lookup in {{dynamicAttributes}}, but HTML attribute names are ASCII case-insensitive. A dynamic {{MAXLENGTH}} misses the check and renders after the derived {{maxlength}}, so the browser keeps the derived one - the opposite of the documented "the developer's own value always wins". Compare dynamic keys case-insensitively.

h3. 10. data-msg attribute names are not sanitised

The name is {{data-msg-}} concatenated with the validator type. FreeMarker's HTML auto-escaping protects the value, not the name, and validator types are freely named in validators.xml, so a type containing a space or an equals sign splits into a second attribute. Config-controlled, so low risk; a character-class guard is one line.

h3. 11. The allowlist admits patterns the browser refuses to compile

HTML compiles {{pattern}} with the Unicode flag, under which an identity escape outside a character class is restricted to syntax characters plus the solidus. An escaped hyphen outside a class is a SyntaxError and the whole attribute is then ignored, so a pattern that passes {{EcmaScriptSafeRegex.isSafe}} can be emitted and silently do nothing. Safe direction - no false reject - but the allowlist should be honest and permit the hyphen escape only inside a character class. Related and very low: {{isSafe}} clears its in-class state at the first unescaped closing bracket, so a literal-bracket class is modelled as class-then-literal, matching neither engine.

h3. 12. Six html5 templates can never render constraints

{{constraints.ftl}} is reachable only through {{html5/common-attributes.ftl}}, which combobox, datetextfield, doubleselect, updownselect, optiontransferselect and inputtransferselect do not include. datetextfield is the notable one given the enum already models the temporal control types. Coverage gap rather than a bug; worth recording alongside the deliberately deferred temporal min/max formatting.

h3. 13. Visitor validators are re-resolved once per field

{{resolveActionValidators()}} caches the top-level list once per form, but the visitor branch inside {{findFieldValidators}} calls the validator manager once per field, and the manager caches only the config list - it constructs fresh validator instances on every call. An N-field form with one visitor validator does N full instantiation passes per render. {{FormFieldValidatorsTest}} pins the top-level call count only.

h3. Suggested test additions
* a form with no action attribute (item 3 has no coverage today)
* a real action passed through the provider test helper, so the message path stops being invisible (item 2)
* end-to-end template rendering of pattern, and of required on radio and file; only minlength is currently asserted through a template



--
This message was sent by Atlassian Jira
(v8.20.10#820010)