[Catalyst-dev] [Catalyst] Major (for me) problem fixed!

[Matt S Trout]

Jonathan Rockway wrote:
> I tracked down the problem, sort of.
> 
> Basically, M::P::O's default "this file is a module to load" regex is:
> 
>      qr/[.]pm$/
> 
> Which allows lots of junk like
> ".#.#\3847*#$#!!foo!!!bar##../../../...pm" which of course cannot
> contain a valid module (only \w+ is allowed in module names; AFAIK).  So
> therefore, I've changed the default regex to:
> 
>      qr{[\/:]\w+[.]pm$};
> 
> Not quite as elegant (!), but it works.  Halfway.
> 
> This allows:
> 
> /foo/bar/../baz/test.pm (foo::baz::test)
> /foo/bar.pm
> bar.pm
> foo/bar.pm
> foo:bar.pm (on weird filesystems)
> 
> But disallows:
> 
> /foo/bar/../baz/.#test.pm
> .#test.pm
> 
> etc.
> 
> To test this yourself, look for:
> 
>     my $file_regex   = $self->{'file_regex'} || qr/\.pm$/;
> 
> near line188 in Module::Pluggable::Object.pm, and replace the qr// part
> with that big regex above.  All unit tests pass, and Catalyst works
> great.  It loads valid modules, and ignores emacs' tempfiles.  Exciting!
> 
> Unfortunately, this isn't the best solution because we don't know how
> far up the directory tree to check for perl semantics.  For example,
> 
> "/cygdrive/c/Documents and Settings/jrockway/Foo/Bar.pm" (yes, Windows
> at work, unfortunately) is valid if the module we're tyring to load is
> "Foo::Bar", but not valid if we're trying to load "Documents and
> Settings::jrockway::Foo::Bar".  What really needs to happen is M::P::O
> needs to open the file, check for a valid package declaration, and
> compare that to the filename.  This is pretty simple and I'm working on
> it right now, but I'd like to hear other ideas.  Opening each file will
> be a slight performance hit, but since it only happens once I'm OK with
> that.

That's a horrible approach, I really don't think it's the answer. It'll fail 
miserably when the package name doesn't match the filename, for a start - 
which is generally inadvisable but perfectly valid.

A better plan seems to be to put together a selection of dumbass filenames as 
provided by version control tools and editors and to exclude those; then we 
can build that into the default config passed to Module::Pluggable (or even 
get it submitted back as a patch to Module::Pluggable itself) and forget about it.