Fix memory leak and invalid continuation after tar header writing fails

Closes phpGH-20003.

Author: ndossche

NEWS

@@ -34,6 +34,10 @@ PHP                                                                        NEWS
   . Fixed bug #67563 (mysqli compiled with mysqlnd does not take ipv6 adress
     as parameter). (nielsdos)
 
+- Phar:
+  . Fix memory leak and invalid continuation after tar header writing fails.
+    (nielsdos)
+
 - SimpleXML:
   . Fixed bug GH-19988 (zend_string_init with NULL pointer in simplexml (UB)).
     (nielsdos)

ext/phar/tar.c

@@ -1211,7 +1211,16 @@ int phar_tar_flush(phar_archive_data *phar, char *user_stub, zend_long len, int
 	}
 
 	zend_hash_apply_with_argument(&phar->manifest, phar_tar_writeheaders, (void *) &pass);
-	/* TODO: memory leak and incorrect continuation if phar_tar_writeheaders fails? */
+
+	if (error && *error) {
+		if (closeoldfile) {
+			php_stream_close(oldfile);
+		}
+
+		/* on error in the hash iterator above, error is set */
+		php_stream_close(newfile);
+		return EOF;
+	}
 
 	/* add signature for executable tars or tars explicitly set with setSignatureAlgorithm */
 	if (!phar->is_data || phar->sig_flags) {
@@ -1294,12 +1303,6 @@ int phar_tar_flush(phar_archive_data *phar, char *user_stub, zend_long len, int
 		php_stream_close(oldfile);
 	}
 
-	/* on error in the hash iterator above, error is set */
-	if (error && *error) {
-		php_stream_close(newfile);
-		return EOF;
-	}
-
 	if (phar->fp && pass.free_fp) {
 		php_stream_close(phar->fp);
 	}

ext/phar/tests/tar_flush_too_long_filename.phpt

@@ -0,0 +1,41 @@
+--TEST--
+Tar flush with too long file name
+--EXTENSIONS--
+phar
+--SKIPIF--
+<?php
+if (getenv('SKIP_SLOW_TESTS')) die('skip');
+if (function_exists('openssl_sign')) die('skip requires openssl disabled for mocking purposes');
+?>
+--INI--
+phar.require_hash=0
+--FILE--
+<?php
+$fname = __DIR__ . '/' . basename(__FILE__, '.php') . '.tar';
+
+// Mock sign to fail at second invocation, tricks failure in phar_create_signature()
+function openssl_sign() {
+    static $counter = 0;
+    $counter++;
+    if ($counter === 2) {
+        return false;
+    }
+    return true;
+}
+
+$phar = new PharData($fname);
+$phar->addEmptyDir('blah1/');
+$phar->setSignatureAlgorithm(Phar::OPENSSL, "randomcrap");
+try {
+    $phar->addEmptyDir('blah2/' . str_repeat('X', 1000));
+} catch (PharException $e) {
+    echo $e->getMessage();
+}
+
+?>
+--CLEAN--
+<?php
+unlink(__DIR__ . '/' . basename(__FILE__, '.clean.php') . '.tar');
+?>
+--EXPECTF--
+tar-based phar "%s" cannot be created, filename "%s" is too long for tar file format