request.cgi crashes if a flag modification date is invalid

RESOLVED FIXED in Bugzilla 3.4

Status

()

P1
critical
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: LpSolit, Assigned: LpSolit)

Tracking

Bugzilla 3.4
Bug Flags:
approval +

Details

Attachments

(1 attachment)

(Assignee)

Description

10 years ago
Due to a very old bug in 2.20 and older (bug 365264), some flags have a modification date set to 0000-00-00. strptime() doesn't return undef and so DateTime tries to build an object with this invalid data, crashing Bugzilla. To fix that, we could either let checksetup.pl fix these dates, e.g. by setting them to the creation date, which is better than nothing, or let sanitycheck.cgi find them and offers to fix them using the same workaround as above (maybe safer?), or let Util::format_time() detect the invalid date and return ''.

While testing data passed to format_time(), it appears that strptime() sets the month to -1 for 0000-00-00 00:00:00. I have no idea if this is something "official" we can use or not to detect invalid dates, but this could also be a way to fix the problem without altering the DB.

Max, which solution do you prefer?

Comment 1

10 years ago
I'd prefer to have checksetup fix it.
(Assignee)

Updated

10 years ago
Severity: normal → critical
Priority: -- → P1
(Assignee)

Comment 2

10 years ago
Created attachment 337295 [details] [diff] [review]
patch, v1
Assignee: attach-and-request → LpSolit
Status: NEW → ASSIGNED
Attachment #337295 - Flags: review?(mkanat)

Comment 3

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

If modification_date is NULL, that will never match (NULL is never less than anything).

Change "have" to "had" in the message.

Also, some DB drivers might not return the number of rows modified from do().
Attachment #337295 - Flags: review?(mkanat) → review-

Updated

10 years ago
Attachment #337295 - Flags: review- → review+

Comment 4

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

Okay, so do returns -1 when we don't know the rows, so that's fine.

And LpSolit pointed out that we don't need to fix NULL modification_time columns (and creation_time is never NULL). So this is fine, just fix the "have" on checkin.

Updated

10 years ago
Flags: approval+
(Assignee)

Comment 5

10 years ago
Checking in Bugzilla/Install/DB.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/DB.pm,v  <--  DB.pm
new revision: 1.56; previous revision: 1.55
done
(Assignee)

Updated

10 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.