Skip Menu |
 

This queue is for tickets about the Net-IMAP-Simple CPAN distribution.

Report information
The Basics
Id: 122707
Status: open
Priority: 0/
Queue: Net-IMAP-Simple

People
Owner: Nobody in particular
Requestors: tlhackque [...] yahoo.com
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: (no value)



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.
Download (untitled) / with headers
text/plain 2.6k
I'm aware of several bugs with get() that I haven't gotten around to fixing. I'm in the "works for me" mindset as I'm quite busy at work. If you'd like to attempt a patch or two, shoot me a PR on github. https://github.com/jettero/net--imap--simple Otherwise, I'll try to get to it when I can. On Fri Aug 04 09:00:54 2017, tlhackque wrote: Show quoted text
> 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.
-- If riding in an airplane is flying, then riding in a boat is swimming. 116 jumps, 48.6 minutes of freefall, 92.9 freefall miles.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.