Fix #69279: Compressed ZIP Phar extractTo() creates garbage files

[cmb69]

When extracting compressed files from an uncompressed Phar, we must not
use the direct file pointer, but rather get an uncompressed file
pointer.

This also fixes #79912, which appears to be a duplicate of #69279.

[afilina]

I read both bug reports. I only have one small comment change suggestion, as it seemed like a forgotten copy-paste artifact. Otherwise, LGTM.

[cmb69]

@bishopb, what do you think about this?

[nikic]

To check my understanding of how these APIs work: phar_get_efp() can return either a compressed or uncompressed FP. Calling phar_open_entry_fp() will convert it into an uncompressed FP. Is that right?

[cmb69]

phar_get_efp() can return either a compressed or uncompressed FP. Calling phar_open_entry_fp() will convert it into an uncompressed FP. Is that right?

Yes.

[nikic]

Heh, that seems like ... not so great API design. Can we run into the reverse issue afterwards, where some code expects phar_get_efp() to return the compressed FP but will now get the uncompressed one?

[cmb69]

To verify this, I created a simple ZIP with a deflated entry bar.txt followed by a stored entry foo.txt. Extracting produced a correctly uncompressed bar.txt, but a garbage foo.txt. Took me a while to find out the foo.txt was extracted from a wrong file offset. The offset would also be wrong without this patch. I think I need to look into this issue first.

[cmb69]

The offset would also be wrong without this patch. I think I need to look into this issue first.

I found that is due to different extra field lengths in the local file header and the central directory header for (some?) ZIPs created by 7z or Info-ZIP's zip (which may be valid or not according to the specification). The offset of the file data in the archive is determined by the extra field length of the central directory header, so may not actually be correct. I don't know why the lengths differ, but unless someone files a bug report about that, I won't investigate further.

phar_get_efp() can return either a compressed or uncompressed FP. Calling phar_open_entry_fp() will convert it into an uncompressed FP. Is that right?

Not quite; I was wrong, previouly. Actually, at least for ZIPs Phar starts with an PHAR_FP (which handles uncompressed data). Calling phar_open_entry_fp() will create an additional PHAR_UFP (which uncompresses), if the entry has to be uncompressed. For further entries, phar_get_efp() returns the PHAR_FP again, and phar_open_entry_fp() will then return the already opened PHAR_UFP (if compression needs to be done).

I've added a test which extracts a deflated, a stored and a deflated entry.

[nikic]

Thanks for the thorough investigation, this makes more sense to me now.