leejo / cgi-fast

The new home for CGI::Fast, removing it from the original CGI.pm distribution
4 stars 5 forks source link

CGI::Fast prevents upload hooks from being passed to the constructor #19

Closed ikegami closed 5 years ago

ikegami commented 5 years ago

Hi,

A CGI upload hook is used, among other reasons, to prevent sensitive user data from being written to disk.

The hook must be provided to the CGI constructor, but CGI::Fast mistakenly identifies the hook as an initializer, and this prevents a hook from being used.

The following version of new address the problem:

sub new {
    my ($self, @args) = @_;

    if (!$args[0] || ( ref($args[0]) && UNIVERSAL::isa($args[0], 'CODE') && !$args[3] ) ) {
        $Ext_Request ||= _create_fcgi_request($in_fh, $out_fh, $err_fh );
        return undef unless $Ext_Request->Accept >= 0;
    }

    CGI->_reset_globals;
    $self->_setup_symbols(@CGI::SAVED_SYMBOLS) if @CGI::SAVED_SYMBOLS;
    return $CGI::Q = $self->SUPER::new(@args);
}
ikegami commented 5 years ago

This bug report and patch was prompted by a call for help on StackOverflow.

leejo commented 5 years ago

@ikegami - thanks. Do you have a test case I can use along with this?

ikegami commented 5 years ago

I imagine you have a basic test case and a test case with an initializer.

The following should be equivalent to the basic test case:

CGI::Fast->new()
CGI::Fast->new(\&hook)
CGI::Fast->new(\&hook, "anything")
CGI::Fast->new(\&hook, "anything", 0)

The following should be equivalent to the intializer test case:

CGI::Fast->new($initializer)
CGI::Fast->new(\&hook, "anything", 0, $initializer)

I believe the upload hook is called for file uploads and put requests. The value returned by the hook isn't used. "anything" is passed to the hook.

leejo commented 5 years ago

@ikegami - patch is applied, I shall upload this to CPAN in a couple of days. Thanks!