[jira] [Work logged] (WW-5704) HTML5 required false-rejects on radio/file when the bound property is never null

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/WW-5704?focusedWorklogId=1041399&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-1041399 ]

ASF GitHub Bot logged work on WW-5704:
--------------------------------------

                Author: ASF GitHub Bot
            Created on: 14/Sep/26 12:23
            Start Date: 14/Sep/26 12:23
    Worklog Time Spent: 10m 
      Work Description: lukaszlenart opened a new pull request, #1939:
URL: https://github.com/apache/struts/pull/1939

   Fixes [WW-5704](https://issues.apache.org/jira/browse/WW-5704)
   
   ## Problem
   
   `RequiredFieldValidator` never sees the request; it inspects the bound property and fails only on null, an empty array or an empty collection. A primitive `int` behind a radio group, or a file property `prepare()` loaded from an existing entity, can therefore never fail server-side — while the browser's `required` still blocks an unselected group or an empty file input. A false reject, which the feature's governing rule forbids.
   
   ## Fix
   
   A radio or file input omits its parameter when left empty, so on such a submit the property keeps whatever it holds — and that is the value the page is rendering. So `required` is emitted only while the bound value is one the validator itself would reject:
   
   | render-time value | radio/file at render | server on empty submit | `required` |
   |---|---|---|---|
   | `null`, `[]`, empty collection | nothing selected | fails | emitted — sides agree |
   | primitive `0`, off-list value, `prepare()`-loaded file | nothing selected | **passes** | not emitted — the ticket's false reject |
   | in-list value | pre-checked | passes | not emitted — inert either way |
   
   - `HtmlConstraintProvider.constraintsFor` gains an `Object value` parameter (the tag's resolved `nameValue`); `UIBean` passes it through. Unreleased interface (`@since 7.4.0`), no deprecation.
   - `StrutsHtmlConstraintProvider.addRequiredField` asks the validator instance: `RequiredFieldValidator.isMissing(value)`, extracted from `validate` so the two cannot drift — the WW-5703 lesson.
   - `File` keeps the raw property instead of a `String` conversion, because OGNL renders a null property as `""` under that conversion, which would hide a missing attachment; no `file.ftl` in any theme reads `nameValue`. The raw lookup honours `struts.el.throwExceptionOnFailure`, and an `UploadedFilesAware` action has no property behind `<s:file name="upload"/>`, so `File` tolerates exactly that one failure (a broken expression still surfaces) — found by `/code-review`, pinned by two tests.
   
   Residual, documented on the site page: a property null on GET but populated only on POST is the one case this cannot see.
   
   ## Verification
   
   - `Html5ConstraintRenderingTest` now pushes `ConstraintAction` (the object whose validators run) and adds: `int`-backed radio renders no `required`; `prepare()`-loaded attachment renders no `required`; null attachment still does; a property-less file input renders under `throwExceptionOnFailure`; a broken expression still throws there.
   - `RequiredFieldValidatorTest.testIsMissingMatchesWhatValidateRejects` pins the predicate.
   - `mvn test -DskipAssembly -pl core` green.
   - Security review and `/code-review high` run; the review's one finding (the throw-on-failure regression) is fixed in the second commit.
   
   Docs: apache/struts-site PR updates `client-side-validation.md`.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   




Issue Time Tracking
-------------------

            Worklog Id:     (was: 1041399)
    Remaining Estimate: 0h
            Time Spent: 10m

> HTML5 required false-rejects on radio/file when the bound property is never null
> --------------------------------------------------------------------------------
>
>                 Key: WW-5704
>                 URL: https://issues.apache.org/jira/browse/WW-5704
>             Project: Struts 2
>          Issue Type: Bug
>          Components: Core Tags
>            Reporter: Lukasz Lenart
>            Assignee: Lukasz Lenart
>            Priority: Major
>             Fix For: 7.4.0
>
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> Found reviewing WW-5695 (PR #1865). Violates that feature's governing rule: emit a constraint only when the browser cannot reject input the server would accept.
> h3. What happens
> {{StrutsHtmlConstraintProvider.addRequiredField}} emits {{required}} for RADIO and FILE only, and its javadoc explains why:
> bq. A control that submits an empty string rather than omitting the parameter therefore passes server-side while the browser blocks it [...] Only RADIO and FILE omit the parameter entirely when empty, so only they agree with the browser.
> That reasoning is about the HTTP request. {{RequiredFieldValidator}} never sees the request - it inspects the already-bound property:
> {code:java}
> public void validate(Object object) throws ValidationException {
>     String fieldName = getFieldName();
>     Object value = this.getFieldValue(fieldName, object);
>     if (value == null) {
>         addFieldError(fieldName, object);
>     } else if (value.getClass().isArray() && Array.getLength(value) == 0) {
>         addFieldError(fieldName, object);
>     } else if (Collection.class.isAssignableFrom(value.getClass()) && ((Collection) value).isEmpty()) {
>         addFieldError(fieldName, object);
>     }
> }
> {code}
> For a primitive-typed or otherwise default-initialised property the server can therefore _never_ fail, while the browser's {{required}} still can.
> h3. Reproduction
> The action declares {{private int priority;}} with a {{required}} field-validator on it, rendered as {{s:radio}} with {{name}} of {{priority}} and a list of 1, 2, 3. On first render nothing is selected, because 0 is not in the list.
> * Browser: no radio in the group is selected, so the submit is blocked.
> * Server: {{getFieldValue}} returns {{Integer.valueOf(0)}}, which is not null, so no error is added and the form would have been accepted.
> Same shape for {{s:file}} whenever {{prepare()}} pre-populates the file property from an existing entity, which is the ordinary edit-an-attachment flow.
> h3. Why it is not simply fixable in the provider
> There is no render-time way to know a property's default value, so the two honest options are:
> * drop the {{RequiredFieldValidator}} to {{required}} mapping entirely, leaving only {{requiredstring}}; or
> * keep it and document it as a deliberate, narrow deviation from the never-false-reject rule.
> h3. Not affected
> The {{requiredstring}} mapping is sound and should stay: {{RequiredStringValidator}} fails on null, on empty, and by default on blank, so the browser's {{required}} can only reject what the server would also reject. That mapping reasons about the value, not the request, and so does not have this defect.
> h3. Test gap
> No test renders {{required}} end-to-end on {{s:radio}} or {{s:file}}; only {{minlength}} is asserted through a template.



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