Skip Menu |

This queue is for tickets about the Net-SFTP-Foreign CPAN distribution.

Report information
The Basics
Id: 61751
Status: resolved
Worked: 3.5 hours (210 min)
Priority: 0/
Queue: Net-SFTP-Foreign

People
Owner: salva [...] cpan.org
Requestors: bsi [...] hosember.hu
Cc:
AdminCc:

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



Subject: Conflict with Log::WarnDie
Hi, While using Log::WarnDie (Log-WarnDie-0.05) with Net::SFTP::Foreign (Net- SFTP-Foreign-1.60), the backend cannot connect to the SSH client process via IPC due to the strong *STDERR tie applied before globally by Log::WarnDie. The solution is very simple, see the attached diff which I've made against the latest 1.60 release. You have to untie *STDERR before trying to use IPC's open2() or open3(). Please apply this patch to the trunk. Regards, Barnabas (BSi) Bona bsi@hosember.hu
Subject: log_warndie_bug.diff
diff -u -r a/lib/Net/SFTP/Foreign/Backend/Unix.pm b/lib/Net/SFTP/Foreign/Backend/Unix.pm --- a/lib/Net/SFTP/Foreign/Backend/Unix.pm 2010-09-29 09:17:22.004245800 +0200 +++ b/lib/Net/SFTP/Foreign/Backend/Unix.pm 2010-09-29 13:39:17.517121800 +0200 @@ -61,11 +61,11 @@ return undef; } local ($@, $SIG{__DIE__}, $SIG{__WARN__}); - return eval { open3(@_[1,0], ">&SSHERR", @_[3..$#_]) } + return eval { untie *STDERR; open3(@_[1,0], ">&SSHERR", @_[3..$#_]) } } else { local ($@, $SIG{__DIE__}, $SIG{__WARN__}); - return eval { open2(@_[0,1], @_[3..$#_]) }; + return eval { untie *STDERR; open2(@_[0,1], @_[3..$#_]) }; } }
untie'ing STDERR will seldomly work and may introduce hard to find bugs in people code using the module. Alos, later versions of the module accept the stderr_fh option on the constructor call allowing to pass a custom file handle to be used as the slave SSH stderr.
From: bsi [...] hosember.hu
On Wed Oct 06 08:44:37 2010, SALVA wrote: Show quoted text
> untie'ing STDERR will seldomly work and may introduce hard to find
bugs Show quoted text
> in people code using the module. > > Alos, later versions of the module accept the stderr_fh option on the > constructor call allowing to pass a custom file handle to be used as
the Show quoted text
> slave SSH stderr.
Indeed it may lead to rare bugs, but in the current form the package cannot be used with Log::WarnDie at all and neither stderr_discard nor stderr_fh helps in this. I admit that untie is could be in some rare circumstances an overkill for this problem but at least it's a workaround and it works. Without it I cannot use Log::WarnDie with Net::SFTP::Foreign. What about creating a new constructor parameter (untie_stderr => 1 or something) to enable this hack until someone find a longstanding solution to this problem?
Subject: Re: [rt.cpan.org #61751] Conflict with Log::WarnDie
Date: Wed, 6 Oct 2010 07:43:55 -0700 (PDT)
To: bug-Net-SFTP-Foreign [...] rt.cpan.org
From: Salvador Fandino <sfandino [...] yahoo.com>
Show quoted text
----- Original Message ----
> From: Barnabas Bona via RT <bug-Net-SFTP-Foreign@rt.cpan.org> > Sent: Wed, October 6, 2010 3:52:46 PM > Subject: [rt.cpan.org #61751] Conflict with Log::WarnDie > > Queue: Net-SFTP-Foreign > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=61751 > > > On Wed Oct 06 08:44:37 2010, SALVA wrote:
> > untie'ing STDERR will seldomly work and may introduce hard to find
> bugs
> > in people code using the module. > > > > Alos, later versions of the module accept the stderr_fh option on the > > constructor call allowing to pass a custom file handle to be used as
> the
> > slave SSH stderr.
> > Indeed it may lead to rare bugs, but in the current form the package > cannot be used with Log::WarnDie at all and neither stderr_discard nor > stderr_fh helps in this. I admit that untie is could be in some rare > circumstances an overkill for this problem but at least it's a > workaround and it works. Without it I cannot use Log::WarnDie with > Net::SFTP::Foreign. What about creating a new constructor parameter > (untie_stderr => 1 or something) to enable this hack until someone find > a longstanding solution to this problem?
The real problem is Log::WarnDie not implementing the FILENO method As a workaround just define it inside your program: sub Log::WarnDie::FILENO { -1 } ... also, you should report this problem to Log::WarnDie author so that it could be used with IPC::Open3 - Salva
From: bsi [...] hosember.hu
On Wed Oct 06 10:44:06 2010, sfandino@yahoo.com wrote: Show quoted text
> [...] > The real problem is Log::WarnDie not implementing the FILENO method > > As a workaround just define it inside your program: > > sub Log::WarnDie::FILENO { -1 } > > ... also, you should report this problem to Log::WarnDie author so > that it could > be used with IPC::Open3 > > - Salva >
Sure, I'll report it to Log::WarnDie author too, but unfortunately the FILENO redefinition dosen't help either. I've prepared a test script (sftp_test.pl) to easily reproduce this problem. Also saved three different debug outputs, one with the unmodified components (sftp_test_output_unmodified.txt), one with the proposed FILENO definition (sftp_test_output_with_fileno.txt) and one with the untie workaround (sftp_test_output_with_untie.txt). You can download them from http://bit.ly/bvtOjj Sorry for always reopening this bug but I rely heavily on your great SFTP module and I want to use it vanilla, without every time patching on a new release.
Sorry for the late reply, I just forgot about this bug report Your test script seems tp work for me when the work-around is in place, in what way does it fail for you? what do you get and what were you expecting? BTW, a better work around would be to use sub Log::WarnDie::FILENO { 2 } - Salva
From: bsi [...] hosember.hu
On Wed Nov 10 12:29:06 2010, SALVA wrote: Show quoted text
> Sorry for the late reply, I just forgot about this bug report > > Your test script seems tp work for me when the work-around is in place, > in what way does it fail for you? what do you get and what were you > expecting? > > BTW, a better work around would be to use > > sub Log::WarnDie::FILENO { 2 } > > - Salva
I'm using ActivePerl v5.10.1 for MSWin32-x86-multi-thread (Binary build 1007 [291969]) on Windows 7 x64b but the problem can be reproduced on other Windows versions too. I've tried also the modified workaround with no success. You can see the expected output and the incorrect outputs in the uploaded zip file: http://bit.ly/bvtOjj (also attached) Expected output (using my untie workaround): sftp_test_output_with_untie.txt Incorrect output with the plain install (unmodified components): sftp_test_output_unmodified.txt Incorrect output using your workaround (FILENO workaround): sftp_test_output_with_fileno.txt Regards, Barna
Subject: sftp_test.zip
Download sftp_test.zip
application/zip 1.6k

Message body not shown because it is not plain text.

Your test script was failing in Windows because of yet another bug, this time inside IPC::Open3. I have included a workaround for it in version 1.63_04 (currently replicating through the CPAN). Note that you will have to still include the sub Log::WarnDie::FILENO { 2 } workaround in your script.
From: bsi [...] hosember.hu
Thanks for the workaround, I could confirm it works well on Windows.