Closed Bug 525025 Opened 15 years ago Closed 15 years ago

Custom field names having UTF8-characters in them crash Bugzilla

Categories

(Bugzilla :: Creating/Changing Bugs, defect)

defect
Not set
major

Tracking

()

RESOLVED FIXED
Bugzilla 3.4

People

(Reporter: LpSolit, Assigned: LpSolit)

Details

(Keywords: intl)

Attachments

(2 files, 1 obsolete file)

Attached patch patch, v1 (obsolete) — Splinter Review
Create a custom field name with UTF-8 characters in it, e.g. "cf_déjà_עברית". The creation succeeds, and you can add values to this field without any problem.

Now file a new bug and submit it:

invalid bug attribute cf_déjà _×¢×ר×ת at Bugzilla/Bug.pm line 3668
	Bugzilla::Bug::AUTOLOAD('Bugzilla::Bug=HASH(0x95e5cb0)') called at Bugzilla/Bug.pm line 811
	Bugzilla::Bug::update('Bugzilla::Bug=HASH(0x95e60e0)', '2009-10-28 19:47:05') called at /var/www/html/bugzilla/post_bug.cgi line 249


Now when you fix AUTOLAOD, you realize you cannot update this custom field in the bug you filed. No error is thrown, and all other changes are committed correctly, but this field remains unaltered. That's because CGI.pm seems to need UTF-8 parameter names to be encoded.
Flags: blocking3.4.3?
Attachment #408892 - Flags: review?(mkanat)
Comment on attachment 408892 [details] [diff] [review]
patch, v1

>+        if (Bugzilla->params->{'utf8'} && $_[0] =~ /^cf_/) {
>+            utf8::encode($_[0]) if utf8::is_utf8($_[0]);
>+        }

  That will change it in the caller, which may not be what we intend.


  In actual fact, we should not be allowing UTF-8 column names. I'm surprised they work at all.
Attachment #408892 - Flags: review?(mkanat) → review-
Do you prefer this? :)
Attachment #408892 - Attachment is obsolete: true
Attachment #408998 - Flags: review?(mkanat)
Comment on attachment 408998 [details] [diff] [review]
patch for 3.4.3, v2

Yeah, that looks great.

But what do we do for installations that already have UTF-8 column names? Just tell them to delete them?
Attachment #408998 - Flags: review?(mkanat) → review+
I suppose deleting them is reasonable, since they never could have been updated, right?
Flags: blocking3.4.3?
Flags: blocking3.4.3+
Flags: approval3.4+
Flags: approval+
(In reply to comment #4)
> I suppose deleting them is reasonable, since they never could have been
> updated, right?

Right. No bugs can have these fields set (as post_bug.cgi crashes and process_bug.cgi ignores these fields), and so they can easily mark these fields as obsolete and delete them.
Summary: Custom field names having UTF8-characters in them crash Bugzilla and cannot be updated → Custom field names having UTF8-characters in them crash Bugzilla
Keywords: intl
IMHO this had its chance to work: 

1) before bug 363153;

2) with some old CGI.pm version (like ТТ 2.16..2.19, one of which most likely caused bug 457524)

I've also asked localizers@bugzilla.org for feedback.
(In reply to comment #6)
> IMHO this had its chance to work: 
> 
> 1) before bug 363153;

  Hmm. Except, at that time, \w+ on the string would only have matched ASCII A-Za-z_ characters. So if you had a Unicode sequence that translated exactly to those (is that even possible?) it would have worked, but otherwise not.

> 2) with some old CGI.pm version (like ТТ 2.16..2.19, one of which most likely
> caused bug 457524)

  Mmm, we did the CGI.pm param() translation to utf8 ourselves as part of bug 363153.
Comment on attachment 408998 [details] [diff] [review]
patch for 3.4.3, v2

As discussed on IRC, we will take this minimal change on the 3.4 branch, but on HEAD we are going to let checksetup.pl remove invalid custom field names.

So we will still need to relnote it for 3.4.3 as we won't delete them automatically.
Attachment #408998 - Attachment description: patch, v2 → patch for 3.4.3, v2
It "could" work, with some UTF8-ignorant-but-safe environment including certain
browsers, CGI/TT module versions, Apache/squid settings combinations...

My point is not to be so certain in our intentions to delete such fields.  

Perhaps we're facing regression introduced by bug 363153.  Then attachment
408892 [review] is probably a valid approach as well.
(In reply to comment #9)
> It "could" work, with some UTF8-ignorant-but-safe environment

But the custom field name is also used as DB column name, and I'm not sure all DBs support all UTF8 characters in their name. So it's still safer to reject those characters.
Attachment #409608 - Flags: review?(mkanat)
Comment on attachment 409608 [details] [diff] [review]
patch for tip, v3

Looks good to me! :-)
Attachment #409608 - Flags: review?(mkanat) → review+
tip:

Checking in Bugzilla/Field.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Field.pm,v  <--  Field.pm
new revision: 1.47; previous revision: 1.46
done
Checking in Bugzilla/Install/DB.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/DB.pm,v  <--  DB.pm
new revision: 1.74; previous revision: 1.73
done


3.4.2:

Checking in Bugzilla/Field.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Field.pm,v  <--  Field.pm
new revision: 1.43.2.1; previous revision: 1.43
done
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
In the interest of doing the releases while I have time, I added this bug to the release notes and checked in the release note item without review.

tip:

Checking in template/en/default/pages/release-notes.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/release-notes.html.tmpl,v  <--  release-notes.html.tmpl
new revision: 1.42; previous revision: 1.41
done

3.4.3:

Checking in template/en/default/pages/release-notes.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/pages/release-notes.html.tmpl,v  <--  release-notes.html.tmpl
new revision: 1.33.2.9; previous revision: 1.33.2.8
done
we suspect this change blocked us from creating custom searches with UTF8 char names.

Is that possible reason? (not sure if custom search is a custom field :)).

We updated bugs.aviary.pl to 3.4.3 and noticed it immediately.
(In reply to comment #15)
> we suspect this change blocked us from creating custom searches with UTF8 char
> names.

You cannot have UTF8 characters in custom field names (while UTF8 characters are still allowed in custom field descriptions, which is what the user see) as they crash Bugzilla. If you already had such characters in custom field names, you are in trouble already.
(In reply to comment #15)
> we suspect this change blocked us from creating custom searches with UTF8 char
> names.

How exactly this fails? Have a look at bug 477513.
(In reply to comment #15)
> we suspect this change blocked us from creating custom searches with UTF8 char
> names.

Unable to reproduce on landfill:

https://landfill.bugzilla.org/bugzilla-3.4-branch/buglist.cgi?cmdtype=runnamed&namedcmd=%E3%82%8F%E3%81%9F%E3%81%97%E3%81%AE%20bug%20|%20%D0%9C%D0%BE%D0%B8%20%D0%BE%D1%88%D0%B8%D0%B1%D0%BA%D0%B8
That's totally unrelated to this bug. Custom searches and custom field names have nothing to do together. This bug is about custom field names.
You need to log in before you can comment on or make changes to this bug.