717217 - The regexp in Bugzilla::BugUrl::JIRA::should_handle() isn't restrictive enough (min two letters required)

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect)

Description

[Simon Green]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:9.0.1) Gecko/20100101 Firefox/9.0.1
Build ID: 20111223083612

Steps to reproduce:

Current code is

sub should_handle {
    my ($class, $uri) = @_;
    return ($uri->path =~ m|/browse/[A-Z]+-\d+$|) ? 1 : 0;



Actual results:

JIRA allows you to specify the regex that a project can be. The default is two or more lower case letters. However in JBoss JIRA the regex is [A-Z][A-Z]+[0-9]{0,2}

http://confluence.atlassian.com/display/JIRA044/Configuring+Project+Keys


Expected results:

There are three possible solutions:

1) Red Hat carry the change ourselves
2) The regex above is changed to m|/browse/\w+-\d+$|
3) We add this as a param value with [A-Z]+-\d+ as the default

I'm in favour of the last one, but wanted your input before writing the code and submitting a patch.

Comment 1

[Frédéric Buclin]

(In reply to Simon Green from comment #0)
> I'm in favour of the last one, but wanted your input before writing the code
> and submitting a patch.

No, we won't accept a parameter for such a minor thing. If JIRA says the correct regexp is  m|/browse/\w+-\d+$|, then we should use it.

Comment 2

[Simon Green]

(In reply to Frédéric Buclin from comment #1)
> No, we won't accept a parameter for such a minor thing.

Thought as much.

> If JIRA says the
> correct regexp is  m|/browse/\w+-\d+$|, then we should use it.

The correct regex for the is |/browse/[A-Z][A-Z]+-\d+$| (i.e. two or more upper case letters, source http://confluence.atlassian.com/display/JIRA044/Configuring+Project+Keys). I have created a patch for this.

Red Hat will carry the change to handle our JIRA server ourselves.

Comment 3

[Simon Green]

Attachment: v1 patch

Comment 4

[Frédéric Buclin]

Comment on attachment 587673 [details] [diff] [review]
v1 patch

timello: this one is for you. Why not using [A-Z]{2,} instead of [A-Z][A-Z]+ ?

Comment 5

[Tiago Mello [:timello]]

(In reply to Frédéric Buclin from comment #4)
> Comment on attachment 587673 [details] [diff] [review]
> v1 patch
> 
> timello: this one is for you. Why not using [A-Z]{2,} instead of [A-Z][A-Z]+
> ?

Yeah, it can be [A-Z]{2,}. I'm going to test it and r+ the patch. We fix it on checkin.

Comment 6

[Tiago Mello [:timello]]

Comment on attachment 587673 [details] [diff] [review]
v1 patch

It looks good and it works as expect. We can either use [A-Z][A-Z]+ or [A-Z]{2,}. However, I think that the first one is more readable, unless there is a good reason to use the last one.

Comment 7

[Tiago Mello [:timello]]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/                                                                                                                                    
modified Bugzilla/BugUrl/JIRA.pm
Committed revision 8096.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.2/                                                                                                                                      
modified Bugzilla/BugUrl/JIRA.pm
Committed revision 8015.