-
Notifications
You must be signed in to change notification settings - Fork 14
Description
[This is a duplicate of https://rt.cpan.org/Ticket/Display.html?id=93056, cause it looks like that queue isn't the official one for this module]
Executive Summary:
Simply setting binmode on the child's filehandles will not properly enable binmode in Win32, due to how IPC::Run implements things. I don't think that a version of IPC::Run which would transparently support setting binmode on filehandles is likely, so properly supporting binary data streams in System::Command will require an explicit binmode parameter.
The hideous details:
Because of the way that pipes are handled on Win32 via IPC::Run, setting binmode on the child's filehandles does not actually result in binary mode.
For example, the following code
use System::Command;
my $buf = pack('N',13);
binmode( \*STDOUT );
$cmd = System::Command->new( 'od', '-c' );
binmode $cmd->stdin;
binmode $cmd->stdout;
$cmd->stdin->print( $buf );
$cmd->stdin->close;
print $cmd->stdout->getline;
when run with binmode properly on results in
0000000 \0 \0 \0 \r
Note the "\r". When this is run on Win32, it results in
0000000 \0 \0 \0
Note the missing "\r".
I know almost nothing about Win32, but I think the following analysis is correct.
System::Command uses IPC::Run on Win32. IPC::Run uses a helper script (IPC::Run::Win32Pump) to shuttle data between processes (apparently using temporary files and copying data from one file to another). IPC::Run::Win32Pump has the following code [1]
110 $buf =~ s/\r//g unless $binmode;
This strips out all carriage returns regardless if they are actually followed by a new line character. In the case of my binary data, that removes the \r, corrupting the stream. I've confirmed that this line is the culprit by commenting out that line and verifying that the test program above works as expected.[1]
The "$binmode" parameter on that line is essentially set by specifying the IPC::Run binary filter. For example, in System::Command, this
61 # start the command
62 if (MSWin32) {
63 $pid = IPC::Run::start(
64 [@cmd],
65 '<pipe' => $in,
66 '>pipe' => $out,
67 '2>pipe' => $err,
68 );
69 }
becomes this:
61 # start the command
62 if (MSWin32) {
63 $pid = IPC::Run::start(
64 [@cmd],
65 '<pipe' => IPC::Run::binary(), $in,
66 '>pipe' => IPC::Run::binary(), $out,
67 '2>pipe' => IPC::Run::binary(), $err,
68 );
69 }
and the test script works as expected.
Comments to the effect of Perl's binmode function not seeming to work on Win32 appear several times in IPC::Run, but at least under 5.18.2, they do seem to work:
perl -E 'say pack("N", 13)' | od -c
0000000 \0 \0 \0 \0 \r \r \n
perl -E 'binmode \*STDOUT; say pack("N", 13)' | od -c
0000000 \0 \0 \0 \0 \r \n
I really don't know if it's possible that IPC::Run could be fixed so that binmode() on the child's filehandles will work, but there's not much activity going on in that codebase, so I think that a solution to get System::Command working on Win32 needs to be done in System::Command.
Unfortunately, I think an explicit binmode option may need to be provided by System::Command so that it can be appropriately applied for Windows and other systems.
Thanks,
Diab
[1]
Somewhat more sophisticated code is found elsewhere in IPC::Run::Win32IO
206 if ( $self->binmode ) {
207 $data_ref = $self->{SOURCE};
208 }
209 else {
210 my $data = ${$self->{SOURCE}}; # Ugh, a copy.
211 $data =~ s/(?<!\r)\n/\r\n/g;
212 $data_ref = \$data;
213 }
[...]
272 $s =~ s/\r\n/\n/g unless $self->binmode;
The code base is complex, so I'm not sure if code to manually fix the
crlf is actually needed.