Closed
Bug 365697
Opened 18 years ago
Closed 16 years ago
email_in.pl is case-sensitive for products due to Bugzilla::User->can_enter_product
Categories
(Bugzilla :: Incoming Email, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 3.0
People
(Reporter: altlist, Assigned: LpSolit)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
mkanat
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mkanat
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1
Build Identifier:
email_in.pl properly sends a notification when it doesn't recognize a product name. However, if the product name matches from a case-insensitive point of view, I get a rather obtuse error message:
can_enter_product reached an unreachable location.
Reproducible: Always
Comment 1•18 years ago
|
||
Wow, that's really weird. Are you on PostgreSQL or MySQL? Could you tell me what the two products were that you were using, and if they have any group controls on them?
Assignee: general → create-and-change
Severity: normal → major
Component: Bugzilla-General → Creating/Changing Bugs
Target Milestone: --- → Bugzilla 3.0
Version: unspecified → 2.23.3
Reporter | ||
Comment 2•18 years ago
|
||
I'm using MySQL. The product names only differed by the case sensitivity, such as "FireFox" vs. "Firefox". But only "FireFox" exists, not "Firefox".
I don't use groups right now.
Comment 3•18 years ago
|
||
Here's the code that's at fault, I'm pretty sure:
my $can_enter =
grep($_->name eq $product_name, @{$self->get_enterable_products});
Assignee: create-and-change → mkanat
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 4•18 years ago
|
||
Okay, this should fix it, and it makes sense to me.
Product names are case-insensitive on both MySQL and PostgreSQL.
Attachment #250394 -
Flags: review?(LpSolit)
Assignee | ||
Comment 5•18 years ago
|
||
Comment on attachment 250394 [details] [diff] [review]
v1
If we want can_enter_product() to be case-insensitive, then we should also make can_see_product() case-insensitive.
Also, you have to make sure that you don't introduce any regression, neither in enter/post/process_bug.cgi, nor in editproducts/components/milestones/versions.cgi. This makes me think: how could you change the case of a product with this change, i.e. if you have a product FireFox, and you want to rename it as Firefox? I know we have a bug about that already, and we should think about this first.
Attachment #250394 -
Flags: review?(LpSolit) → review-
Reporter | ||
Comment 6•18 years ago
|
||
> This makes me think: how could you change the case of a product with this
> change, i.e. if you have a product FireFox, and you want to rename it as
> Firefox?
We have the same problem with email addresses, although I can't find a ticket for that.
Updated•17 years ago
|
Component: Creating/Changing Bugs → Incoming Email
Assignee | ||
Comment 8•16 years ago
|
||
This is IMO cleaner, especially now that we know that lc() has some problems with some locales.
Assignee: mkanat → LpSolit
Attachment #250394 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #327716 -
Flags: review?(mkanat)
Comment 9•16 years ago
|
||
Comment on attachment 327716 [details] [diff] [review]
patch for 3.2 and HEAD, v2
Why not just call Product->check?
Assignee | ||
Comment 10•16 years ago
|
||
(In reply to comment #9)
> Why not just call Product->check?
Because ->check will inform the user that the product doesn't exist instead of the current message which says it either doesn't exist or you don't have access to it. This is the kind of information we don't want to leak.
Comment 11•16 years ago
|
||
Comment on attachment 327716 [details] [diff] [review]
patch for 3.2 and HEAD, v2
Okay, sure, this looks fine then! :-)
Attachment #327716 -
Flags: review?(mkanat) → review+
Comment 12•16 years ago
|
||
I'm not sure of the performance impact of that change, so I don't really want to approve it for 3.0.
Flags: approval3.2+
Flags: approval+
Assignee | ||
Comment 13•16 years ago
|
||
Minimal change for 3.0.5: I use lc()
Attachment #327716 -
Attachment is obsolete: true
Attachment #327872 -
Flags: review?(mkanat)
Updated•16 years ago
|
Attachment #327872 -
Flags: review?(mkanat) → review+
Updated•16 years ago
|
Attachment #327716 -
Attachment description: patch, v2 → patch for 3.2 and HEAD, v2
Attachment #327716 -
Attachment is patch: false
Updated•16 years ago
|
Attachment #327716 -
Attachment is obsolete: false
Attachment #327716 -
Attachment is patch: true
Assignee | ||
Comment 15•16 years ago
|
||
tip:
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v <-- User.pm
new revision: 1.166; previous revision: 1.165
done
3.1.4:
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v <-- User.pm
new revision: 1.164.2.1; previous revision: 1.164
done
3.0.4:
Checking in Bugzilla/User.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/User.pm,v <-- User.pm
new revision: 1.148.2.7; previous revision: 1.148.2.6
done
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Summary: email_in.pl interface is case-sensitive for products → email_in.pl is case-sensitive for products due to Bugzilla::User->can_enter_product
You need to log in
before you can comment on or make changes to this bug.
Description
•