zopefoundation / zope.schema

http://zopeschema.readthedocs.io
Other
7 stars 22 forks source link

Required List accepts empty list #123

Closed szakitibi closed 1 year ago

szakitibi commented 1 year ago

What I did:

I have created a form using Plone 6. The form had a required List field. The built in OrderedSelectWidget accepted the empty list - see plone.app.z3cform #168, #169, #170.

The OrderedSelectWidget has been fixed with commit plone.app.z3cform@0c07c38e, but I'm concerned about the underlying issue with validation. In case someone removes the required attribute and submits the form, it will be accepted on backend.

What actually happened:

For example a required List validates None type properly:

>>> from zope.schema import List
>>> obj = List(title="A required list", required=True)
>>> obj.validate(None)
*** zope.schema._bootstrapinterfaces.RequiredMissing

But passing an empty list raises no error:

>>> obj.validate([])

What I expect to happen:

>>> obj.validate([])
*** zope.schema._bootstrapinterfaces.RequiredMissing

Note:

I'm aware of the IMinMaxLength, but using that results too short error:

>>> obj = List(title="Workaround", required=True, min_length=1)
>>> obj.validate([])
*** zope.schema._bootstrapinterfaces.TooShort: ([], 1)

Which looks odd, when a field is marked with the required red dot.

What version of Python and Zope/Addons I am using:

Plone 6.0.5 (6016) CMF 3.0 Zope 5.8.2 Python 3.10.6 (main, May 29 2023, 11:10:38) [GCC 11.3.0] PIL 9.5.0 (Pillow) WSGI: On Server: waitress 2.1.2

szakitibi commented 1 year ago

O.K.

This could be a bit problematic. I've did a test run with a potential fix.

Making required Collections actually required could brake things a ton of unexpected ways. Like Plone 6 instance right away fails to start due to this line https://github.com/plone/plone.app.contenttypes/blob/master/plone/app/contenttypes/browser/migration.py#L47 . Although if I remember correctly using default=[] is a big NO-NO according to Plone docs.

d-maurer commented 1 year ago

szakitibi wrote at 2023-6-9 06:11 -0700:

What I did:

I have created a form using Plone 6. The form had a required List field. The built in OrderedSelectWidget accepted the empty list - see plone.app.z3cform #168, #169, #170.

The OrderedSelectWidget has been fixed with commit @.***(https://github.com/plone/plone.app.z3cform/commit/0c07c38eb0433796fb3d2816faecd29eb7b347da), but I'm concerned about the underlying issue with validation. In case someone removes the required attribute and submits the form, it will be accepted on backend.

An empty list is a perfect list - thus, a List field with the empty list as value has a value.

Keep in mind that zope.schema not only targets the browser interface (via a widget mediation) but also the program code. In the latter case, a missing list value would be represented by None and not by the empty list.

I see 3 options for you:

  1. on the schema level: use a constraint (not_empty)

  2. on the converter (i.e. field<->widget value) level: register a converter for the field with maps an empty selection to None

  3. on the validator level: register a custom validator for the field

szakitibi commented 1 year ago

An empty list is a perfect list - thus, a List field with the empty list as value has a value.

:+1: