- Rewrite/Finish metadata handling

7 files changed, 193 insertions, 59 deletions

diff --git a/ext/phar/TODO b/ext/phar/TODO
index 3f1570d389..a772894544 100644
--- a/ext/phar/TODO
+++ b/ext/phar/TODO
@@ -4,9 +4,8 @@ Version 1.0.0
  X implement ini handler for phar.readonly and phar.require_hash that allows 
    enabling it on PHP_INI_ALL if it is disabled in the system, but does not 
    allow disabling it if it is enabled in the system [Greg]
- X implement reading in metadata in manifest as [type32][len16][metadata...] 
-   where 0 type is used to finish metadata for this file [Greg]
- * implement writing out of metadata to new manifest
+ X implement reading in metadata in manifest as <len32><metadata...> [Marcus]
+ X implement writing out of metadata to new manifest [Marcus]
  X if SPL is disabled, enable only static methods of class Phar and disable 
    class PharFileInfo completely [Marcus]
  * implement in-phar locking, so that a file that is opened for reading can't 
@@ -20,8 +19,8 @@ Version 1.0.0
    PharFileInfo class
  * add uncompressAllFiles(), compressAllFilesGZ() and compressAllFilesBZ2() 
    to Phar class
- * add setMetaData($key, $contents) to PharFileInfo
- * add getMetaData($key = null) to PharFileInfo
+ X add PharFileInfo::setMetaData($metadata) [Marcus]
+ X add PharFileInfo::getMetaData() [Marcus]
  * always throw exceptions from the Phar object, and E_RECOVERABLE_ERROR from 
    streams interface
  * ability to have Phar object return file class as offsetGet() result
