Skip to content

fix(filters): scope source filters per compilation unit#604

Closed
fglock wants to merge 1 commit intomasterfrom
fix/source-filter-leak-require
Closed

fix(filters): scope source filters per compilation unit#604
fglock wants to merge 1 commit intomasterfrom
fix/source-filter-leak-require

Conversation

@fglock
Copy link
Copy Markdown
Owner

@fglock fglock commented Apr 29, 2026

Summary

Source filters installed via Filter::Util::Call::filter_add lived on a thread-local global stack and the wasFilterInstalled flag was never reset between files. As a result, a filter installed while compiling file A leaked into any file A require'd.

This broke jcpan -t URI::git:

  • Test/Base.pm does use Spiffy -Base;, which calls Spiffy::spiffy_filter to install a regex filter that prepends my $self = shift; to every sub.
  • Spiffy's import then calls Exporter::export(...), which lazily requires Exporter::Heavy.
  • While Heavy.pm was being parsed, the Spiffy filter was still active and got applied to it, mangling heavy_export and producing the spurious "my" variable $self masks earlier declaration warning at Exporter/Heavy.pm line 237.
  • With heavy_export broken, field was never installed in Test::Base, and the next line field _filters => [qw(norm trim)]; parsed as two consecutive barewords and died with syntax error near "=> [qw".

Fix

Treat the source-filter stack and the "installed during use" flag as part of the per-file compilation context:

  • FilterUtilCall.java: add FilterStateSnapshot, saveAndResetFilterState(), restoreFilterState().
  • ModuleOperators.java: snapshot and clear the filter state before compiling a require/do'd file and restore it in finally, so filters installed by the caller cannot leak into the required file (and vice versa).

Test plan

  • jcpan -t URI::git passes all 3 subtests (was: syntax error in Test::Base)
  • make (full unit-test suite) green

Generated with Devin

Source filters installed via Filter::Util::Call::filter_add lived on a
thread-local global stack and the "wasFilterInstalled" flag was never
reset between files. As a result, a filter installed by file A leaked
into any file A required.

Concretely, `jcpan -t URI::git` failed because:

  use Spiffy -Base;        # in Test/Base.pm

calls Spiffy::spiffy_filter (which installs a regex filter that prepends
"my $self = shift;" to every sub), then Spiffy's import calls
Exporter::export(...), which lazily requires Exporter::Heavy. While
Heavy.pm was being parsed, the Spiffy filter was still active and got
applied to Heavy.pm's source, mangling heavy_export and producing the
spurious 'my variable $self masks earlier declaration' warning at
Exporter/Heavy.pm line 237. With heavy_export broken, `field` was never
installed in Test::Base, so the next line

  field _filters => [qw(norm trim)];

parsed as two consecutive barewords and died with "syntax error near
=> [qw".

Fix: treat the source-filter stack and the "installed during use" flag
as part of the per-file compilation context. ModuleOperators now snapshots
and resets that state before compiling a required/do'd file and restores
it in finally, so filters installed by the caller cannot leak into the
required file (and vice versa).

Test plan:
- `jcpan -t URI::git` now passes all 3 subtests
- `make` (full unit-test suite) is green

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@fglock fglock closed this Apr 29, 2026
@fglock
Copy link
Copy Markdown
Owner Author

fglock commented Apr 29, 2026

Subsumed by #597 — unified into a single PR.

#597 adopts this PR's cleaner design (wire-up at ModuleOperators.do_file rather than PerlLanguageProvider.executePerlCode, and the FilterStateSnapshot / saveAndResetFilterState / restoreFilterState naming). Earlier I had wired into executePerlCode and needed a skipSaveRestore carve-out for preprocessWithBeginFilters to avoid regressing perl5_t/t/op/incfilter.t from 143/153 to 14/153. This PR's placement at do_file sits outside executePerlCode and so is naturally untouched by the recursive executePerlCode call inside preprocessWithBeginFilters — no carve-out needed. Cleaner.

Verified after refactor:

  • ./jperl src/test/resources/unit/source_filter_scope.t 4/4 (with fix; 1/4 with fix neutralised — bug correctly caught)
  • perl5_t/t/op/incfilter.t 148/153 (no regression from master baseline)
  • make and ./gradlew test (CI's task) green
  • make test-bundled-modules: all 34 YAML tests pass

#597 also includes:

  • regression test src/test/resources/unit/source_filter_scope.t
  • bundling of real YAML.pm 1.31 into src/main/perl/lib/ (so the YAML test suite runs against real YAML.pm semantics rather than YAML::PP)
  • 34 upstream YAML-1.31 tests added under src/test/resources/module/YAML/t/
  • two related fixes in do CODEREF / do FILEHANDLE: STORE no longer called on user's tied $_, and __FILE__ now resolves to the stringified GLOB/CODE
  • design doc updates in dev/design/source_filters.md

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.

1 participant