editwhines.cgi and whine.pl no longer work in 4.1, because they use Bugzilla::Group::ValidateGroupName() which has been removed in bug 574556. Undefined subroutine &Bugzilla::Group::ValidateGroupName called at /var/www/html/bugzilla/editwhines.cgi line 237.

Oh, and moreover, _contact_exact_group() and _cc_exact_group() now disclose whether a group exists or not, because they now call Bugzilla::Group->check() which throws a message of the form "There is no group named 'foo'." while till now, they were throwing "The group you specified, foo, is not valid here.", which doesn't specify if the group doesn't exist or is not visible. So this is an information leak.

(In reply to comment #1) > Oh, and moreover, _contact_exact_group() and _cc_exact_group() now disclose > whether a group exists or not, Yes, because we decided that it's acceptable to disclose whether a group exists or not. There are already similar changes in Bugzilla::Bug. It's far more important to tell people that they are *not properly protecting a bug* than to hide the names of groups.

You're right about ValidateGroupName still being used, though. I did a grep and I thought that it only showed up in Search.pm, but I will restore it. Thankfully this is a trivially easy patch.

Do you actually want or need to review this patch? It's a simple reversal.

(In reply to comment #2) > Yes, because we decided that it's acceptable to disclose whether a group exists > or not. There are already similar changes in Bugzilla::Bug. It's far more > important to tell people that they are *not properly protecting a bug* than to > hide the names of groups. We can correctly protect bugs without disclosing whether a group exists or not. The message used by invalid_group_name is the correct one: "group foo is not valid here". It doesn't say why it's not valid, and I think that enough information. We should restore the error message.

Comment on attachment 458283 [details] [diff] [review] v1 The two methods in Search.pm I mentioned in comment 1 should re-use ValidateGroupName() to display the right error message. And this will also allow us to display a common error message independently of the reason the group is not acceptable here.

I don't know if you read the patch on the other bug, but I explicitly removed ValidateGroupName and replaced it with an object method instead. You are reporting a regression from the removal of ValidateGroupName, in other files. The patch I attached fixes that bug. If you have a problem with the new policy regarding group name disclosure, we should take that up in a separate bug, because it is a different issue.

I've posted to the developers list about the group name policy change, because it's really something that everybody should see, and the explanation was very long and involved.

Comment on attachment 458283 [details] [diff] [review] v1 r=LpSolit

Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/ modified Bugzilla/Group.pm Committed revision 7388.