diff --git a/ext/phar/phar.c b/ext/phar/phar.c
index cb79ab4c85..9a337fd099 100644
--- a/ext/phar/phar.c
+++ b/ext/phar/phar.c
@@ -380,45 +380,31 @@ static int phar_open_loaded(char *fname, int fname_len, char *alias, int alias_l
  * Parse out metadata from the manifest for a single file
  *
  * Meta-data is in this format:
- * [type32][len16][data...]
+ * [len32][data...]
  * 
- * where type32 is a 32-bit type indicator, len16 is a 16-bit length indicator,
- * and data is the actual meta-data.
+ * data is the serialized zval
  */
-static int phar_parse_metadata(php_stream *fp, char **buffer, char *endbuffer, zval *metadata TSRMLS_DC) /* {{{ */
+static int phar_parse_metadata(php_stream *fp, char **buffer, char *endbuffer, zval **metadata TSRMLS_DC) /* {{{ */
 {
-	zval *dataarray, *found;
-	php_uint32 datatype;
-	php_uint16 len;
-	char *data;
-
-	do {
-		/* for each meta-data, add an array index to the metadata array
-		   if the index already exists, convert to a sub-array */
-		PHAR_GET_32(*buffer, datatype);
-		PHAR_GET_16(*buffer, len);
-		data = (char *) emalloc(len+1);
-		if (endbuffer - *buffer < len) {
-			efree(data);
+	const unsigned char *p;
+	php_uint32 buf_len;
+	php_unserialize_data_t var_hash;
+
+	PHAR_GET_32(*buffer, buf_len);
+	
+	if (buf_len) {
+		ALLOC_INIT_ZVAL(*metadata);
+		p = (const unsigned char*) *buffer;
+		PHP_VAR_UNSERIALIZE_INIT(var_hash);
+		if (!php_var_unserialize(metadata, &p, p + buf_len,  &var_hash TSRMLS_CC)) {
+			PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
+			zval_ptr_dtor(metadata);
+			*metadata = NULL;
 			return FAILURE;
-		} else {
-			memcpy(data, *buffer, len);
-			data[len] = '\0';
-		}
-		if (SUCCESS == zend_hash_index_find(HASH_OF(metadata), datatype, (void**)&found)) {
-			if (Z_TYPE_P(found) == IS_ARRAY) {
-				dataarray = found;
-			} else {
-				MAKE_STD_ZVAL(dataarray);
-				array_init(dataarray);
-				add_next_index_zval(dataarray, found);
-			}
-		} else {
-			dataarray = metadata;
 		}
-		add_index_stringl(dataarray, datatype, data, len, 0);
-	} while (*(php_uint32 *) *buffer && *buffer < endbuffer);
-	*buffer += 4;
+		PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
+	}
+	*buffer += buf_len;
 	return SUCCESS;
 }
 /* }}}*/
@@ -708,9 +694,9 @@ int phar_open_file(php_stream *fp, char *fname, int fname_len, char *alias, int
 		PHAR_GET_32(buffer, entry.crc32);
 		PHAR_GET_32(buffer, entry.flags);
 		if (*(php_uint32 *) buffer) {
-			MAKE_STD_ZVAL(entry.metadata);
-			array_init(entry.metadata);
-			phar_parse_metadata(fp, &buffer, endbuffer, entry.metadata TSRMLS_CC);
+			if (phar_parse_metadata(fp, &buffer, endbuffer, &entry.metadata TSRMLS_CC) == FAILURE) {
+				MAPPHAR_FAIL("unable to read metadata in .phar file \"%s\"");
+			}
 		} else {
 			buffer += 4;
 		}
@@ -1564,6 +1550,8 @@ int phar_flush(phar_entry_data *data TSRMLS_DC) /* {{{ */
 	php_uint32 newcrc32;
 	php_stream *file, *oldfile, *newfile, *compfile;
 	php_stream_filter *filter;
+	php_serialize_data_t metadata_hash;
+	smart_str metadata_str = {0};
 
 	if (PHAR_G(readonly)) {
 		return EOF;
@@ -1747,17 +1735,25 @@ int phar_flush(phar_entry_data *data TSRMLS_DC) /* {{{ */
 			4: compressed filesize
 			4: crc32
 			4: flags
-			4+: metadata TODO: copy the actual metadata, 0 for now
+			4: metadata-len
+			+: metadata
 		*/
+		metadata_str.c = 0;
+		if (entry->metadata) {
+			PHP_VAR_SERIALIZE_INIT(metadata_hash);
+			php_var_serialize(&metadata_str, &entry->metadata, &metadata_hash TSRMLS_CC);
+			PHP_VAR_SERIALIZE_DESTROY(metadata_hash);
+		}
 		copy = time(NULL);
 		phar_set_32(entry_buffer, entry->uncompressed_filesize);
 		phar_set_32(entry_buffer+4, copy);
 		phar_set_32(entry_buffer+8, entry->compressed_filesize);
 		phar_set_32(entry_buffer+12, entry->crc32);
 		phar_set_32(entry_buffer+16, entry->flags);
-		copy = 0;
-		phar_set_32(entry_buffer+20, 0);
-		if (sizeof(entry_buffer) != php_stream_write(newfile, entry_buffer, sizeof(entry_buffer))) {
+		phar_set_32(entry_buffer+20, metadata_str.len);
+		if (sizeof(entry_buffer) != php_stream_write(newfile, entry_buffer, sizeof(entry_buffer))
+		|| metadata_str.len != php_stream_write(newfile, metadata_str.c, sizeof(metadata_str.len))) {
+			smart_str_free(&metadata_str);
 			if (oldfile) {
 				php_stream_close(oldfile);
 			}
@@ -1765,6 +1761,7 @@ int phar_flush(phar_entry_data *data TSRMLS_DC) /* {{{ */
 			php_error_docref(NULL TSRMLS_CC, E_RECOVERABLE_ERROR, "unable to write temporary manifest of file \"%s\" to manifest of new phar \"%s\"", entry->filename, data->phar->fname);
 			return EOF;
 		}
+		smart_str_free(&metadata_str);
 	}
 	
 	/* now copy the actual file data to the new phar */
diff --git a/ext/phar/phar_internal.h b/ext/phar/phar_internal.h
index cbe6b23e14..028a723e71 100755
--- a/ext/phar/phar_internal.h
+++ b/ext/phar/phar_internal.h
@@ -39,6 +39,8 @@
 #include "ext/standard/crc32.h"
 #include "ext/standard/md5.h"
 #include "ext/standard/sha1.h"
+#include "ext/standard/php_var.h"
+#include "ext/standard/php_smart_str.h"
 #if HAVE_SPL
 #include "ext/spl/spl_array.h"
 #include "ext/spl/spl_directory.h"
diff --git a/ext/phar/phar_object.c b/ext/phar/phar_object.c
index 7e4fd88a00..b816b70bea 100755
--- a/ext/phar/phar_object.c
+++ b/ext/phar/phar_object.c
@@ -522,6 +522,41 @@ PHP_METHOD(PharFileInfo, getPharFlags)
 }
 /* }}} */
 
+/* {{{ proto int PharFileInfo::getMetaData()
+ * Returns the metadata of the entry
+ */
+PHP_METHOD(PharFileInfo, getMetadata)
+{
+	PHAR_ENTRY_OBJECT();
+
+	if (entry_obj->ent.entry->metadata) {
+		RETURN_ZVAL(entry_obj->ent.entry->metadata, 1, 0);
+	}
+}
+/* }}} */
+
+/* {{{ proto int PharFileInfo::setMetaData(mixed $metadata)
+ * Returns the metadata of the entry
+ */
+PHP_METHOD(PharFileInfo, setMetadata)
+{
+	zval *metadata;
+	PHAR_ENTRY_OBJECT();
+
+	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "z", &metadata) == FAILURE) {
+		return;
+	}
+
+	if (entry_obj->ent.entry->metadata) {
+		zval_ptr_dtor(&entry_obj->ent.entry->metadata);
+		entry_obj->ent.entry->metadata = NULL;
+	}
+
+	MAKE_STD_ZVAL(entry_obj->ent.entry->metadata);
+	ZVAL_ZVAL(entry_obj->ent.entry->metadata, metadata, 1, 0);
+}
+/* }}} */
+
 #endif /* HAVE_SPL */
 
 /* {{{ phar methods */
@@ -587,6 +622,11 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_entry___construct, 0, 0, 1)
 	ZEND_ARG_INFO(0, flags)
 ZEND_END_ARG_INFO();
 
