I ran into an "interesting" bug in CGI.pm today, and figured I'd post here as this is a potential problem in Apache 2/modperl 2 setups.

I have a form of application server that runs as a CGI in a modperl 2 system, where the client submits a request in XML format, using a POST request.

Last night an error was reported - looking in the log I saw that the data from the POST had been truncated. After a bit of searching, including using tcpdump to verify that the data was really being sent correctly, I found a post on the modperl mailing list which pointed in the right direction.

It appears that in some circumstances apache 2/modperl 2 don't read the full POST in a single call.

CGI.pm has the following call to read the POST: 

sub read_from_client {
    my($self, $buff, $len, $offset) = @_;
    local $^W=0;                # prevent a warning
    return $MOD_PERL
        ? $self->r->read($$buff, $len, $offset)
        : read(\*STDIN, $$buff, $len, $offset);
}

[download]
The problem here is that $self->r->read() doesn't necessary read $len bytes, and CGI.pm doesn't check this.

My fix, based on a fix posted by Stas Bekman in the modperl list, is to simply call read_from_client() in a loop. The patch to CGI.pm (version 3.01) looks like this: 

*** CGI.pm~ 2003-12-12 15:09:45.000000000 -0800
--- CGI.pm 2003-12-12 15:12:29.000000000 -0800
***************
*** 551,558 ****
        }
   
        if ($meth eq 'POST') {
!    $self->read_from_client(\$query_string,$content_length,0)
!        if $content_length > 0;
     # Some people want to have their cake and eat it too!
     # Uncomment this line to have the contents of the query string
     # APPENDED to the POST data.
--- 551,565 ----
        }
   
        if ($meth eq 'POST') {
!  if($content_length > 0) {
!    my $len = $content_length;
!    while ($len > 0) {
!      my $data = '';
!      my $read = $self->read_from_client(\$data,$content_length,0);
!      $len -= $read;
!      $query_string .= $data if $read > 0;
!    }
!  }
     # Some people want to have their cake and eat it too!
     # Uncomment this line to have the contents of the query string
     # APPENDED to the POST data.

[download]
Michael

Replies are listed 'Best First'.
Re: CGI.pm bug in Apache 2/mod-perl 2 setup
by tachyon (Chancellor) on Dec 13, 2003 at 02:48 UTC

    You have created an infinite loop in the event that $content_length > data_delivered_via_post. This could (will) occur if the client terminates the connection part way through the post which is quite possible with long posts. The Content-Length header arrives before the physical content stream. In CGI::Simple the code looks like: 

            if ( $length ) {
            # we may not get all the data we want with a single read o
+n large
            # POSTs as it may not be here yet! Credit Jason Luther for
+ patch
            # CGI.pm < 2.99 suffers from same bug
            read( STDIN, $data, $length );
            while ( length($data) < $length ) {
              last unless read( STDIN, my $buffer, 4096 );
              $data .= $buffer;
            }
            unless ( $length == length $data ) {
                $self->cgi_error( "500 Bad read on POST! wanted $lengt
+h, got ".(length $data) );
              return;
            }
        }

    [download]

    The significant point is that we exit the loop if we do not read any data. This the second exit condition you need to include in your patch to preclude going infinite if you don't get all the expected data for whatever reason. I would suggest something like this (totally untested code) perhaps: 

      if($content_length > 0) {
    my $len = $content_length;
    while ($len > 0) {
      my $data = '';
      my $read = $self->read_from_client(\$data,$len,0);
     last unless $read;
      $len -= $read;
      $query_string .= $data;
    }
  }

    [download]

    This will exit the loop if we get no data and only ask for what we still expect.....

    cheers

    tachyon

[reply]
[d/l]
[select]
      Ah - nice catch. Thanks!

      Michael

[reply]
Re: CGI.pm bug in Apache 2/mod-perl 2 setup
by tilly (Archbishop) on Dec 13, 2003 at 22:42 UTC
    That wasn't fixed?

    I would have to wonder if that was due to moving from 2.x to the 3.x code base. Because I privately reported that bug in the 2.x line over a year ago. (As you can see from my mention at Re: Reasons for looking at your favourite module's source, I knew about it quite a while ago.)

[reply]
      Aparently it is fixed if you also use the latest version of modperl. I got this from Stas Bekman:
      Yes, but you also need the latest mp2. There was a bug both in mp2 and CGI.pm. Both fixed in the latest versions.

      modperl-2.0/Changes:

      =item 1.99_11 - November 10, 2003

      (...)

      rewrite $r->read() and perlio read functions to use the same function,which completely satisfies the read request if possible, on the way getting rid of get_client_block and its supporting functions which have problems and will most likely will be removed from the httpd-API in the future. Directly manipulate bucket brigades instead. (Stas)

      Unfortunately this doesn't help the case where the system uses precompiled binaries (in my case RedHat 8). So I'll use this patched version of CGI.pm until I can get a better version of modperl installed.

      Michael

[reply]
      There hasn't been a major code base change.   The versions of CGI.pm have simply incremented out of the 2.x range.   The version numbers are contiguous - 3.01, 3.00, 2.99, 2.98, 2.97, etc.   Lot'sa tweaks to track real world problems, but no revolutions.
[reply]

Back to Meditations