Skip to content

Commit 044eeb3

Browse files
Net::SMTP support: fix $+ alternation bug and IO::File::new_tmpfile recursion (#448)
Two fixes that make all 19 libnet (Net::SMTP) test programs pass (110/110 subtests): 1. Fix $+ (LAST_PAREN_MATCH) for regex alternations: lastCaptureString() was returning the highest-numbered capture group regardless of whether it participated in the match. Now iterates backwards to find the first non-null group, matching Perl 5 semantics. This fixes Net::Netrc token parser which uses $+ to extract quoted/unquoted tokens. 2. Fix IO::File::new_tmpfile infinite recursion: The pure-Perl new_tmpfile() called $class->new which causes StackOverflowError when subclasses override new() to call new_tmpfile(). Replaced with bless+gensym to match the XS behavior (no polymorphic dispatch through subclass new). Generated with [Devin](https://cli.devin.ai/docs) Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 parent 38ade03 commit 044eeb3

4 files changed

Lines changed: 188 additions & 3 deletions

File tree

dev/modules/net_smtp.md

Lines changed: 173 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
# Net::SMTP Support for PerlOnJava
2+
3+
## Status: COMPLETE — 19/19 test programs pass (110/110 subtests)
4+
5+
**Branch**: `feature/net-telnet-support`
6+
**Date started**: 2026-04-06
7+
**Module**: libnet 3.15 (Net::SMTP is part of the libnet distribution)
8+
**Test command**: `./jcpan -t Net::SMTP`
9+
10+
## Background
11+
12+
Net::SMTP is part of the libnet CPAN distribution, which provides client-side
13+
networking modules for SMTP, FTP, NNTP, POP3, and related protocols. The module
14+
is pure Perl, relies on IO::Socket::INET for connectivity and Net::Cmd for
15+
command/response handling.
16+
17+
## Test Results Summary
18+
19+
### Current State: 17/19 test programs pass, 2 failures
20+
21+
| Test File | Result | Status |
22+
|-----------|--------|--------|
23+
| t/changes.t | skipped (author) | OK |
24+
| t/config.t | ok | PASS |
25+
| t/critic.t | skipped (author) | OK |
26+
| t/datasend.t | 0/54 | **FAIL** — StackOverflowError |
27+
| t/ftp.t | skipped (no config) | OK |
28+
| t/hostname.t | ok | PASS |
29+
| t/netrc.t | 7/20 | **FAIL**`$+` bug + read-only error |
30+
| t/nntp.t | skipped (no config) | OK |
31+
| t/nntp_ipv6.t | skipped (no fork) | OK |
32+
| t/nntp_ssl.t | skipped (no SSL) | OK |
33+
| t/pod.t | skipped (author) | OK |
34+
| t/pod_coverage.t | skipped (author) | OK |
35+
| t/pop3_ipv6.t | skipped (no fork) | OK |
36+
| t/pop3_ssl.t | skipped (no SSL) | OK |
37+
| t/require.t | ok | PASS |
38+
| t/smtp.t | skipped (no config) | OK |
39+
| t/smtp_ipv6.t | skipped (no fork) | OK |
40+
| t/smtp_ssl.t | skipped (no SSL) | OK |
41+
| t/time.t | ok | PASS |
42+
43+
## Bugs Found
44+
45+
### Bug 1: IO::File::new_tmpfile infinite recursion — FIXED
46+
47+
**Affected tests**: t/datasend.t (0/54 subtests, StackOverflowError)
48+
49+
**Symptom**: `StackOverflowError` with infinite recursion between
50+
`IO::File::new_tmpfile` (line 163) and `Foo::new` (line 32 of datasend.t).
51+
52+
**Root cause**: PerlOnJava's pure-Perl `IO::File::new_tmpfile()` calls
53+
`$class->new` (line 163) to create the filehandle object. In standard Perl 5,
54+
`new_tmpfile` is an XS function in `IO::Handle` that calls C's `tmpfile()`
55+
directly — it never dispatches through Perl-level `new()`. The PerlOnJava
56+
version uses polymorphic dispatch, so when a subclass (like the test's `Foo`)
57+
overrides `new()` to call `new_tmpfile()`, it creates infinite recursion:
58+
59+
```
60+
Foo->new() → Foo->new_tmpfile() → IO::File::new_tmpfile()
61+
→ $class->new() [where $class="Foo"]
62+
→ Foo->new() → ... StackOverflow
63+
```
64+
65+
**Fix**: Replace `$class->new` with `bless gensym(), $class` in `new_tmpfile()`.
66+
This directly creates a blessed glob (same as what `IO::Handle::new` does
67+
internally) without polymorphic method dispatch.
68+
69+
**File**: `src/main/perl/lib/IO/File.pm`, line 163
70+
71+
### Bug 2: `$+` (LAST_PAREN_MATCH) returns wrong group in alternations — FIXED
72+
73+
**Affected tests**: t/netrc.t (7/20 subtests, wrong lookup + read-only crash)
74+
75+
**Symptom**: `Net::Netrc->lookup('foo')` returns undef because the `.netrc`
76+
parser fails to extract quoted tokens. The parser at `Net/Netrc.pm` line 91
77+
uses:
78+
```perl
79+
(my $tok = $+) =~ s/\\(.)/$1/g;
80+
```
81+
where the regex is: `s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//`
82+
83+
For input `"foo"`:
84+
- `$2` = `foo` (content inside quotes, participated in match)
85+
- `$3` = `""` (unquoted alternative, did NOT participate)
86+
- Perl 5: `$+` = `foo` (highest-numbered group that participated)
87+
- PerlOnJava: `$+` = `""` (highest-numbered group, regardless)
88+
89+
**Root cause**: `RuntimeRegex.lastCaptureString()` returns
90+
`lastCaptureGroups[length-1]` — the last array element — without checking if
91+
that group actually participated in the match. Non-participating groups have
92+
`null` values in the array (from Java's `Matcher.group()` returning null).
93+
The fix should iterate backwards and return the first non-null entry.
94+
95+
**Secondary symptom**: After lookup fails, `undef->{password}` at line 103
96+
throws "Modification of a read-only value attempted" because the return
97+
value from `lookup()` is a read-only undef that auto-vivification cannot write to.
98+
99+
**File**: `src/main/java/org/perlonjava/runtime/regex/RuntimeRegex.java`,
100+
method `lastCaptureString()` (line 1190)
101+
102+
## Implementation Plan
103+
104+
### Phase 1: Fix `$+` variable for alternation groups ✓ COMPLETE
105+
106+
1. Fix `RuntimeRegex.lastCaptureString()` to iterate backwards through
107+
`lastCaptureGroups` and return the first non-null value
108+
2. Verify with: `./jperl -e '"test" =~ /(a)|(test)|(c)/; print "$+\n"'`
109+
Should print `test`, not empty string
110+
111+
**File**: `src/main/java/org/perlonjava/runtime/regex/RuntimeRegex.java`
112+
113+
### Phase 2: Fix IO::File::new_tmpfile recursion ✓ COMPLETE
114+
115+
1. Change `my $fh = $class->new;` to `my $fh = bless gensym(), $class;`
116+
2. Verify with datasend.t tests
117+
118+
**File**: `src/main/perl/lib/IO/File.pm`
119+
120+
### Phase 3: Verify and finalize ✓ COMPLETE
121+
122+
1. Rebuild: `make dev`
123+
2. Rerun: `./jcpan -t Net::SMTP` — All 19 test programs pass (110/110 subtests)
124+
3. Run full test suite: `make` — All unit tests pass
125+
4. Commit and create PR
126+
127+
## Test Verification
128+
129+
```bash
130+
# Build
131+
make
132+
133+
# CPAN tests
134+
./jcpan -t Net::SMTP
135+
136+
# Verify $+ fix
137+
./jperl -e '"test" =~ /(a)|(test)|(c)/; print "got: |$+|\n"'
138+
# Expected: got: |test|
139+
140+
# Verify new_tmpfile fix
141+
./jperl -e '
142+
package Foo;
143+
use IO::File;
144+
our @ISA = qw(IO::File);
145+
sub new { my $fh = shift->new_tmpfile; $fh }
146+
my $f = Foo->new;
147+
print defined($f) ? "ok\n" : "not ok\n";
148+
'
149+
# Expected: ok
150+
```
151+
152+
## Progress Tracking
153+
154+
### Current Status: COMPLETE
155+
156+
### Completed Phases
157+
- [x] Phase 1: Fix `$+` for alternation groups (2026-04-06)
158+
- Fixed `lastCaptureString()` to iterate backwards and return first non-null group
159+
- File changed: `RuntimeRegex.java`
160+
- [x] Phase 2: Fix IO::File::new_tmpfile recursion (2026-04-06)
161+
- Replaced `$class->new` with `bless gensym(), $class`
162+
- File changed: `IO/File.pm`
163+
- [x] Phase 3: Verify and finalize (2026-04-06)
164+
- All 19 test programs pass (110/110 subtests)
165+
- All unit tests pass (no regressions)
166+
167+
### Files Modified
168+
- `src/main/java/org/perlonjava/runtime/regex/RuntimeRegex.java``lastCaptureString()` method
169+
- `src/main/perl/lib/IO/File.pm``new_tmpfile()` method
170+
171+
## Related Documents
172+
- `dev/modules/net_telnet.md` — Net::Telnet (same branch, same libnet family)
173+
- `dev/modules/lwp_useragent.md` — LWP uses socket I/O

src/main/java/org/perlonjava/core/Configuration.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public final class Configuration {
3333
* Automatically populated by Gradle/Maven during build.
3434
* DO NOT EDIT MANUALLY - this value is replaced at build time.
3535
*/
36-
public static final String gitCommitId = "92020251e";
36+
public static final String gitCommitId = "38ade0348";
3737

3838
/**
3939
* Git commit date of the build (ISO format: YYYY-MM-DD).

src/main/java/org/perlonjava/runtime/regex/RuntimeRegex.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1191,7 +1191,15 @@ public static String lastCaptureString() {
11911191
if (lastCaptureGroups == null || lastCaptureGroups.length == 0) {
11921192
return null;
11931193
}
1194-
return lastCaptureGroups[lastCaptureGroups.length - 1];
1194+
// $+ returns the highest-numbered capture group that actually participated
1195+
// in the match (i.e., is non-null). Non-participating groups in alternations
1196+
// have null values from Java's Matcher.group().
1197+
for (int i = lastCaptureGroups.length - 1; i >= 0; i--) {
1198+
if (lastCaptureGroups[i] != null) {
1199+
return lastCaptureGroups[i];
1200+
}
1201+
}
1202+
return null;
11951203
}
11961204

11971205
/**

src/main/perl/lib/IO/File.pm

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,11 @@ sub new_tmpfile {
160160
my $class = shift;
161161
@_ == 0 or croak "usage: $class->new_tmpfile()";
162162
require File::Temp;
163-
my $fh = $class->new;
163+
# Use bless+gensym directly instead of $class->new to avoid infinite
164+
# recursion when subclasses override new() to call new_tmpfile().
165+
# In standard Perl 5, new_tmpfile is an XS function in IO::Handle
166+
# that calls C's tmpfile() without dispatching through Perl-level new().
167+
my $fh = bless gensym(), $class;
164168
my ($tmp_fh, $tmp_name) = File::Temp::tempfile(UNLINK => 1);
165169
if (defined $tmp_fh) {
166170
close $tmp_fh;

0 commit comments

Comments
 (0)