510958 - Allow hooks to exit() under mod_perl

Status: RESOLVED FIXED

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Max Kanat-Alexander]

Under mod_perl, exit() is implemented as a special form of "die". Unfortunately, when called by a hook, this is caught as though it were an error.

There is a workaround specified in the mod_perl docs, here:

http://perl.apache.org/docs/2.0/api/ModPerl/Util.html#C_exit_

Comment 1

[Max Kanat-Alexander]

Attachment: v1

Okay, I've tested this with my own extension and it works.

Comment 2

[Max Kanat-Alexander]

Attachment: v2

The previous patch was throwing the following error under mod_perl when loading Apache:

Prototype mismatch: sub ModPerl::EXIT () vs none at /var/www/html/everythingsolved.com/sites/gnome-working/Bugzilla/Hook.pm line 39.

Comment 3

[David Lawrence [:dkl]]

Ok in testing this patch, I may be hitting up against the extend of my ModPerl knowledge and need to pick your brain to see what I am doing wrong. 

When a person puts an exit() call in an extension .pl code file, what should I be seeing to the client? Currently with the patch I get a internal 500 error and the error_log gives me a premature end of script headers error against whatever cgi I am looking at. Is that what I should be seeing? If I just put a code error of some kind in the .pl file, I then get the normal extension_invalid error message.

Can you maybe paste an example of extension code that utilizes exit and maybe I am doing it in an unsupported away.

FWIW, I get the same premature header error when using exit without the patch.

Thanks
Dave

Comment 4

[Max Kanat-Alexander]

(In reply to comment #3)
> When a person puts an exit() call in an extension .pl code file, what should I
> be seeing to the client?

  You should be seeing nothing. Of course, one would generally expect that CGI headers had been sent first, or Apache might get confused.


> Can you maybe paste an example of extension code that utilizes exit and maybe I
> am doing it in an unsupported away.

  There's an exit() in traceparser here:

  http://bzr.everythingsolved.com/bugzilla-traceparser/trunk/annotate/head%3A/lib/TraceParser/Hooks.pm#L107

> FWIW, I get the same premature header error when using exit without the patch.

  Did you restart httpd after applying the patch? Are the checksetup permissions and the SELinux permissions right?

Comment 5

[David Lawrence [:dkl]]

Comment on attachment 394892 [details] [diff] [review]
v2

Ok, now that I figured out what I was doing wrong prior, I have been able to complete testing for this new function. All works as expected and now when I call exit() from my extension code, it will display the template proper and stop execution without throwing an extension error. r=dkl

Comment 6

[Frédéric Buclin]

Comment on attachment 394892 [details] [diff] [review]
v2

>+    else {
>+        # Create a fake constant. We have to do this in a string eval,
>+        # otherwise this will always be defined.
>+        eval('sub ModPerl::EXIT;');
>+    }

Why is that useful? You never call ModPerl::EXIT if you don't use mod_perl. I don't understand what's the point here.

Comment 7

[Max Kanat-Alexander]

(In reply to comment #6)
> Why is that useful? You never call ModPerl::EXIT if you don't use mod_perl. I
> don't understand what's the point here.

  Because ModPerl::EXIT exists as a bareword in the code.

Comment 8

[Max Kanat-Alexander]

Checking in Bugzilla/Hook.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Hook.pm,v  <--  Hook.pm
new revision: 1.29; previous revision: 1.28
done