| Subject: | getfh() issues |
N:I:S v 1.2209
getfh() and the methods that it calls have several issues and need attention.
1) if new_tmpfile fails, no error is returned.
my $file = IO::File->new_tmpfile;
should be
my $file = IO::File->new_tmpfile or return;
The call can fail if the file can't be created; e.g. out of inodes; disk full; permission issue; etc.
2) 'final' does a seek, but does not check for errors.
final => sub { seek $file, 0, 0; $file },
should be
final => sub { seek $file, 0, SEEK_SET or return; $file },
It is unlikely that seek will fail, but it has been known to happen - e.g. if the file gets inadvertently closed or the file handle corrupted.
Also, although SEEK_SET is documented 0, it's better to use the constant (use Fcntl qw/:seek/).
3) memory use
getfh() executes a FETCH; _process_cmd calls _read_multiline, which buffers the entire message in an array. Only after the entire message is buffered does _process_cmd call the process method - once for each line.
This means that a large message - e.g. one with large attachments - has to allocate memory for the entire message, which can be 10s of MB. (People e-mail photos, large documents...)
The code should be re-worked so that the temporary file is written as the data comes in.
Since the temporary file is seekable, if necessary, the end handling can be done by recording the start of the last few lines (with tell) in an array & truncating the temporary file as needed. But that's probably not necessary.
4) The print to the temporary file should check for errors and cause getfh to return an error if one occurs. print can fail due to a hardware error - or a simple disk full.
5) read_multiline can _seterrstr - but doesn't signal _process_cmd that an error has occurred, so the error isn't passed up to the caller.
6) getfh - in fact all the commands - break if $/ isn't \n. This can easily happen in the caller; e.g. if it slurps a file into memory for put. In this case, the getline() calls on the socket all hang. The easiest fix is to add
local $/ = "\n";
at the entry of all public methods.