191085 - process_bug.cgi: FetchSQLData vs. FetchOneColumn and other oddness

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, defect, P1)

Description

[Tobias Burnus]

I was told that one gets this in process_bug.cgi:
 You tried to change the product field from 1 to ZEDV, but only the owner or
 submitter of the bug, or a sufficiently empowered user, may change that
 field.

This seems to be fixed by:
--- process_bug.cgi     28 Nov 2002 10:49:41 -0000      1.172
+++ process_bug.cgi     29 Jan 2003 10:34:33 -0000
@@ -218,7 +218,7 @@
 if ( $::FORM{'id'} ) {
     SendSQL("SELECT name FROM products, bugs " .
             "WHERE products.id = bugs.product_id AND bug_id = $::FORM{'id'}");
-    $::oldproduct = FetchSQLData();
+    $::oldproduct = FetchOneColumn();
 }

Using my test accound (no extra rights at all): If I load a bug and do
absolutely no changes and hit "Submit" I get:
  Changes submitted for bug 42
  Email sent to: no one
  Excluding: (all others [my real account should get all)
This is still the case with this patch and might be related to bug 187861.

(One might change this for consistancy, but it is not needed:
@@ -775,7 +775,7 @@
 # to which the bug is restricted.
 if ( $::FORM{'id'} ) {
     SendSQL("SELECT group_id FROM bug_group_map WHERE bug_id = $::FORM{'id'}");
-    my ($havegroup) = FetchSQLData();
+    my $havegroup = FetchOneColumn();

Comment 1

[Bradley Baetz (:bbaetz)]

Bug 187861 is only related to multipart POST; ie only attachment.cgi

Where do you get that message where you shouldn't? I can't repro this.

Waht perl/DBI version?

Those two should be identical...

Comment 2

[Tobias Burnus]

Attachment: Pass the value not the number of objects in the array

> Bug 187861 is only related to multipart POST; ie only attachment.cgi
> Where do you get that message where you shouldn't? I can't repro this.
I think I cannot reproduce it in the moment either. Simply ignore that part.

The attachement fixes:
-    $::oldproduct = FetchSQLData();
+    $::oldproduct = FetchOneColumn();

and changes for consitency
-    my ($havegroup) = FetchSQLData();
+    my $havegroup = FetchOneColumn();

If you don't like the latter, ignore that ;-)

Comment 3

[Bradley Baetz (:bbaetz)]

Oh, hang on. Bleh, my mistake.

Untested patch coming

Comment 4

[Bradley Baetz (:bbaetz)]

Attachment: fix

The eval I added hid the wantarray-ness from the caller, and so we always got
an array which was then coerced into scalar context - ie the number of
elements.

Doing the return directly from within the eval seems to not work, so try this
instead.

Can someone pleae verify that this fixes this problem, and that it does not
regress bug 189446?

Comment 5

[Bradley Baetz (:bbaetz)]

jussi, please see comment 4. You reported that other bug...

Comment 6

[Jussi Sirpoma]

Yes, the problem in bug 189446 comes back when I use this patch.

Comment 7

[Bradley Baetz (:bbaetz)]

jussi: What perl version? What if you also localise ->{HandleError} as well?

Comment 8

[Jussi Sirpoma]

Re comment 7:

I'm still seeing the bug after adding: 
local Bugzilla->instance->dbh->{HandleError};
 
perl version is 5.6.1

Comment 9

[Bradley Baetz (:bbaetz)]

Jussi. Hmm. With that patch, can you:

a) change _handle_error so taht you can check whether its still being called.
b) Turn on tracing (add a call to ->trace(2, "trace.out") after conncting in
Bugzilla::DB::_connect), and attach that log.

What DBD::mysql version?

Comment 10

[Jussi Sirpoma]

Attachment: trace from a failed attempt

a) Yes, _handle_error is called with the modified patch (with HandleError)

Checking for		 DBI (v1.32)   ok: found v1.32
Checking for	  DBD::mysql (v2.1010) ok: found v2.1014

Comment 11

[Bradley Baetz (:bbaetz)]

Hmm. Maybe try setting those attribtues on the sth, rather than the dbh?

ie |local $_current_sth->{RaiseError};| ?

Comment 12

[Jussi Sirpoma]

Yes, that works!

Comment 13

[Bradley Baetz (:bbaetz)]

Attachment: take 2

Disable ->{RaiseError} on the sth, not the dbh.

Comment 14

[Dave Miller [:justdave]]

Comment on attachment 113321 [details] [diff] [review]
take 2

ok, looks sane to me, assuming the intention is to make it ignore the error...

Comment 15

[Bradley Baetz (:bbaetz)]

Fixed.