331343 - The timestamp is quoted twice in Bugzilla::Flag::modify()

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, defect)

Description

[lixan]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; .NET CLR 1.0.3705; .NET CLR 2.0.50215)
Build Identifier: 

parameter $timestamp in function Bugzilla::flag::modify allready quoted by
function Bugzilla::flag::process, but quoted again


Reproducible: Always

Steps to Reproduce:

Comment 1

[Frédéric Buclin]

This is a known bug. It will be fixed by bug 300549.

Comment 2

[lixan]

Bug 300549 milestone 2.24.
But this bug block using flags in 2.22

Comment 3

[Frédéric Buclin]

(In reply to comment #2)
> But this bug block using flags in 2.22

You can still use flags in 2.22. I don't think I will fix this bug in 2.22. It will depend on how invasive it is.

Comment 4

[lixan]

This allways reproducible in 2.22 rc1
Step to reproduce:
1. Add flag (If you have one, don't needed).
2. Set flag (in show_bug.cgi) to question '?'.
3. Press "Commit"
3. Update bug.
4. Set flag to '+'.
5. Press "Commit".

And you will get errror in process_bug.cgi

DBD::mysql::st execute failed: Incorrect datetime value: ''2006-03-24 08:25:23'' for column 'modification_date' 

Bugzilla::DB::SendSQL('UPDATE flags\x{a}   SET    setter_id = 1,\x{a}  ...') called at Bugzilla/Flag.pm line 683
Bugzilla::Flag::modify('Bugzilla::CGI=HASH(0x22f1a1c)',                 '\'2006-03-24 08:25:23\'') called at Bugzilla/Flag.pm line 453

I am change Bugzilla\flag.pm line 612
from:
  my $sql_timestamp = ($timestamp ? &::SqlQuote($timestamp) : "NOW()"); 
to:
  my $sql_timestamp = ($timestamp ? $timestamp : "NOW()");

It is enough for closing bug in 2.22.
Better solution correct Bugzilla::flag::process for right  Bugzilla::flag::modify calling. (but this require more modifications)

Of course, in version 2.24 you may fix Bug 300549, ànd this problem disappear.

Comment 5

[Frédéric Buclin]

That's strange that you crash when changing a flag. Which version of MySQL are you using? And which version of DBD::mysql?

Anyway, I will take a closer look to fix it on 2.22 as well (and maybe even on 2.20.2). We have a few days left before the release of 2.22. I will post a patch on time.

Comment 6

[lixan]

(In reply to comment #5)
> That's strange that you crash when changing a flag. Which version of MySQL are
> you using? And which version of DBD::mysql?
Windows 2003 Server with SP1.
Bugzilla 2.22rc1 on IIS 6.0
MySQL 5.0.18 
ActivePerl 5.8.7 Build 518
DBI 1.49 
DBD::mysql 3.0002

But this bug don't depend on it.

Comment 7

[Frédéric Buclin]

(In reply to comment #6)

> But this bug don't depend on it.

It depends. We cannot reproduce the problem using Apache on Linux. But I agree that quoting twice the timestamp doesn't depend on your config.

Comment 8

[Frédéric Buclin]

Attachment: patch, v1

Tested on 2.22 and 2.23.1, on both MySQL and PostgreSQL. On MySQL, due to the timestamp being quoted twice, the modification date is set to '0000-00-00 00:00:00'. This patch fixes this problem.

Comment 9

[Max Kanat-Alexander]

Yeah, there's some dataloss because of this bug (the modification_date), so we should fix it before 2.22.

Comment 10

[Max Kanat-Alexander]

Comment on attachment 217051 [details] [diff] [review]
patch, v1

Seems like it would be better to change the callers to not quote the timestamp. It's strange for a function to expect a quoted timestamp.

Comment 11

[Max Kanat-Alexander]

Comment on attachment 217051 [details] [diff] [review]
patch, v1

Okay, LpSolit says that this is the minimal change needed to fix the bug, and since we're so close to release I think it's a good idea to do the minimum fix.

This is r+ if you add an "XXX" comment saying that we need to change things so that callers don't quote the timestamp.

Comment 12

[Frédéric Buclin]

tip:

Checking in Bugzilla/Flag.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v  <--  Flag.pm
new revision: 1.60; previous revision: 1.59
done

2.22rc1:

Checking in Bugzilla/Flag.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Flag.pm,v  <--  Flag.pm
new revision: 1.59.2.1; previous revision: 1.59
done