Fw: [PECL-DEV] Re: Phar behaviour - aiming for cohesion

Hi Greg, OK, rearranged thoughts before I crash out. Sling this one around and see what you come up with: .zip or .tar files created with Phar: There is *only* the default mini-stub option. You can't call setStub() or setDefaultStub() on anything but a pure .phar. Filenames passed to the constructor need to be filename.zip or filename.tar. All writable .zip and .tar files can be treated as Phar objects by the extension. Third-party (as in non-Phar-created) archives have a *temporary* .phar dir for the duration of the script, containing the default mini-stub. Aliasing probably isn't allowed. (Questions: Should the temporary stub implant bypass phar.readonly? What restrictions should there be? Should Phar-created .zip and .tar files be treated the exact same way, i.e. have temporary stubs and share the same restrictions?) Conversion from zip or tar to phar: the run-anywhere default stub is added during conversion, as with a new Phar object, and can be altered using setStub()/setDefaultStub(). The filename (automatically) becomes .phar/.phar.bz2/.phar.gz. Conversion from phar to zip or tar: There is NO stub-changing following conversion; it isn't possible to change the stub in anything other than a pure .phar file. The name of the converted beastie is (automatically) .phar.zip or .phar.tar/.phar.tar.bz2/.phar.tar.gz as appropriate. Expected behaviour of .phar.tar/.phar.zip files under PHP CLI or clientside CGI is as for pure .phar files. The appropriate third-party tools work as expected. Browsers try to download rather than run converted files. Manually suffixing the filename with .php allows the file to run on the server, as with a normal .phar. (NB This already works, just the stub-changing restriction and automated file naming aren't in place.) This way everyone knows pretty much what they're dealing with, since anything with .phar in its name has the extended run-anywhere stub by default - and anything else cannot. Does this sound vaguely coherent/sane? More to the point, does it sound vaguely secure/do-able? - Steph ----- Original Message ----- From: "Steph Fox" <steph@zend.com> To: "Gregory Beaver" <greg@chiaraquartet.net> Cc: <pecl-dev@lists.php.net> Sent: Sunday, February 10, 2008 1:45 AM Subject: [PECL-DEV] Re: Phar behaviour

Hi Greg, Darn >'s are playing up again... ==============

4. file.phar.zip (or tar) when Phar is not enabled

currently, this should be a parse error, but it may be useful to detect .zip/.tar and add an error message to core that questions whether this is a phar archive. ============== It's not a parse error in the PHP sense... PHP just sees it as a zip or tar and prints out the (binary) contents accordingly. Cue beep(). Thing is, in a server context it wouldn't even be down to PHP to decipher it; the only place that would happen is under CLI (or CGI used as CLI). E.g. what I'm getting (now I found the source of the zip archive corruption - not your fault after all) is: // with Phar enabled: C:\sandbox\php5\Release_TS>php created.phar.zip C:\sandbox\php5\Release_TS>php converted.phar.zip Hello World C:\sandbox\php5\Release_TS>php thirdpartyzip.phar.zip Warning: Unknown: failed to open stream: phar error: ".phar/stub.php" is not a file in phar "thirdpartyzip.phar.zip" in Unknown on line 0 PK♥♦¶ ,

