Need to have unique id for fields in fieldsets

[frapell]

Having 2 fields with the same id (i.e. when an existing fieldset is copied) leads to unintended errors.

Steps to reproduce:

1. Create a FormFolder
2. Remove all fields
3. Add a FieldsetFolder called Folder 1
4. Create a FormStringField inside of it called Name (It should default to 255 for Max Length, if not, make it so)
5. Back to the FormFolder go to folder_contents view
6. Mark Folder 1, click copy and then paste
7. Rename the copy as Folder 2 with folder-2 as id

At this point, you should have a /folder-1 and /folder-2 folders, and a name field inside each of them.

If you submit this form, you get

Traceback (innermost last):
  Module ZPublisher.Publish, line 138, in publish
  Module ZPublisher.mapply, line 77, in mapply
  Module ZPublisher.Publish, line 48, in call_object
  Module Products.CMFFormController.FSControllerPageTemplate, line 91, in __call__
  Module Products.CMFFormController.BaseControllerPageTemplate, line 26, in _call
  Module Products.CMFFormController.FormController, line 383, in validate
  Module ZPublisher.mapply, line 77, in mapply
  Module ZPublisher.Publish, line 48, in call_object
  Module Products.CMFFormController.FSControllerValidator, line 57, in __call__
  Module Products.CMFFormController.Script, line 145, in __call__
  Module Products.CMFCore.FSPythonScript, line 127, in __call__
  Module Shared.DC.Scripts.Bindings, line 322, in __call__
  Module Shared.DC.Scripts.Bindings, line 359, in _bindAndExec
  Module Products.PythonScripts.PythonScript, line 344, in _exec
  Module script, line 41, in fgvalidate_base
   - <FSControllerValidator at /Plone/test-form/fgvalidate_base>
   - Line 41
  Module Products.PloneFormGen.content.form, line 562, in fgvalidate
  Module Products.Archetypes.Field, line 348, in validate
  Module Products.Archetypes.Field, line 362, in validate_validators
  Module Products.validation.chain, line 137, in __call__
  Module Products.PloneFormGen.validators.MaxLengthValidator, line 40, in __call__
AttributeError: 'list' object has no attribute 'replace'

The reason for this, is because the form has 2 fields with the same id, and so when the form gets submitted, instead of having 2 separate values, they are merged and treated as a list with 2 values.

Now, if you manually create the second folder, instead of copy & pasting, the field inside of it will be named name-1, which prevents this bug.

A possible fix would be to add a subscriber so when pasting content, it will be traversed and all children would be renamed to avoid conflicts...

A better fix in my opinion would be to add a prefix to the field id and name attributes qhen rendering the form, containing the fieldsets ids. So for instance in this case, the form would have 2 fields, one with an id and name of folder-1-name and a second one with id and name of folder-2-name

[spereverde]

We also have this issue on production sites. I agree with the renaming being a good fix option, maybe be aware that it might break honeypot solutions since they use field id's. Does anyone know if this issue also happens in collective.easyform, or how it's fixed there?

[ThibautBorn]

As far as I know, there is no option to copy fieldsets in collective.easyform. If users manually try to reuse a name/id in 2 fieldsets, easyform prevents it due to a validation check. People could in theory bypass the validation by editing the xml directly, which leads to other validation issues on sent.

[frapell]

@spereverde Could you expand a bit on what you mean by "honeypot solutions" ?

[spereverde]

@frapell sure.

• https://admin.kuleuven.be/icts/english/services/wms/content/forms_PloneFormGen (look for honeypot in page)
• https://sixfeetup.com/blog/captchas-plone-site-honeypot-fields

In easyform there is a custom class field for this, in pfg I guess this honeypot field would be hidden with custom css or css inside a package somewhere, using the field's unique css id/class (= shortname of field). I guess in most cases it's not a problem since the honeypotfield is usually not inside a folder or a fieldset, but when it is developers would have to update the css in their setups

[spereverde]

Maybe useful to note that we usually have this issue with old and new style fieldsets, and not with folders

[frapell]

@spereverde Thanks! wasn't aware of such techniques :) My idea of fix (by prefixing the fieldset id to the field id) should only affect the fields created inside fieldsets, so these should continue to work fine... I guess a note should be made in the README to be very clear how it works.

[mauritsvanrees]

I fear that this may break custom code, for example custom validators or scripts that expect to find "name" in the request, and now it should be "fieldset-name" or something. But then: if submitting already does not work, and this fixes it, then it is a win.

But I would only change these names/ids on the fly when it is really needed. So only change anything when there actually is a duplicate field name.

[spereverde]

For consistency, we might consider using validation as in easyform?