diff --git a/.github/workflows/author-testing.yml b/.github/workflows/author-testing.yml index ef18681..a826301 100644 --- a/.github/workflows/author-testing.yml +++ b/.github/workflows/author-testing.yml @@ -3,7 +3,7 @@ name: author-testing on: push: branches: - - "*" + - "main" tags-ignore: - "*" pull_request: @@ -17,26 +17,16 @@ jobs: AUTHOR_TESTING: 1 AUTOMATED_TESTING: 1 RELEASE_TESTING: 1 - PERL_CARTON_PATH: $GITHUB_WORKSPACE/local - - strategy: - fail-fast: false - matrix: - perl-version: - - "5.30" container: - image: perldocker/perl-tester:${{ matrix.perl-version }} + image: perldocker/perl-tester:5.42 steps: - - uses: actions/checkout@v1 - - name: perl -V - run: perl -V + - uses: actions/checkout@v6 + - run: perl -V - name: Install Author Dependencies run: dzil authordeps | cpm install -g --show-build-log-on-failure - - name: Install Dependencies - run: dzil listdeps | cpanm - # cannot use cpm due to https://github.com/skaji/cpm/issues/161 - #run: dzil listdeps | cpm install -g --show-build-log-on-failure - + run: dzil listdeps | cpm install -g --show-build-log-on-failure - - name: dzil test run: dzil test diff --git a/.github/workflows/testsuite.yml b/.github/workflows/testsuite.yml index 9b4e054..8af84cc 100644 --- a/.github/workflows/testsuite.yml +++ b/.github/workflows/testsuite.yml @@ -3,7 +3,7 @@ name: linux on: push: branches: - - "*" + - "main" tags-ignore: - "*" pull_request: @@ -21,7 +21,7 @@ jobs: PERL_CARTON_PATH: $GITHUB_WORKSPACE/local steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - run: perl -V - name: Install Dependencies uses: perl-actions/install-with-cpm@v1 @@ -66,7 +66,7 @@ jobs: container: perldocker/perl-tester:${{ matrix.perl-version }} steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v6 - run: perl -V - name: Install Dependencies run: cpm install -g --show-build-log-on-failure diff --git a/.gitignore b/.gitignore index d2e49e2..b22983e 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,4 @@ Makefile.old MANIFEST.bak Overload-FileCheck-* .build +local/ diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..7a81b91 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,52 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Project Overview + +Overload::FileCheck is a Perl XS module that hooks into Perl's OP dispatch mechanism to mock file check operators (-e, -f, -d, -s, etc.) and stat/lstat at the interpreter level. Designed for unit testing scenarios where you need to simulate filesystem conditions without real files. + +## Build & Test Commands + +```bash +# Build (compiles XS/C code) +perl Makefile.PL && make + +# Run full test suite +make test + +# Run a single test +prove -lv t/02_basic-mock.t + +# Author testing via Dist::Zilla +dzil test + +# Build a release +dzil build +``` + +Makefile.PL is auto-generated from dist.ini — edit dist.ini for build configuration, not Makefile.PL directly. + +## Architecture + +### Two-layer design + +**Perl layer** (`lib/Overload/FileCheck.pm`): Public API — `mock_file_check`, `mock_stat`, `mock_lstat`, `mock_all_file_checks`, `mock_all_from_stat`, and their `unmock_*` counterparts. Manages the mapping from operator names (e.g., '-e') to Perl OP types (e.g., `OP_FTIS`). Provides export groups `:check` (return value constants), `:stat` (stat index constants and helpers like `stat_as_file()`), and `:all`. + +**XS layer** (`FileCheck.xs` + `FileCheck.h`): Replaces Perl's default `pp_*` OP handlers with custom ones that call back into Perl. Three handler types: `pp_overload_ft_yes_no` (boolean ops like -e, -f), `pp_overload_ft_int`/`pp_overload_ft_nv` (numeric ops like -s, -M), and `pp_overload_stat` (stat/lstat). `FileCheck.h` contains compatibility macros for Perl 5.14 vs 5.15+ internal API differences. + +### Return value protocol + +Mock callbacks return: `CHECK_IS_TRUE` (1), `CHECK_IS_FALSE` (0), or `FALLBACK_TO_REAL_OP` (-1) for boolean ops. Numeric ops (-s, -M, -C, -A) return the actual value. Stat mocks return an arrayref of 13 elements (or empty arrayref for "file not found"). + +### Test structure + +Tests in `t/` use Test2 framework. Many test files for individual operators (test-e.t, test-f.t, etc.) are symlinks to template files (`test-true-false.t` for boolean ops, `test-integer.t` for numeric ops) — the test determines which operator to exercise based on its own filename. + +## Key Conventions + +- Minimum Perl version: **5.010** (enforced in dist.ini and CI) +- CI tests across Perl 5.10 through latest dev release +- Operators accepted with or without dash: `'-e'` and `'e'` are equivalent +- Code style: PerlTidy with 2-space indent (see `tidyall.ini`), PerlCritic severity 3 +- Distribution managed with **Dist::Zilla** (`dist.ini`); `[@Git]` plugin handles version tagging and push diff --git a/Changes b/Changes index 5e0ecf4..1dd1beb 100644 --- a/Changes +++ b/Changes @@ -2,6 +2,11 @@ Revision history for Overload-FileCheck {{$NEXT}} +0.014 2026-02-22 10:19:05-07:00 America/Denver + +- fix: prevent filehandle reference leak in $_last_call_for +- fix: spelling typos + 0.013 2022-02-23 08:36:12-07:00 America/Denver - Fix a PL_statcache bug when checking: -l $f || -e _ diff --git a/Makefile.PL b/Makefile.PL index c8afb4d..85dd29f 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -1,4 +1,4 @@ -# This file was automatically generated by Dist::Zilla::Plugin::MakeMaker v6.031. +# This file was automatically generated by Dist::Zilla::Plugin::MakeMaker v6.033. use strict; use warnings; diff --git a/README.md b/README.md index 376e62a..6ce7056 100644 --- a/README.md +++ b/README.md @@ -20,9 +20,6 @@ By using mock\_all\_file\_checks you can set a hook function to reply any -X che use strict; use warnings; -use strict; -use warnings; - use Test::More; use Overload::FileCheck q{:all}; @@ -755,7 +752,7 @@ Nicolas R # COPYRIGHT AND LICENSE -This software is copyright (c) 2022 by cPanel, L.L.C. +This software is copyright (c) 2026 by cPanel, L.L.C. This is free software; you can redistribute it and/or modify it under the same terms as the Perl 5 programming language system itself. diff --git a/dist.ini b/dist.ini index 57380a6..3354452 100644 --- a/dist.ini +++ b/dist.ini @@ -2,7 +2,7 @@ name = Overload-FileCheck author = Nicolas R license = Perl_5 copyright_holder = cPanel, L.L.C. -copyright_year = 2022 +copyright_year = 2026 ;[PPPort] @@ -80,7 +80,9 @@ run = git status --porcelain | grep 'M Makefile.PL' && git commit -m 'Makefile.P [GatherDir] exclude_filename = Makefile.PL +exclude_filename = CLAUDE.md ;exclude_filename = ppport.h +exclude_match = ^local/ ; -- static meta-information [MetaResources] diff --git a/examples/synopsis.pl b/examples/synopsis.pl index 77f0f84..a014720 100644 --- a/examples/synopsis.pl +++ b/examples/synopsis.pl @@ -3,9 +3,6 @@ use strict; use warnings; -use strict; -use warnings; - use Test::More; use Overload::FileCheck q{:all}; diff --git a/lib/Overload/FileCheck.pm b/lib/Overload/FileCheck.pm index 51bf9d8..654e4ba 100644 --- a/lib/Overload/FileCheck.pm +++ b/lib/Overload/FileCheck.pm @@ -584,7 +584,10 @@ sub _check { $file = $_last_call_for if !defined $file && defined $_last_call_for && !defined $_current_mocks->{ $MAP_FC_OP{'stat'} }; my ( $out, @extra ) = $_current_mocks->{$optype}->($file); - $_last_call_for = $file; + # Only cache string filenames, not filehandle references. + # Storing a ref here prevents the filehandle from being garbage collected, + # causing resource leaks (e.g. sockets staying open). See GH #179. + $_last_call_for = ref($file) ? undef : $file; # FIXME return undef when not defined out @@ -757,11 +760,11 @@ sub _stat_for { ); foreach my $k ( keys %$opts ) { - $k = lc($k); - $k =~ s{^st_}{}; - next unless defined $name2ix{$k}; + my $name = lc($k); + $name =~ s{^st_}{}; + next unless defined $name2ix{$name}; - $stat[ $name2ix{$k} ] = $opts->{$k}; + $stat[ $name2ix{$name} ] = $opts->{$k}; } return \@stat; diff --git a/t/fh-ref-leak.t b/t/fh-ref-leak.t new file mode 100644 index 0000000..98e5e58 --- /dev/null +++ b/t/fh-ref-leak.t @@ -0,0 +1,66 @@ +#!/usr/bin/perl + +# Test that file check operators do not retain references to filehandles +# passed as arguments. This prevents garbage collection of the filehandle, +# which can cause resource leaks (e.g. sockets staying open). +# +# See: https://github.com/cpanel/Test-MockFile/issues/179 + +use strict; +use warnings; + +use Test2::Bundle::Extended; +use Test2::Tools::Explain; + +use Scalar::Util qw(weaken); +use Overload::FileCheck -from_stat => \&my_stat, qw{:check}; + +sub my_stat { + my ( $stat_or_lstat, $f ) = @_; + return FALLBACK_TO_REAL_OP(); +} + +# Test that filehandle references are not retained by $_last_call_for +{ + my $weak_ref; + + { + open my $fh, '<', '/dev/null' or die "Cannot open /dev/null: $!"; + $weak_ref = $fh; + weaken($weak_ref); + + ok( defined $weak_ref, "weak ref is defined before scope exit" ); + + # Trigger a file check on the filehandle — this used to store $fh + # in $_last_call_for, preventing garbage collection. + no warnings; + -f $fh; + } + + ok( !defined $weak_ref, "filehandle is garbage collected after -f check (no ref leak)" ); +} + +# Test with -S (the operator from the original bug report) +{ + my $weak_ref; + + { + open my $fh, '<', '/dev/null' or die "Cannot open /dev/null: $!"; + $weak_ref = $fh; + weaken($weak_ref); + + no warnings; + -S $fh; + } + + ok( !defined $weak_ref, "filehandle is garbage collected after -S check (no ref leak)" ); +} + +# Test that string filenames still work for _ caching (no regression) +{ + no warnings; + ok( -f $0, "-f \$0 works" ); + ok( -e _, "-e _ works after -f on string filename" ); +} + +done_testing; diff --git a/t/stat-helpers.t b/t/stat-helpers.t index b38f660..9e657fa 100644 --- a/t/stat-helpers.t +++ b/t/stat-helpers.t @@ -100,4 +100,21 @@ is stat_as_file( atime => 8, mtime => 9, ctime => 10 ), $expect, 'atime + mtime is stat_as_file( perms => 0755 ), [ 0, 0, S_IFREG | 0755, (0) x 10 ], 'stat_as_file with perms 0755'; +# keys with st_ prefix and mixed case should work +$expect = [@regular_file]; +$expect->[7] = 42; +is stat_as_file( st_size => 42 ), $expect, 'st_size prefix accepted'; + +$expect = [@regular_file]; +$expect->[7] = 99; +is stat_as_file( SIZE => 99 ), $expect, 'uppercase SIZE accepted'; + +$expect = [@regular_file]; +$expect->[7] = 55; +is stat_as_file( St_Size => 55 ), $expect, 'mixed case St_Size accepted'; + +$expect = [@regular_file]; +$expect->[9] = 12345; +is stat_as_file( ST_MTIME => 12345 ), $expect, 'ST_MTIME accepted'; + done_testing;