groupfeature admin validation refuses valid values

[ietf-svn-bot]

owner:mark@painless-security.com resolution_fixed type_defect | by rjsparks@nostrum.com

---

There are several JSONField fields in GroupFeature
Most of them should be able to hold an empty list '[]' but the admin does not accept that as a valid value, probably because it is falsy and the field is required.

---

Issue migrated from trac:3331 at 2022-03-04 09:06:15 +0000

[ietf-svn-bot]

@rjsparks@nostrum.com changed status from new to accepted

[ietf-svn-bot]

@mark@painless-security.com changed status from accepted to assigned

[ietf-svn-bot]

@mark@painless-security.com set owner to mark@painless-security.com

[ietf-svn-bot]

@mark@painless-security.com commented

---

That analysis seems to be pretty close, but not exactly what's going on. Other people have commented on this, such as on Stack Overflow. The bug is that (), [], and {} are explicitly called out in the JSONFeild class as being empty. There are even multiple bug reports about it.

The Stack Overflow article has a suggestion on how to fix it, which seems straightforward enough. However, this has been fixed in the latest release of django, which poses the question of whether there is a way to fix it in such a way that we remember to un-do it when that django upgrade happens. Robert, do you have a standard method of marking some bit of code as needing updating whenever the next django upgrade happens?

[ietf-svn-bot]

@mark@painless-security.com commented

---

Oh, wait, I see now that we're using a third-party JSONField library instead of one included in django. Let me see what to do with that.

[ietf-svn-bot]

@mark@painless-security.com commented

---

It looks like the jsonfield library suffers from the same pitfall as the django field does. So, I'll go ahead and implement the same solution.

[ietf-svn-bot]

@mark@painless-security.com changed status from assigned to closed

[ietf-svn-bot]

@mark@painless-security.com set resolution to fixed

[ietf-svn-bot]

@mark@painless-security.com commented

---

Fixed in 604d6ed:

Add a new Django field, IETFJSONField

This field is needed because the plain JSONField does not permit empty arrays - [] - or empty objects - {} - when the field is marked as required. Those values explicitly evaluate to a null value, and are rejected.

Instead, the IETFJSONField accepts two new arguments to control this:

• empty_values: An array of values that should evaluate to null/empty, and be rejected.
• accepted_empty_values: An array of values that should not evaluate to null/empty, and be accepted.

This allows the programmer to specify either a positive or negative statement of what values to accept.

Fixes issue #3331. Commit ready for merge.

[ietf-svn-bot]

@rjsparks@nostrum.com commented

---

This commit is missing the migration it needs. I'm taking care of that while merging, but in the future, please be sure to include it.

Does the code still need a flag to indicate it needs attention when upgrading to Django 3?

[ietf-svn-bot]

@rjsparks@nostrum.com commented

---

Fixed in c596605:

Merged in 604d6ed from mark@painless-security.com:
Add a new Django field, IETFJSONField
This field is needed because the plain JSONField does not permit empty arrays - [] - or empty objects - {} - when the field is marked as required. Those values explicitly evaluate to a null value, and are rejected.
Instead, the IETFJSONField accepts two new arguments to control this:

• empty_values: An array of values that should evaluate to null/empty, and be rejected.
• accepted_empty_values: An array of values that should not evaluate to null/empty, and be accepted.
  This allows the programmer to specify either a positive or negative statement of what values to accept.
  Fixes issue groupfeature admin validation refuses valid values #3331.