▲   ∟              index.php│▒/╚(PHM╬╚WP‗H═╔╔¤/╩IQ▓V░À♥ PK☺☻  ¶  ,
▲   ∟                            index.phpPK♣♠    ☺ ☺ 7   E

// without Phar enabled: C:\sandbox\php5\Release_TS>php created.phar.zip

DÐÂ☺        Hello World          ↕ index.phpnu♫ x
DÐÂ☺        I8ÖV¡r<   <   ♫ ↕ .phar/stub.phpnu♫ x

C:\sandbox\php5\Release_TS>php converted.phar.zip Hello World C:\sandbox\php5\Release_TS>php thirdpartyzip.phar.zip PK♥♦¶ ,

▲   ∟              index.php│▒/╚(PHM╬╚WP‗H═╔╔¤/╩IQ▓V░À♥ PK☺☻  ¶  ,
▲   ∟                            index.phpPK♣♠    ☺ ☺ 7   E

==============

5. file.phar.gz when Phar is enabled/disabled 6. file.phar.bz2 when Phar is enabled/disabled

ditto - this should be a parse error. ============== You mean when Phar is enabled, or generally? Remembering that it isn't usually down to PHP to deal with a .bz2, .gz, .tar or .zip... ==============

IMHO Phar should attempt to insert a mini-stub into file.zip or file.tar, the same way it currently does if a third-party ZIP archive is renamed to file.phar.zip. Also IMHO, a file named file.phar.zip or file.phar.tar should work exactly like a standard .phar AND exactly like a standard .zip or .tar, if Phar is enabled. I'm not quite sure what should happen there if Phar isn't installed.

It may be obvious to me, but maybe it is one of my assumptions - without the phar extension, the only file format that can reasonably be expected to work is the phar file format. zip/tar files always require the phar extension in order to work. ============== Yeah I think you failed to spot my deliberate mistake... I set out to talk about third-party zip/tar archives opened with "new Phar('whatever.zip');" and got lost in CLI stuff somewhere along the way - there were actually two separate issues there, the second of which you dealt with later. A third-party zip manually renamed to .phar.zip, when operated on as a Phar object, is given a new mini-stub and treated as an honorary .phar (rightly or wrongly, this is current behaviour). From that point on it acts like a Phar-created .phar.zip file. I think if that's expected behaviour for third-party files named file.phar.zip you might as well allow it for file.zip too, without requiring the potentially confusing renaming step... IF this is a good idea in the first place. If the current behaviour's wrong for files that happen to be named .phar.zip but don't have a .phar stub, there needs to be a stub check when operating on existing files rather than a write. That's easily enough changed too, I think - it's just a question of deciding which behaviour is correct. ==============

This brings me to the next question: What exactly is file.phar.zip? Is it a .phar that was created as a PHAR/ZIP archive, or a .phar that was converted to a ZIP archive, and is there supposed to be any difference? The difference I'm seeing here - assuming the default stub was used - is that if you run a .phar that was converted to a ZIP archive without Phar installed you get something like: C:\sandbox\php5\Release_TS>php myphar.phar.zip

DÐÂ☺        Hello World          ↕ index.phpnu♫ x
DÐÂ☺        ERROR: manifest length read was "320" should be "1713645682"

whereas the same .phar created as a PHAR/ZIP will run under PHP regardless of whether Phar is enabled or not. (Caveat A: I don't know whether either behaviour should be considered buggy. Caveat B: I'm seeing a new-since-last-week bug in phar.zip creation which may be affecting my results.)

Probably the best approach is to use the default stub, and require the user to set the stub. Alternately, we could have an optional parameter for the stub =============== Hmm, my original results were affected by my home-made bug. Current behaviour is that conversions get to keep the existing stub, but phar.zip files created by Phar have the default .zip stub. Ideally these should both behave in the same way by default. If you decide to have that optional parameter in the conversion methods, it should probably be Boolean and mean 'whether to retain the existing stub', with the default behaviour being... which? My instinct would be to make the default the same as for a newly created phar.zip - it's less confusing, it means a .phar.zip is usually just a .zip that can also be operated on as a Phar object if the extension happens to be available. (But see above - if the Phar constructor is changed to accept all writable zips, you might as well lose .phar.zip as a special name for zips with a .phar mini-stub and just call them .zip, keeping .phar.zip for conversions only. In that case they could be *expected* to behave differently, and the default should be to retain the existing stub.) Either way, it's still possible to re-set the stub following conversion using setStub(), so I don't see the need for a complex argument there... That reminds me, I'd still like to see $phar->setDefaultStub('index.php', 'web.php'); as a convenience method rather than have to use Phar::makeDefaultStub('index.php', 'web.php'); in userspace, since it only gets used in one way. Would you accept a patch introducing that? ===============

Last point: Since any renaming at present is wholly left to the user, there's no way to know what a file is by just looking at it. I think the conversion methods should actually change the name of the file appropriately so that third-party tools (and users!) have a fighting chance.

I think this is reasonable, and the name could be an optional argument to the function to allow user overriding of default behavior. =============== Cool :) =============== 2) it may make some sense to allow creation of tar/zip files that are not executable phar archives (do not contain .phar/stub.php or .phar/alias.txt). If they are not executable, the security concern of enabling a virus would disappear, and we could use phar to make tar/zip archives with phar.readonly=1. This would be very useful for folks making on-demand archives on a website. For instance, PEAR's website could use this to make a bundle of many packages for installation all at once. ================ Agreed - but let's get the philosophy sorted out first :) ================ 3) does it make sense to add an extract() method to Phar object? ================ That would be at odds with the whole purpose of 'read-only' and 'require hash', no? Needs thinking about IMHO, it's complex. - Steph Greg -- PECL development discussion Mailing List (http://pecl.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php