CGI::Fast not honoring ENV values unless in BEGIN [rt.cpan.org #70609]

leejo commented 10 years ago

https://rt.cpan.org/Ticket/Display.html?id=70609

According to the POD, CGI::Fast honors the environment variables
FCGI_SOCKET_PATH and FCGI_LISTEN_QUEUE. However, experimentally these
variables only take effect if assigned in a BEGIN block prior to loading
CGI::Fast. Without this, invoking the program causes it to simply run
once and exit. eg.

#!perl
use CGI::Fast;

local $ENV{FCGI_SOCKET_PATH} = ":9000";
local $ENV{FCGI_LISTEN_QUEUE} = 20;

while ($q = CGI::Fast->new) {
    print $q->header;
    print "<html><body>The foo input is ", $q->param('foo'),
"</body></html>";
}

prints 

Content-Type: text/html; charset=ISO-8859-1

<html><body>The foo input is </body></html>

on STDOUT and exits immediately. To fix this you have to change the code
as follows:

BEGIN {
    $ENV{FCGI_SOCKET_PATH} = ":9000";
    $ENV{FCGI_LISTEN_QUEUE} = 20;
}
use CGI::Fast;

I can't say whether this is a code or POD bug since I'm not sure what
the original intent was, but it's definitely one or the other.

leejo commented 10 years ago

mark@summersault.com - 2011-08-31 14:00:18

Thanks for the report. This sounds like something to fix.

leejo commented 10 years ago

MSCHWERN - 2011-08-31 21:06:10

If I may add my $0.02... I believe the best internal fix is to delay the creation of the request until it is needed in new(). Something like this in the constructor...

$request ||= create_fcgi_request();
return unless $request->Accept >= 0;

And then create_fcgi_request is something like...

sub create_fcgi_request {
    if( $ENV{FCGI_SOCKET_PATH} ) {
        my $path    = $ENV{FCGI_SOCKET_PATH};
        my $backlog = $ENV{FCGI_LISTEN_QUEUE} || 100;
        my $socket  = FCGI::OpenSocket( $path, $backlog );
        return FCGI::Request( \*STDIN, \*STDOUT, \*STDERR,
                              \%ENV, $socket, 1 );
    }
    else {
        return FCGI::Request();
    }
}

This will have the benefit of making the code work as currently documented and eliminates the awkward "configure before load" trap.

In addition, because environment variables are an awkward way to configure a module from the inside, I'd suggest adding this alternative to the interface...

use CGI::Fast socket_path => ':9000', listen_queue => '50';

This makes it impossible to set the configuration too late. These arguments would override the defaults and the environment variables.

This is still a global set on module load, which can bring problems. Usually configuring the object, not the module, is better. But since CGI::Fast is only intended to be loaded once per CGI process it should be ok. And if it turns out to be necessary, an additional override can be added to the object constructor later.

leejo commented 10 years ago

mark@summersault.com - 2011-09-01 13:59:38

If I may add my $0.02... I believe the best internal fix is to delay the creation of the request until it is needed in new(). Something like this in the constructor...
$request ||= create_fcgi_request();
return unless $request->Accept >= 0;
And then create_fcgi_request is something like...
sub create_fcgi_request {
    if( $ENV{FCGI_SOCKET_PATH} ) {
        my $path    = $ENV{FCGI_SOCKET_PATH};
        my $backlog = $ENV{FCGI_LISTEN_QUEUE} || 100;
        my $socket  = FCGI::OpenSocket( $path, $backlog );
        return FCGI::Request( \*STDIN, \*STDOUT, \*STDERR,
                              \%ENV, $socket, 1 );
    }
    else {
        return FCGI::Request();
    }
}
This will have the benefit of making the code work as currently documented and eliminates the awkward "configure before load" trap.

In addition, because environment variables are an awkward way to configure a module from the inside, I'd suggest adding this alternative to the interface...
use CGI::Fast socket_path => ':9000', listen_queue => '50';
This makes it impossible to set the configuration too late. These arguments would override the defaults and the environment variables.

This is still a global set on module load, which can bring problems. Usually configuring the object, not the module, is better. But since CGI::Fast is only intended to be loaded once per CGI process it should be ok. And if it turns out to be necessary, an additional override can be added to the object constructor later.

Thanks for the input! We'll consider that solution.

Mark

leejo / cgi-fast

CGI::Fast not honoring ENV values unless in BEGIN [rt.cpan.org #70609] #3