Skip to content

fix: make binmode the default on Win32 to prevent newline mangling#192

Merged
toddr merged 3 commits intocpan-authors:mainfrom
Koan-Bot:koan.atoomic/fix-issue-116
Mar 22, 2026
Merged

fix: make binmode the default on Win32 to prevent newline mangling#192
toddr merged 3 commits intocpan-authors:mainfrom
Koan-Bot:koan.atoomic/fix-issue-116

Conversation

@Koan-Bot
Copy link
Contributor

@Koan-Bot Koan-Bot commented Mar 18, 2026

Summary

On Windows, IPC::Run was silently translating newlines (\n\r\n) by default when piping data to/from child processes. This corrupts binary data (PDFs, images, etc.) and surprises callers who expect raw bytes to pass through unchanged. This PR makes binmode the default, so newline translation is opt-in rather than opt-out.

Fixes #116

Changes

  • Set $binmode = 1 as the default for both < and > redirection in harness() (lib/IPC/Run.pm)
  • Update t/binmode.t to expect \r\n to be preserved by default; binary(0) now represents the old translating behavior
  • Remove local $TODO from t/win32_newlines.t — the tests should now pass on Win32

Notes

  • binary() / binary(1) behave the same as before (binmode on)
  • binary(0) can be used to explicitly re-enable the old newline-translation behavior for callers that need it
  • binmode has no effect on non-Win32 platforms, so this change is Windows-only in practice

Test plan

  • All 763 existing tests pass (prove -Ilib t/ — all green)
  • t/win32_newlines.t is now expected to pass on Win32 (was previously marked local $TODO)

Generated by Kōan /fix


Quality Report

Changes: 3 files changed, 8 insertions(+), 6 deletions(-)

Code scan: clean

Tests: passed (OK)

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

@toddr
Copy link
Member

toddr commented Mar 19, 2026

@Koan-Bot rebase

1 similar comment
@toddr
Copy link
Member

toddr commented Mar 21, 2026

@Koan-Bot rebase

@Koan-Bot Koan-Bot force-pushed the koan.atoomic/fix-issue-116 branch from a8ffe9f to 6ef2138 Compare March 21, 2026 09:19
Koan-Bot added a commit to Koan-Bot/IPC-Run that referenced this pull request Mar 21, 2026
@Koan-Bot
Copy link
Contributor Author

Rebase: fix: make binmode the default on Win32 to prevent newline mangling

Branch koan.atoomic/fix-issue-116 rebased onto main and force-pushed.

Diff: 3 files changed, 8 insertions(+), 6 deletions(-)

Review feedback was analyzed and applied.

Actions

  • Rebased koan.atoomic/fix-issue-116 onto upstream/main
  • Force-pushed koan.atoomic/fix-issue-116 to origin
  • CI failed (attempt 1)
  • Applied CI fix (attempt 1)
  • Pushed CI fix (attempt 1)
  • CI failed (attempt 2)
  • CI still failing after 2 fix attempts

CI

CI still failing after 2 fix attempts.

Last failure logs
... (truncated)
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx

Automated by Kōan

@toddr
Copy link
Member

toddr commented Mar 22, 2026

@Koan-Bot rebase

Koan-Bot added a commit to Koan-Bot/IPC-Run that referenced this pull request Mar 22, 2026
@Koan-Bot Koan-Bot force-pushed the koan.atoomic/fix-issue-116 branch from 21fd876 to 13afd54 Compare March 22, 2026 02:49
@toddr toddr marked this pull request as ready for review March 22, 2026 02:53
@Koan-Bot
Copy link
Contributor Author

Rebase: fix: make binmode the default on Win32 to prevent newline mangling

Branch koan.atoomic/fix-issue-116 rebased onto main and force-pushed.

Diff: 5 files changed, 18 insertions(+), 8 deletions(-)

Review feedback was analyzed and applied.

Actions

  • Rebased koan.atoomic/fix-issue-116 onto upstream/main
  • Force-pushed koan.atoomic/fix-issue-116 to origin
  • CI failed (attempt 1)
  • CI still failing after 2 fix attempts

CI

CI still failing after 2 fix attempts.

Last failure logs
... (truncated)
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx

Automated by Kōan

@toddr
Copy link
Member

toddr commented Mar 22, 2026