+static
+ZEND_BEGIN_ARG_INFO_EX(arginfo_entry_setMetadata, 0, 0, 1)
+	ZEND_ARG_INFO(0, metadata)
+ZEND_END_ARG_INFO();
+
 zend_function_entry php_entry_methods[] = {
 	PHP_ME(PharFileInfo, __construct,        arginfo_entry___construct,  0)
 	PHP_ME(PharFileInfo, getCompressedSize,  NULL,                       0)
@@ -596,6 +636,8 @@ zend_function_entry php_entry_methods[] = {
 	PHP_ME(PharFileInfo, getCRC32,           NULL,                       0)
 	PHP_ME(PharFileInfo, isCRCChecked,       NULL,                       0)
 	PHP_ME(PharFileInfo, getPharFlags,       NULL,                       0)
+	PHP_ME(PharFileInfo, getMetadata,        NULL,                       0)
+	PHP_ME(PharFileInfo, setMetadata,        arginfo_entry_setMetadata,  0)
 	{NULL, NULL, NULL}
 };
 #endif
diff --git a/ext/phar/tests/metadata_read.phpt b/ext/phar/tests/metadata_read.phpt
index 195ec28c95..83c96cf9e1 100644
--- a/ext/phar/tests/metadata_read.phpt
+++ b/ext/phar/tests/metadata_read.phpt
@@ -1,8 +1,7 @@
 --TEST--
-Phar with meta-data (Read)
+Phar with meta-data (read)
 --SKIPIF--
-<?php if (!extension_loaded("phar")) print "skip";
-if (!extension_loaded("zlib")) print "skip zlib not present"; ?>
+<?php if (!extension_loaded("phar")) print "skip";?>
 --INI--
 phar.require_hash=0
 --FILE--
@@ -12,13 +11,51 @@ $pname = 'phar://' . $fname;
 $file = "<?php __HALT_COMPILER(); ?>";
 
 $files = array();
-$files['c'] = array('cont' => '*', 'meta' => array(1, 'hi there'));
+$files['a'] = array('cont' => 'a');
+$files['b'] = array('cont' => 'b', 'meta' => 'hi there');
+$files['c'] = array('cont' => 'c', 'meta' => array('hi', 'there'));
+$files['d'] = array('cont' => 'd', 'meta' => array('hi'=>'there','foo'=>'bar'));
 include 'phar_test.inc';
 
-var_dump(file_get_contents($pname.'/c'));
+foreach($files as $name => $cont) {
+	var_dump(file_get_contents($pname.'/'.$name));
+}
 
+$phar = new Phar($fname);
+foreach($files as $name => $cont) {
+	var_dump($phar[$name]->getMetadata());
+}
+
+unset($phar);
+
+foreach($files as $name => $cont) {
+	var_dump(file_get_contents($pname.'/'.$name));
+}
 ?>
+===DONE===
 --CLEAN--
 <?php unlink(dirname(__FILE__) . '/' . basename(__FILE__, '.clean.php') . '.phar.php'); ?>
 --EXPECT--
-string(1) "*"
+string(1) "a"
+string(1) "b"
+string(1) "c"
+string(1) "d"
+NULL
+string(8) "hi there"
+array(2) {
+  [0]=>
+  string(2) "hi"
+  [1]=>
+  string(5) "there"
+}
+array(2) {
+  ["hi"]=>
+  string(5) "there"
+  ["foo"]=>
+  string(3) "bar"
+}
+string(1) "a"
+string(1) "b"
+string(1) "c"
+string(1) "d"
+===DONE===
diff --git a/ext/phar/tests/metadata_write.phpt b/ext/phar/tests/metadata_write.phpt
new file mode 100755
index 0000000000..e9be873b2c
--- /dev/null
+++ b/ext/phar/tests/metadata_write.phpt
@@ -0,0 +1,61 @@
+--TEST--
+Phar with meta-data (write)
+--SKIPIF--
+<?php if (!extension_loaded("phar")) print "skip";?>
+--INI--
+phar.require_hash=0
+--FILE--
+<?php
+$fname = dirname(__FILE__) . '/' . basename(__FILE__, '.php') . '.phar.php';
+$pname = 'phar://' . $fname;
+$file = "<?php __HALT_COMPILER(); ?>";
+
+$files = array();
+$files['a'] = array('cont' => 'a');
+$files['b'] = array('cont' => 'b', 'meta' => 'hi there');
+$files['c'] = array('cont' => 'c', 'meta' => array('hi', 'there'));
+$files['d'] = array('cont' => 'd', 'meta' => array('hi'=>'there','foo'=>'bar'));
+include 'phar_test.inc';
+
+foreach($files as $name => $cont) {
+	var_dump(file_get_contents($pname.'/'.$name));
+}
+
+$phar = new Phar($fname);
+$phar['a']->setMetadata(42);
+$phar['b']->setMetadata(NULL);
+$phar['c']->setMetadata(array(25, 'foo'=>'bar'));
+$phar['d']->setMetadata(true);
+
+foreach($files as $name => $cont) {
+	var_dump($phar[$name]->getMetadata());
+}
+
+unset($phar);
+
+foreach($files as $name => $cont) {
+	var_dump(file_get_contents($pname.'/'.$name));
+}
+?>
+===DONE===
+--CLEAN--
+<?php unlink(dirname(__FILE__) . '/' . basename(__FILE__, '.clean.php') . '.phar.php'); ?>
+--EXPECT--
+string(1) "a"
+string(1) "b"
+string(1) "c"
+string(1) "d"
+int(42)
+NULL
+array(2) {
+  [0]=>
+  int(25)
+  ["foo"]=>
+  string(3) "bar"
+}
+bool(true)
+string(1) "a"
+string(1) "b"
+string(1) "c"
+string(1) "d"
+===DONE===
diff --git a/ext/phar/tests/phar_test.inc b/ext/phar/tests/phar_test.inc
index 82987cb19d..b419812744 100755
--- a/ext/phar/tests/phar_test.inc
+++ b/ext/phar/tests/phar_test.inc
@@ -14,7 +14,7 @@ foreach($files as $name => $cont)
 	$time = isset($ftime) ? $ftime : @mktime(12, 0, 0, 3, 1, 2006);
 	$flags= 0;
 	$perm = 0x000001B6;
-	$meta = array();
+	$meta = NULL;
 
 	// overwrite if array
 	if (is_array($cont))
@@ -33,15 +33,11 @@ foreach($files as $name => $cont)
 	if (empty($ulen)) $ulen = strlen($cont);
 	if (empty($clen)) $clen = strlen($comp);
 	if (empty($crc32))$crc32= crc32($cont);
+	if (isset($meta)) $meta = serialize($meta);
 
 	// write manifest entry
 	$manifest .= pack('V', strlen($name)) . $name;
-	$manifest .= pack('VVVVV', $ulen, $time, $clen, $crc32, $flags|$perm);
-	foreach($meta as $type => $data)
-	{
-		$manifest .= pack('Vv', $type, strlen($data)) . $data;
-	}
-	$manifest .= pack('V', 0);
+	$manifest .= pack('VVVVVV', $ulen, $time, $clen, $crc32, $flags|$perm, strlen($meta)) . $meta;
 
 	// globals
 	$gflags |= $flags;