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