Thanks kcott. This is all very helpful.

I shall have a good look at the colour points later today. No, I am not using warnings. I'm a relatively new convert to use strict; as you will see from Backdating strict posted just 2 months ago. As always, that was good advice from the Monastery!

• There are variables used that are (presumably) defined elsewhere (I noted %file and %data). This means that other code might also be using, and possibly modifying, them: there may be side-effects. Aim to pass needed data to subroutines as arguments and keep a tight rein on what's used and where.
• As far as I can tell, $file{'joolzimage', 'file'} is a "Multi-dimensional array emulation". This is ancient code, used in Perl4, before references became available in Perl5. Aim to use "real" multidimensional arrays; e.g. $file{joolzimage}{file}.

Back in the 1990's I sort of learnt Perl by looking at other people's code which was in a CGI environment. I get the feeling the code I was looking at wasn't very well written to start with and I was trying to get things to work using that as a guide and no formal background in programming or computer science. I look back occasionally at code from those days and wonder how I ever got it working and amazement that some of it is still working today!

The code that generates %data and %file sits in a require file on all my websites except this latest one. Here I have taken the advice of the Monastery and not only used Template, but also extracted the require *.pl; into a use *; module instead.

package Site::Common;

use strict;

use DBI;
use DBD::mysql;
use Exporter;
use Template;
use Site::Variables;

our @ISA = qw(Exporter);
our @EXPORT = qw(%data %file $template);

my %cookie = ($ENV{'HTTP_COOKIE'}.';') =~ /(\S+)=(\S+);/g;

# Some subs removed...

our (%data, %file);
if ($ENV{'GATEWAY_INTERFACE'}) {
    my $query_string;
    $query_string = $ENV{'QUERY_STRING'} if $ENV{'QUERY_STRING'};
    if ($ENV{'REQUEST_METHOD'} eq 'POST') {
        if ($ENV{'CONTENT_TYPE'} =~ /^multipart\/form-data/i) {
            my ($key, $name);
            my $boundary = <STDIN>;
            chomp $boundary;
            binmode STDIN;
            local $/;
            my @parts = split /$boundary?/, <STDIN>;
            foreach my $p(@parts) {
                if ( $p =~ /^\s*Content-Disposition: +form-data; +name
+=\"(\w+)?\"(; +filename=\"(.+)?\")?\r\n/i ) {
                    $key  = $1;
                    $name = $3;
                    if ( $p =~ /Content-Type: +(\w+\/\w+)?\r\n/i ) {
                        $file{$key, 'type'} = $1;
                        $file{$key, 'name'} = $name;
                        (undef, $file{$key, 'file'}) = split /\r\n\r\n
+/, $p, 2;
                        $data{$key} = 'FILE';
                    } else {
                        (undef, $data{$key}) = split /\r\n\r\n/, $p, 2
+;
                        $data{$key} =~ s/(\r|\n)+$//g;
                    }
                }
            }
        } else {
            $query_string .= '&' if $query_string;
            $query_string .= <STDIN>;
        }
    }
    if ($query_string) {
        my @pairs = split /&/, $query_string;
        foreach my $p(@pairs) {
            $p =~ tr/+/ /;
            $p =~ s/%([a-fA-F0-9][a-fA-F0-9])/pack("C", hex($1))/eg;
            my ($key, $val) = split /=/, $p, 2;
            $data{$key} = $val;
        }
    }
    chomp %data;
}


1;
[download]

This is code I wrote a long time ago to deal with form uploads from websites. It followed much frustration with CGI. Essentially it puts all the upload parameters as key/value pairs into %data and any file uploads into %file with the first dimension of the hash being the web form's input name and the second being either 'file' for the actual binary file content or 'name' for the filename.

So, no - this doesn't get changed by other code.

With the knowledge I have gained in the past couple of months from the Monastery, I plan to refactor this code but in a way that it still exports those two variables so it doesn't break all the existing web scripts. At the same time I will add better methods to access the same data. Being extracted to a module should pave the way for it to be refactored.

There is a lot of what can only be called legacy code on my webserver...refactoring it without breaking anything is going to be a challenge!

Edit - corrected typo...thanks AnomalousMonk for pointing it out