670116 - flagtypes.cc_list accepts NULL values, but the code doesn't handle it correctly

Status: NEW

(Bugzilla :: Attachments & Requests, enhancement)

Description

[Frédéric Buclin]

The flagtypes.cc_list DB column is of type varchar(200), i.e. it accepts NULL values, but the code in Bugzilla always assumes cc_list to be defined, i.e. its value is expected to be "" when the CC list is empty (that's because the CC list field in editflagtypes.cgi passes an empty string when left empty).

In config/generate_test_data.pl (our QA script to generate data), we leave this field undefined, and so NULL is stored in the DB. This makes

 split(/[, ]+/, $cc_list)

to throw "Use of uninitialized value $cc_list in split at Bugzilla/Flag.pm line 955." every time it accesses this flagtype.

So we have two ways to fix the problem:
1) Change the DB schema to "varchar(200) NOTNULL" for this column.
2) Fix the code to a) store NULL instead of "" in the DB when the CC list is empty, and b) fix the code to correctly handle undefined CC lists.

Personally, I have no strong opinion. 1) would be easier as we wouldn't have to track all places in the code to make sure it doesn't fail when it gets "undef". mkanat, do you have a preference?

In the meantime, I will fix config/generate_test_data.pl to explicitly pass "" for the CC list, instead of undef.

Comment 1

[Frédéric Buclin]

generate_test_data.pl has been patched:

Committing to: bzr+ssh://[e-mail]/bugzilla/qa/4.0/
modified config/generate_test_data.pl
Committed revision 192.

Committing to: bzr+ssh://[e-mail]/bugzilla/qa/3.6/
modified config/generate_test_data.pl
Committed revision 153.

Committing to: bzr+ssh://[e-mail]/bugzilla/qa/3.4/
modified config/generate_test_data.pl
Committed revision 120.

Comment 2

[Max Kanat-Alexander]

I think NOT NULL DEFAULT '' sounds good to me. Keeps things simple.