Bug #95479 for Courriel: Uses File::Slurp, known to be buggy and vulnerable

Thu May 08 18:24:34 2014 ether [...] cpan.org - Ticket created

Subject:

Uses File::Slurp, known to be buggy and vulnerable

e.g. look at https://rt.cpan.org/Ticket/Display.html?id=83126 and be dismayed Path::Slurp::Tiny and Path::Tiny are both excellent alternatives.

Sun May 11 17:22:12 2014 DROLSKY [...] cpan.org - Correspondence added

Given the way Courriel uses File::Slurp, I don't think those bugs are an issue. I don't pass any encoding params in and I just want bytes anyway.

Sun May 11 17:22:12 2014 The RT System itself - Status changed from 'new' to 'open'

Sun May 11 17:22:13 2014 DROLSKY [...] cpan.org - Status changed from 'open' to 'rejected'

Mon May 12 10:23:28 2014 rt-cpan [...] trout.me.uk - Correspondence added

On Sun May 11 17:22:12 2014, DROLSKY wrote: Show quoted text

> Given the way Courriel uses File::Slurp, I don't think those bugs are > an issue. I don't pass any encoding params in and I just want bytes > anyway.

Then it should be a trivial fix to move to a better library, thereby encouraging people who respect your code to do so, and meaning that you can never accidentally change the Courriel code in such a way as to trigger the bug. Writing a comment saying "never change how we use File::Slurp here because it's shitty and dangerous" would also be acceptable, although that honestly seems like more effort than just fixing it. Either way, this is still an outstanding issue from my POV

Mon May 12 10:23:29 2014 rt-cpan [...] trout.me.uk - Status changed from 'rejected' to 'open'

Mon May 12 11:09:32 2014 DROLSKY [...] cpan.org - Correspondence added

On Mon May 12 10:23:28 2014, MSTROUT wrote: Show quoted text

> On Sun May 11 17:22:12 2014, DROLSKY wrote:

> > Given the way Courriel uses File::Slurp, I don't think those bugs are > > an issue. I don't pass any encoding params in and I just want bytes > > anyway.

> > Then it should be a trivial fix to move to a better library, thereby > encouraging people who respect your code to do so, and meaning that > you can never accidentally change the Courriel code in such a way as > to trigger the bug. > > Writing a comment saying "never change how we use File::Slurp here > because it's shitty and dangerous" would also be acceptable, although > that honestly seems like more effort than just fixing it. > > Either way, this is still an outstanding issue from my POV

It looks like File::Slurp::Tiny doesn't have the same issue. What do you think of that one?

Mon May 12 11:10:06 2014 DROLSKY [...] cpan.org - Correspondence added

On Mon May 12 11:09:32 2014, DROLSKY wrote: Show quoted text

> On Mon May 12 10:23:28 2014, MSTROUT wrote:

> > On Sun May 11 17:22:12 2014, DROLSKY wrote:

> > > Given the way Courriel uses File::Slurp, I don't think those bugs > > > are > > > an issue. I don't pass any encoding params in and I just want bytes > > > anyway.

> > > > Then it should be a trivial fix to move to a better library, thereby > > encouraging people who respect your code to do so, and meaning that > > you can never accidentally change the Courriel code in such a way as > > to trigger the bug. > > > > Writing a comment saying "never change how we use File::Slurp here > > because it's shitty and dangerous" would also be acceptable, although > > that honestly seems like more effort than just fixing it. > > > > Either way, this is still an outstanding issue from my POV

> > It looks like File::Slurp::Tiny doesn't have the same issue. What do > you think of that one?

Also, I have comaint on File::Slurp. Is there any reason it's unfixable? Couldn't I just make it work exactly like File::Slurp::Tiny?

Mon May 12 12:19:12 2014 ether [...] cpan.org - Correspondence added

CC:	matthewt [...] shadowcat.co.uk
Subject:	Re: [rt.cpan.org #95479] Uses File::Slurp, known to be buggy and vulnerable
Date:	Mon, 12 May 2014 09:18:58 -0700
To:	Dave Rolsky via RT <bug-Courriel [...] rt.cpan.org>
From:	Karen Etheridge <ether [...] cpan.org>

On Mon, May 12, 2014 at 11:10:06AM -0400, Dave Rolsky via RT wrote: Show quoted text

> Also, I have comaint on File::Slurp. Is there any reason it's unfixable? Couldn't I just make it work exactly like File::Slurp::Tiny?

Fixing File::Slurp would be excellent if possible, although its API is not the greatest (an out of band error rather than simply dying, etc). Leon Timmermans has been trying to salvage the good bits of the API into File::Slurp::Tiny to serve as a drop-in replacement, but it doesn't seem that it's quite there yet (it may never get there; see above). Alternatively, and this would be great regardless, a re-release with a big fat "this is deprecated and shouldn't be used anymore" notice would be very nice indeed.

Mon May 12 12:31:33 2014 rt-cpan [...] trout.me.uk - Correspondence added

On Mon May 12 11:10:06 2014, DROLSKY wrote: Show quoted text

> Also, I have comaint on File::Slurp. Is there any reason it's > unfixable? Couldn't I just make it work exactly like > File::Slurp::Tiny?

As we discussed on IRC - 15:15 <@mst> the point here is "this module was old, badly-written, I'm glad uri adopted it and kept it going for a few more years, but there are now proper replacements and it's time to thank him for maintaining somebody else's piece of shit code and let it die" 15:23 <@mst> autarch: basically, as with e.g. Class::Load -> Module::Runtime, we're doing a clean sweep trying to get most of CPAN to upgrade to the new thing maintained by cabal people

Mon May 12 13:31:20 2014 ether [...] cpan.org - Correspondence added

On 2014-05-08 15:24:34, ETHER wrote: Show quoted text

> e.g. look at https://rt.cpan.org/Ticket/Display.html?id=83126 and be > dismayed > > Path::Slurp::Tiny and Path::Tiny are both excellent alternatives.

oops s/Path::Slurp::Tiny/File::Slurp::Tiny/ sorry

Sun May 18 11:37:20 2014 DROLSKY [...] cpan.org - Status changed from 'open' to 'resolved'

Sun May 18 11:37:22 2014 DROLSKY [...] cpan.org - Fixed in 0.35 added