Having an `echo` on the code results in invalid ZIP file that doesn't work on Android

AlbertVilaCalvo commented 2 years ago

Expected Behavior

Not sure if this is a bug or it is expected.

I've been using the library to generate a pkpass for iOS. It works fine on iOS, no issues.

Today I've tested it on Android and it didn't work. I tried 3 apps from Google Play, and I got errors on all them:

Pass is not espass or pkpass format
Could not import your pass. The file is not valid. Some issuers don't support Android.
Data no vàlida - Passi no vàlid (translation: Data not valid - Pass not valid)

I started changing things until I ended up almost like your example.php. It still didn't work. Finally I commented the rest of the code (read env variables, load data etc.), which made it work. This is how I discovered that 2 echo I had there were the culprit.

In addition, I've seen that, on a Mac, if you rename a valid .pkpass to .zip and double-click it, it decompresses correctly. However, if you have an echo, it says that it cannot decompress it because the zip format is not compatible .

imatge

Translation: We could not decompress "pass-php-pkpass-amb-echo.zip". It has an incompatible format.

That said, doing unzip -t pass-php-pkpass-amb-echo.zip gives:

Archive:  pass-php-pkpass-amb-echo.zip
warning [pass-php-pkpass-amb-echo.zip]:  4 extra bytes at beginning or within zipfile
  (attempting to process anyway)
    testing: signature                OK
    testing: manifest.json            OK
    testing: pass.json                OK
    testing: icon.png                 OK
    testing: icon@2x.png              OK
    testing: logo.png                 OK
No errors detected in compressed data of pass-php-pkpass-amb-echo.zip.

Actual Behavior

Not sure, maybe this is expected.

Steps to Reproduce the Problem

Add echo 'hello'; to example.php, after the require('../vendor/autoload.php');.

The resulting .pkpass file doesn't work on Android then, but works fine on iOS.

tschoffelen commented 2 years ago

That is definitely to be expected. The script outputs the pass file, so if you output anything else as well, it gets added to the pass file.

That's how PHP works, and I don't think there is a way to prevent that.

Maybe a good thing to add to the README however for people that aren't aware?

AlbertVilaCalvo commented 2 years ago

I don't do PHP so I'm not familiar with all this. I thought that the pass would contain what you give to setData() and addFile().

I also thought about having some warning to the README so that people are not caught by this, but considering that I'm a newcomer to PHP, I leave it to your criterion.

One question: would it be possible to validate the resulting zip file? Eg in createZip(). I'm trying to open it with different zip apps and they say that is corrupt/damaged. I've found this: https://stackoverflow.com/questions/17499460/php-ziparchive-check-if-zip-file-is-broken-incomplete

tschoffelen commented 2 years ago

I don't think ZIP validation has any added advantage, since we wouldn't be able to catch things like additional output, since that's sent to the browser, not part of the ZIP file our script is generating.

And we're using very standard code to create the ZIP itself, using PHP's official ZIP extension, so it shouldn't be possible to create an archive that isn't valid, as long as you don't output any other content when outputting the ZIP.

tschoffelen / php-pkpass