Fix ctl_path length boundary - #63
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an off-by-one boundary in ctl_path validation so Unix-domain control socket paths always leave room for the required terminating NUL in sun_path, improving early validation and error reporting.
Changes:
- Reject
ctl_pathvalues whose length is>= sizeof(sun_path)(i.e., enforce a usable limit ofsizeof(sun_path) - 1). - Update the validation error message to report the usable maximum length.
- Add a regression test covering the boundary (
limitaccepted,limit + 1rejected) and include it inMANIFEST.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| lib/Net/OpenSSH.pm | Tightens ctl_path length validation to enforce sizeof(sun_path) - 1 usable limit and updates the reported maximum. |
| t/ctl-path.t | Adds a boundary regression test for accepted/rejected ctl_path lengths. |
| MANIFEST | Includes the new t/ctl-path.t test file in the distribution manifest. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| my $ssh = Net::OpenSSH->new(host => 'localhost', ctl_path => $good, | ||
| connect => 0, strict_mode => 0); |
There was a problem hiding this comment.
Fixed in commit 404b1e3 by passing an explicit ssh_version in this constructor call. Re-ran perl -Ilib t/ctl-path.t.
| $ssh = Net::OpenSSH->new(host => 'localhost', ctl_path => $bad, | ||
| connect => 0, strict_mode => 0); |
There was a problem hiding this comment.
Fixed in commit 404b1e3 by passing an explicit ssh_version in this negative-path constructor call too. Re-ran perl -Ilib t/ctl-path.t.
Summary
Reject control socket paths that fill
sun_pathwithout room for the required terminating NUL byte.Changes
sizeof(sun_path) - 1as the maximum usable path length.MANIFEST.Fixes #44.
Testing
perl -Ilib -c lib/Net/OpenSSH.pmperl -Ilib t/ctl-path.t