When making a file upload element conditionally required when another field is filled or another option is chosen, the field doesn't become required.
Steps te reproduce:
1. Create a form
2. Create a radiobutton element
3. Option A and option B
4. Create a file upload element, set it to not required
5. In the conditional settings, make it required when radiobutton element has Option A selected
6. Select option A and submit the form
7. The form is submitted
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | conditional_required-2860529-9.jpeg | 57.97 KB | artureiji |
| #7 | conditional_required-2860529-7.patch | 924 bytes | jrockowitz |
| #5 | webform.webform.2860529_cond_required_file (1).yml | 2.93 KB | jrockowitz |
| #4 | webform.webform.2860529_cond_required_file.yml | 2.72 KB | jrockowitz |
Comments
Comment #2
jrockowitz commentedI am willing to bet that core's #states API does not handle requiring file uploads. Still the required attribute is supported by most browsers. https://www.w3schools.com/jsref/prop_fileupload_required.asp
Comment #3
artureiji commentedHi @jrockowitz,
I've run some tests considering what you've commented and indeed if the required="required" attribute is set on the file input, the browser supports it's validation.
But since that attribute can be removed by browser tools (e.g. chrome inspector) isn't that still an (security?) issue since that there's no backend validation for those conditional fields?
I've tested the form mentioned by @FkoNL but with an additional textfield, which worked perfectly with #state API. But removing the required attribute resulted in both the file field and the textfield not being validated.
Comment #4
jrockowitz commentedThe fact the conditional logic in Webform 8.x-5.x is client-side only means that malicious users can bypass conditionally required fields. Users submitting bad data is not exactly a security issue... but it is still a problem.
Attached is a test form that demonstrates that conditionally required file upload are not validated.
Comment #5
jrockowitz commentedThis example includes text field which is required.
Comment #6
jrockowitz commentedThis is definitely a core issue. Please create a ticket. I might be able to come up with temporary work-around.
Comment #7
jrockowitz commentedPlease test the attached patch.
Comment #9
artureiji commentedWorked like a charm.
Nice work @jrockowitz
Attached evidence screenshot.
regards,
Artur Aciole
Comment #10
jrockowitz commentedI committed the patch.