@Koan-Bot rebase windows is still failing

Koan-Bot and others added 3 commits March 21, 2026 21:38
On Windows, IPC::Run was silently translating newlines (\n <-> \r\n) by
default when piping data to/from child processes. This corrupts binary
data (PDFs, images, etc.) and is surprising behavior for any caller that
expects raw data to pass through unchanged.

Change the default $binmode from 0 to 1 for both input ('<') and output
('>')  redirection operators in harness(). Users who need the old
newline-translating behavior can opt back in with binary(0).

Update t/binmode.t to reflect the new defaults:
- "no binary" now preserves \r\n (binmode=1 default)
- binary(0) now represents explicit opt-in to translation (old behavior)
- Remove Win32-specific expected-value adjustments for the default case

Remove local $TODO from t/win32_newlines.t — the tests are now expected
to pass on Win32.

Fixes cpan-authors#116

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- **t/close_stdin.t**: Added `$nl` variable (`\r\n` on Win32, `\n` elsewhere) and used it in 3 string comparisons at lines 68, 135, 172 — these tests now account for binmode=1 preserving `\r\n` from child processes on Windows
- **t/pump.t**: Added `$nl` variable and updated 3 string comparisons at lines 60, 68, 85; also adjusted the `/hello\n/g` regex to `/hello\r?\n/g` and the `pos()` check to use dynamic length — matches the binmode=1 behavior on Windows
- **t/run.t**: Added `$out =~ s/\r\n/\n/g if IPC::Run::Win32_MODE()` normalization before 3 assertions at lines 525, 543, and 947 — consistent with the existing normalization pattern already used in `eok()` and at line 1039
- **t/win32_newlines.t**: Removed `%todo_tests` hash and `local $TODO` marker — tests 3, 8, 9 now pass on Win32 with binmode=1 default, so they should no longer be marked TODO

All changes fix Windows CI failures caused by binmode=1 default making `\r\n` pass through unchanged where tests previously expected `\n`.
@Koan-Bot Koan-Bot force-pushed the koan.atoomic/fix-issue-116 branch from 13afd54 to 46ffbbb Compare March 22, 2026 03:40
@Koan-Bot
Copy link
Contributor Author

Rebase: fix: make binmode the default on Win32 to prevent newline mangling

Branch koan.atoomic/fix-issue-116 rebased onto main and force-pushed.

Diff: 8 files changed, 35 insertions(+), 20 deletions(-)

Review feedback was analyzed and applied.

Changes

  • t/close_stdin.t: Added $nl variable (\r\n on Win32, \n elsewhere) and used it in 3 string comparisons at lines 68, 135, 172 — these tests now account for binmode=1 preserving \r\n from child processes on Windows
  • t/pump.t: Added $nl variable and updated 3 string comparisons at lines 60, 68, 85; also adjusted the /hello\n/g regex to /hello\r?\n/g and the pos() check to use dynamic length — matches the binmode=1 behavior on Windows
  • t/run.t: Added $out =~ s/\r\n/\n/g if IPC::Run::Win32_MODE() normalization before 3 assertions at lines 525, 543, and 947 — consistent with the existing normalization pattern already used in eok() and at line 1039
  • t/win32_newlines.t: Removed %todo_tests hash and local $TODO marker — tests 3, 8, 9 now pass on Win32 with binmode=1 default, so they should no longer be marked TODO

All changes fix Windows CI failures caused by binmode=1 default making \r\n pass through unchanged where tests previously expected \n.

Actions

  • Rebased koan.atoomic/fix-issue-116 onto upstream/main
  • Applied review feedback
  • Force-pushed koan.atoomic/fix-issue-116 to origin
  • CI passed

CI

CI passed.


Automated by Kōan

@toddr toddr merged commit febdbdb into cpan-authors:main Mar 22, 2026
25 checks passed
toddr-bot added a commit to toddr-bot/IPC-Run that referenced this pull request Mar 22, 2026
The TODO tests from PR cpan-authors#216 (tests 3, 8, 9) were already resolved by
PR cpan-authors#192 which made binmode the default on Win32. The TODO markers were
removed in that PR but left behind a stale $test_num variable and a
historical comment that are no longer needed. Remove both.

Closes cpan-authors#223

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

binmode is a necessity for win32 usability and must be default

2 participants