Bugzilla 3.0.x lets you mark bugs as VERIFIED or CLOSED with no resolution

RESOLVED FIXED in Bugzilla 3.0

Status

()

Bugzilla
Creating/Changing Bugs
--
major
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Frédéric Buclin, Assigned: Frédéric Buclin)

Tracking

3.0.3
Bugzilla 3.0
Bug Flags:
approval3.0 +
blocking3.0.4 +
blocking2.22.4 -
blocking2.20.6 -

Details

(Whiteboard: [doesn't affect 3.2])

Attachments

(1 attachment)

(Assignee)

Description

10 years ago
By hacking the URL, it's possible to ask Bugzilla 3.0.x to close a bug as VERIFIED or CLOSED without giving it any resolution, and no error is thrown. The reason is that process_bug.cgi assumes that if you want to mark a bug as such, then the bug is already resolved. This is a very bad assumption.

Bugzilla 3.2 is not affected, but I'm almost 100% sure that Bugzilla 2.20.x and 2.22.x are affected as process_bug.cgi is mostly the same across the 2.x series and 3.0.
Flags: blocking3.0.4+
Flags: blocking2.22.4?
Flags: blocking2.20.6?
(Assignee)

Updated

10 years ago
Whiteboard: [doesn't affect 3.2]
(Assignee)

Comment 1

10 years ago
Created attachment 304808 [details] [diff] [review]
patch, v1
Assignee: create-and-change → LpSolit
Status: NEW → ASSIGNED
Attachment #304808 - Flags: review?(justdave)

Comment 2

10 years ago
This isn't a security or dataloss issue, so it doesn't go on the older branches.
Flags: blocking2.22.4?
Flags: blocking2.22.4-
Flags: blocking2.20.6?
Flags: blocking2.20.6-
(Assignee)

Comment 3

10 years ago
Well, it still corrupts the data as you have closed bugs with no resolution at all. And sanitycheck.cgi is complaining about them. So not dataloss, but data corruption. That's not really better.

Comment 4

10 years ago
(In reply to comment #3)
> Well, it still corrupts the data as you have closed bugs with no resolution at
> all. And sanitycheck.cgi is complaining about them. So not dataloss, but data
> corruption. That's not really better.

  Well, I wouldn't say it's data corruption, it's just that the database gets into a state that it's not supposed to be in. All the data is *there* and available, it's just that certain fields aren't correctly coordinated. Also, you can only make it happen by hacking the URL. So no, this doesn't need a backport that far.
(Assignee)

Comment 5

10 years ago
Comment on attachment 304808 [details] [diff] [review]
patch, v1

/me suspects this patch to remain in Dave's long review queue for too long. Asking Max to have a chance to get a review this year. ;)
Attachment #304808 - Flags: review?(mkanat)

Comment 6

10 years ago
Comment on attachment 304808 [details] [diff] [review]
patch, v1

Yeah, this is fine.

However, since this is 3.0, you should say "reopen them" instead of "mark them as REOPENED".
Attachment #304808 - Flags: review?(mkanat)
Attachment #304808 - Flags: review?(justdave)
Attachment #304808 - Flags: review+
(Assignee)

Comment 7

10 years ago
(In reply to comment #6)
> However, since this is 3.0, you should say "reopen them" instead of "mark them
> as REOPENED".

"mark them as REOPENED" will never happen. Only "mark them as VERIFIED/CLOSED", as discussed on IRC.
Flags: approval3.0+
(Assignee)

Comment 8

10 years ago
Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.351.2.8; previous revision: 1.351.2.7
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v  <--  user-error.html.tmpl
new revision: 1.204.2.10; previous revision: 1.204.2.9
done
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Comment on attachment 304808 [details] [diff] [review]
patch, v1

>+  [% ELSIF error == "bug_status_not_allowed" %]
>+    [% title = "Bug Status Not Allowed" %]
>+    You cannot mark open [% terms.bugs %] as [% status_descs.$status FILTER html %].
>+    You have to mark them as [% status_descs.RESOLVED FILTER html %] first.

You obviously didn't run tests, or you would have noticed the "Bug" usage. ;)

Changed "Bug" to "[% terms.Bug %]".

Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v  <--  user-error.html.tmpl
new revision: 1.204.2.11; previous revision: 1.204.2.10
done

Comment 10

10 years ago
(In reply to comment #9)
> You obviously didn't run tests, or you would have noticed the "Bug" usage. ;)

  I'm fairly sure the tests don't catch that inside strings.
(In reply to comment #10)
> (In reply to comment #9)
> > You obviously didn't run tests, or you would have noticed the "Bug" usage. ;)
> 
>   I'm fairly sure the tests don't catch that inside strings.

They should. :(
(Assignee)

Comment 12

10 years ago
(In reply to comment #10)
> (In reply to comment #9)
> > You obviously didn't run tests, or you would have noticed the "Bug" usage. ;)
> 
>   I'm fairly sure the tests don't catch that inside strings.
> 

I obviously ran tests, thanks! Tinderbox didn't burn, meaning that running runtests.pl locally didn't complain either. And yes, Max is right, our tests do not catch "Bug" in directives, as per bug 233282.

Comment 13

10 years ago
(In reply to comment #11)
> They should. :(

  Patches accepted: bug 233282
You need to log in before you can comment on or make changes to this bug.