Item5946: using static CGI methods not fastcgi safe

How to reproduce:

1. set up TWiki using mod_fastcgi or mod_fcgid
2. create two topics that both use a TWikiForm, not necessarily the same form definition, but with overlapping formfield names in the definitions
3. open topic1 in tab1
4. open topic2 in tab2
5. save topic1
6. reload topic2 in tab2
7. BAM: you get the form info of topic1 loaded into topic2

How to debug:

sub renderForEdit {
    my( $this, $web, $topic, $value ) = @_;

print STDERR "Text::renderForEdit($web, $topic, $value)\n";

    my $textfield =
        CGI::textfield(
            -class => $this->cssClasses('twikiInputField',
                                        'twikiEditFormTextField'),
            -name => $this->{name},
            -size => $this->{size},
            -value => $value );

print STDERR "textfield=$textfield\n";

    return ( '', $textfield );
}

Now go watch your error logs. You will get something like this:

Text::renderForEdit(Morgue, TopicType, TopicStub to TopicType)
textfield=<input type="text" name="Summary" value="places for free PSD resources" size="75" class="twikiInputField twikiEditFormTextField" />

Note, that CGI::textfield did NOT use the $value given to it. I can't say why

Ways to resolve this

    my $textfield = $session->{cgi}->textfield(...);

Anybody more familiar with CGI ?

-- TWiki:Main/MichaelDaum - 20 Aug 2008

Comments

Hummm... I really need to look at this. I already had a problem with global variables used by CGI, but it should be solved now. How did you setup fastcgi? (FastCGI engine from TWikiStandAlone scratch branch needs revision after some changes performed recently).

-- TWiki:Main.GilmarSantosJr - 20 Aug 2008

I think I've found it: TWiki::Engine::CLI::prepareBody() does not call CGI::initialize_globals() sometimes.

-- TWiki:Main.MichaelDaum - 20 Aug 2008

When I comment out the content length check, things are fine again. There are other methods that do a content length check ... and may fail also. Don't know.

sub prepareBody {
    my ( $this, $req ) = @_;
print STDERR "prepareBody called\n";

#    return unless $ENV{CONTENT_LENGTH};
    CGI::initialize_globals();

print STDERR "init globals\n";

    my $cgi = new CGI();
    my $err = $cgi->cgi_error;
    throw TWiki::EngineException($1, $2)
        if defined $err && $err =~ /\s*(\d{3})\s*(.*)/o;
    $this->{cgi} = $cgi;
}

-- TWiki:Main.MichaelDaum - 20 Aug 2008

Yeah. It makes sense. I placed CGI::initialize_globals() before new CGI, but if CGI is used without OO interface, then it uses the previous-cached object (in FastCGI context, data from other request).

I'll remove CGI::initialize_globals() from TWiki::Engine::CGI::prepareBody() and put it at the beginning of request loop in TWiki::Engine::FastCGI::run(). CGI engine doesn't need it, since there is one process for request, I mean, every request has a clean context.

I'm settin this to urgent and AppliesTo Extension, FastCGIEngineContrib. (I'll fix this as soon as I start to work on it).

-- TWiki:Main.GilmarSantosJr - 20 Aug 2008

For now I took your fastcgi code from the standalone scratch area. It would be great to have that code available as a contrib. I've done some changes to FastCGI.pm already to accomodate autoreoading if twiki_fastcgi.pl 's file modification time changes. We will also need some way that configure forces all persistent backends (fastcgi, http) to reload their code so that new configuration settings take effect.

-- TWiki:Main.MichaelDaum - 21 Aug 2008

It'll be available, as soon as I can. Refer to tasks at Item4662 and TWiki:Codev.PersonalRoadmapForGilmarSantosJr

I need to finish unit tests to work on packaging other engines. I have some enhancements in mind to both FastCGI and HTTP.

The auto reload feature is an option to HTTP engine, but should be added to other persistent engines (and without adding dependencies).

-- TWiki:Main.GilmarSantosJr - 21 Aug 2008

Okay. No problem. For now I am using the fastcgi stuff from your scratch area ... and have come across a couple of issues already. For example, we can't fork anymore, that is TWiki::Sandbox::sysCommand() fails. I was able to work around it by not cloning STDERR in the fastcgi constructor and replacing it by some other filebased IO::File. Again, I have no further clue why this is so.

-- TWiki:Main.MichaelDaum - 22 Aug 2008

The problem is that FCGI.pm ties STDOUT, STDERR and STDIN. So, after fork we have to untie STDERR before closing it. See the lasts comments at TWiki:Codev.TWikiStandAlone. I'll try to have unit tests, documentation upgrades and FastCGIEngineContrib done until sunday night.

-- TWiki:Main.GilmarSantosJr - 22 Aug 2008