Phar->buildFromIterator: use spl_iterator_apply(), add 4 tests (mem leaks not yet...

diff --git a/ext/phar/phar_object.c b/ext/phar/phar_object.c
index 3dc62a731921dbdab559d293c121c37113ddc7f1..cc097180d19ec7e8cfc818490e609fd2f937b5f5 100755 (executable)
--- a/ext/phar/phar_object.c
+++ b/ext/phar/phar_object.c
@@ -306,26 +306,130 @@ PHP_METHOD(Phar, __construct)
                return; \
        }
 
-/* {{{ proto Phar::buildFromIterator(Iterator iter[, string base_directory])
+static int phar_build(zend_object_iterator *iter, void *puser TSRMLS_DC)
+{
+       zval **value;
+       zend_uchar key_type;
+       ulong int_key;
+       struct _t {
+               phar_archive_object *p;
+               zend_class_entry *c;
+               char *b;
+               uint l;
+               zval *ret;
+       } *p_obj = (struct _t*) puser;
+       uint str_key_len, base_len = p_obj->l;
+       phar_entry_data *data;
+       php_stream *fp;
+       long contents_len;
+       char *fname, *error, *str_key, *base = p_obj->b, *opened;
+       zend_class_entry *ce = p_obj->c;
+       phar_archive_object *phar_obj = p_obj->p;
+
+       iter->funcs->get_current_data(iter, &value TSRMLS_CC);
+       if (EG(exception)) {
+               return ZEND_HASH_APPLY_STOP;
+       }
+       if (!value) {
+               /* failure in get_current_data */
+               zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Iterator %s returned no value", ce->name);
+       }
+       if (Z_TYPE_PP(value) != IS_STRING) {
+               zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Iterator %s returned an invalid value (must return a string)", ce->name);
+               return ZEND_HASH_APPLY_STOP;
+       }
+       if (!base_len && iter->funcs->get_current_key) {
+               key_type = iter->funcs->get_current_key(iter, &str_key, &str_key_len, &int_key TSRMLS_CC);
+               if (EG(exception)) {
+                       return ZEND_HASH_APPLY_STOP;
+               }
+               if (key_type == HASH_KEY_IS_LONG) {
+                       zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Iterator %s returned an invalid key (must return a string)", ce->name);
+                       return ZEND_HASH_APPLY_STOP;
+               }
+               if (str_key[str_key_len - 1] == '\0') str_key_len--;
+       } else {
+               zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Iterator %s returned an invalid key (must return a string)", ce->name);
+               return ZEND_HASH_APPLY_STOP;
+       }
+
+       fname = Z_STRVAL_PP(value);
+       if (base_len) {
+               if (strstr(fname, base)) {
+                       str_key_len = Z_STRLEN_PP(value) - base_len;
+                       if (str_key_len <= 0) {
+                               return ZEND_HASH_APPLY_KEEP;
+                       }
+                       str_key = fname + base_len;
+               } else {
+                       zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Iterator %s returned a path \"%s\" that is not in the base directory \"%s\"", ce->name, fname, base);
+                       return ZEND_HASH_APPLY_STOP;
+               }
+       }
+#if PHP_MAJOR_VERSION < 6
+       if (PG(safe_mode) && (!php_checkuid(fname, NULL, CHECKUID_ALLOW_ONLY_FILE))) {
+               zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Iterator %s returned a path \"%s\" that safe mode prevents opening", ce->name, fname);
+               return ZEND_HASH_APPLY_STOP;
+       }
+#endif
+
+       if (php_check_open_basedir(fname TSRMLS_CC)) {
+               zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Iterator %s returned a path \"%s\" that open_basedir prevents opening", ce->name, fname);
+               return ZEND_HASH_APPLY_STOP;
+       }
+
+       /* try to open source file, then create internal phar file and copy contents */
+       fp = php_stream_open_wrapper(fname, "rb", STREAM_MUST_SEEK|0, &opened);
+       if (!fp) {
+               zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Iterator %s returned a file that could not be opened \"%s\"", ce->name, fname);
+               return ZEND_HASH_APPLY_STOP;
+       }
+
+       if (!(data = phar_get_or_create_entry_data(phar_obj->arc.archive->fname, phar_obj->arc.archive->fname_len, str_key, str_key_len, "w+b", &error TSRMLS_CC))) {
+               zend_throw_exception_ex(spl_ce_BadMethodCallException, 0 TSRMLS_CC, "Entry %s cannot be created: %s", str_key, error);
+               efree(error);
+               php_stream_close(fp);
+               return ZEND_HASH_APPLY_STOP;
+       } else {
+               if (error) {
+                       efree(error);
+               }
+               contents_len = php_stream_copy_to_stream(fp, data->fp, PHP_STREAM_COPY_ALL);
+       }
+       php_stream_close(fp);
+
+       add_assoc_string(p_obj->ret, str_key, opened, 1);
+
+       data->internal_file->compressed_filesize = data->internal_file->uncompressed_filesize = contents_len;
+       phar_entry_delref(data TSRMLS_CC);
+       return ZEND_HASH_APPLY_KEEP;
+}
+
+/* {{{ proto array Phar::buildFromIterator(Iterator iter[, string base_directory])
  * Construct a phar archive from an iterator.  The iterator must return a series of strings
  * that are full paths to files that should be added to the phar.  The iterator key should
  * be the path that the file will have within the phar archive.
  *
  * If base directory is specified, then the key will be ignored, and instead the portion of
  * the current value minus the base directory will be used
+ *
+ * Returned is an array mapping phar index to actual file added
  */
 PHP_METHOD(Phar, buildFromIterator)
 {
-       zend_object_iterator *iter = NULL;
-       zval *obj, **value;
-       char *str_key, *base, *error;
-       uint str_key_len, base_len = 0;
-       zend_uchar key_type;
-       ulong int_key;
-       zend_class_entry *ce;
+       zval *obj;
+       char *error;
+       uint base_len = 0;
+       char *base;
+       struct {
+               phar_archive_object *p;
+               zend_class_entry *c;
+               char *b;
+               uint l;
+               zval *ret;
+       } pass;
        PHAR_ARCHIVE_OBJECT();
 
-
        if (PHAR_G(readonly)) {
                zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC,
                        "Cannot write out phar archive, phar is read-only");
@@ -336,120 +440,19 @@ PHP_METHOD(Phar, buildFromIterator)
                RETURN_FALSE;
        }
 
-       ce = Z_OBJCE_P(obj);
-
-       iter = ce->get_iterator(ce, obj, 0 TSRMLS_CC);
-       if (iter && !EG(exception)) {
-               obj = zend_iterator_wrap(iter TSRMLS_CC);
-       } else {
-               if (!EG(exception)) {
-                       zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Object of type %s did not create an Iterator", ce->name);
-               }
-               return;
-       }
-
-       if (iter) {
-               iter->index = 0;
-               if (iter->funcs->rewind) {
-                       iter->funcs->rewind(iter TSRMLS_CC);
-               }
-               if (FAILURE == iter->funcs->valid(iter TSRMLS_CC)) {
-                       zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Iterator %s returned no values", ce->name);
-                       return;
-               }
-               do {
-                       phar_entry_data *data;
-                       php_stream *fp;
-                       long contents_len;
-                       char *fname;
-                       iter->funcs->get_current_data(iter, &value TSRMLS_CC);
-                       if (EG(exception)) {
-                               break;
-                       }
-                       if (!value) {
-                               /* failure in get_current_data */
-                               zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Iterator %s returned no value", ce->name);
-                       }
-                       if (Z_TYPE_PP(value) != IS_STRING) {
-                               zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Iterator %s returned an invalid value (must return a string)", ce->name);
-                               break;
-                       }
-                       if (!base_len && iter->funcs->get_current_key) {
-                               key_type = iter->funcs->get_current_key(iter, &str_key, &str_key_len, &int_key TSRMLS_CC);
-                               if (EG(exception)) {
-                                       break;
-                               }
-                               if (key_type == HASH_KEY_IS_LONG) {
-                                       zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Iterator %s returned an invalid key (must return a string)", ce->name);
-                                       break;
-                               }
-                               if (str_key[str_key_len - 1] == '\0') str_key_len--;
-                       } else {
-                               zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Iterator %s returned an invalid key (must return a string)", ce->name);
-                               break;
-                       }
-
-                       fname = Z_STRVAL_PP(value);
-                       if (base_len) {
-                               if (strstr(fname, base)) {
-                                       str_key_len = Z_STRLEN_PP(value) - base_len;
-                                       if (str_key_len <= 0) {
-                                               continue;
-                                       }
-                                       str_key = fname + base_len;
-                               } else {
-                                       zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Iterator %s returned a path \"%s\" that is not in the base directory \"%s\"", ce->name, fname, base);
-                                       break;
-                               }
-                       }
-#if PHP_MAJOR_VERSION < 6
-                       if (PG(safe_mode) && (!php_checkuid(fname, NULL, CHECKUID_ALLOW_ONLY_FILE))) {
-                               zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Iterator %s returned a path \"%s\" that safe mode prevents opening", ce->name, fname);
-                               break;
-                       }
-#endif
-
-                       if (php_check_open_basedir(fname TSRMLS_CC)) {
-                               zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Iterator %s returned a path \"%s\" that open_basedir prevents opening", ce->name, fname);
-                               break;
-                       }
+       array_init(return_value);
 
-                       /* try to open source file, then create internal phar file and copy contents */
-                       fp = php_stream_open_wrapper(fname, "rb", STREAM_MUST_SEEK|0, NULL);
-                       if (!fp) {
-                               zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0 TSRMLS_CC, "Iterator %s returned a file that could not be opened \"%s\"", ce->name, fname);
-                               break;
-                       }
+       pass.c = Z_OBJCE_P(obj);
+       pass.p = phar_obj;
+       pass.b = base;
+       pass.l = base_len;
+       pass.ret = return_value;
 
-                       if (!(data = phar_get_or_create_entry_data(phar_obj->arc.archive->fname, phar_obj->arc.archive->fname_len, str_key, str_key_len, "w+b", &error TSRMLS_CC))) {
-                               zend_throw_exception_ex(spl_ce_BadMethodCallException, 0 TSRMLS_CC, "Entry %s cannot be created: %s", str_key, error);
-                               efree(error);
-                               break;
-                       } else {
-                               if (error) {
-                                       efree(error);
-                               }
-                               contents_len = php_stream_copy_to_stream(fp, data->fp, PHP_STREAM_COPY_ALL);
-                       }
-                       data->internal_file->compressed_filesize = data->internal_file->uncompressed_filesize = contents_len;
-                       phar_entry_delref(data TSRMLS_CC);
-                       /* This could cause an endless loop if index becomes zero again.
-                        * In case that ever happens we need an additional flag. */
-                       iter->funcs->move_forward(iter TSRMLS_CC);
-                       if (EG(exception)) {
-                               break;
-                       }
-                       if (iter->funcs->valid(iter TSRMLS_CC) == FAILURE) {
-                               /* reached end of iteration */
-                               break;
-                       }
-               } while (1);
-               if (!EG(exception)) {
-                       phar_flush(phar_obj->arc.archive, 0, 0, &error TSRMLS_CC);
-                       if (error) {
-                               zend_throw_exception_ex(phar_ce_PharException, 0 TSRMLS_CC, error);
-                               efree(error);
-                       }
+       if (SUCCESS == spl_iterator_apply(obj, (spl_iterator_apply_func_t) phar_build, (void *) &pass)) {
+               phar_flush(phar_obj->arc.archive, 0, 0, &error TSRMLS_CC);
+               if (error) {
+                       zend_throw_exception_ex(phar_ce_PharException, 0 TSRMLS_CC, error);
+                       efree(error);
                }
        }
 

diff --git a/ext/phar/tests/phar_buildfromiterator1.phpt b/ext/phar/tests/phar_buildfromiterator1.phpt
new file mode 100644 (file)
index 0000000..6552858
--- /dev/null
+++ b/ext/phar/tests/phar_buildfromiterator1.phpt
@@ -0,0 +1,28 @@
+--TEST--
+Phar::buildFromIterator() readonly
+--SKIPIF--
+<?php if (!extension_loaded("phar")) print "skip"; ?>
+--INI--
+phar.require_hash=0
+phar.readonly=0
+--FILE--
+<?php
+$phar = new Phar(dirname(__FILE__) . '/buildfromiterator.phar');
+try {
+       ini_set('phar.readonly', 1);
+       $phar->buildFromIterator(1);
+} catch (Exception $e) {
+       var_dump(get_class($e));
+       echo $e->getMessage() . "\n";
+}
+?>
+===DONE===
+--CLEAN--
+<?php 
+unlink(dirname(__FILE__) . '/buildfromiterator.phar');
+__HALT_COMPILER();
+?>
+--EXPECT--
+string(24) "UnexpectedValueException"
+Cannot write out phar archive, phar is read-only
+===DONE===

diff --git a/ext/phar/tests/phar_buildfromiterator2.phpt b/ext/phar/tests/phar_buildfromiterator2.phpt
new file mode 100644 (file)
index 0000000..44cde5e
--- /dev/null
+++ b/ext/phar/tests/phar_buildfromiterator2.phpt
@@ -0,0 +1,26 @@
+--TEST--
+Phar::buildFromIterator() wrong object
+--SKIPIF--
+<?php if (!extension_loaded("phar")) print "skip"; ?>
+--INI--
+phar.require_hash=0
+phar.readonly=0
+--FILE--
+<?php
+try {
+       $phar = new Phar(dirname(__FILE__) . '/buildfromiterator.phar');
+       $phar->buildFromIterator(new stdClass);
+} catch (Exception $e) {
+       var_dump(get_class($e));
+       echo $e->getMessage() . "\n";
+}
+?>
+===DONE===
+--CLEAN--
+<?php 
+unlink(dirname(__FILE__) . '/buildfromiterator.phar');
+__HALT_COMPILER();
+?>
+--EXPECTF--
+Warning: Phar::buildFromIterator() expects parameter 1 to be Traversable, object given in %sphar_buildfromiterator2.php on line %d
+===DONE===

diff --git a/ext/phar/tests/phar_buildfromiterator3.phpt b/ext/phar/tests/phar_buildfromiterator3.phpt
new file mode 100644 (file)
index 0000000..d3bac72
--- /dev/null
+++ b/ext/phar/tests/phar_buildfromiterator3.phpt
@@ -0,0 +1,54 @@
+--TEST--
+Phar::buildFromIterator() iterator, but object passed
+--SKIPIF--
+<?php if (!extension_loaded("phar")) print "skip"; ?>
+--INI--
+phar.require_hash=0
+phar.readonly=0
+--FILE--
+<?php
+class myIterator implements Iterator
+{
+    var $a;
+    function __construct(array $a)
+    {
+        $this->a = $a;
+    }
+    function next() {
+        echo "next\n";
+        return next($this->a);
+    }
+    function current() {
+        echo "current\n";
+        return current($this->a);
+    }
+    function key() {
+        echo "key\n";
+        return key($this->a);
+    }
+    function valid() {
+        echo "valid\n";
+        return current($this->a);
+    }
+    function rewind() {
+        echo "rewind\n";
+        return reset($this->a);
+    }
+}
+try {
+       $phar = new Phar(dirname(__FILE__) . '/buildfromiterator.phar');
+       $phar->buildFromIterator(new myIterator(array()), new stdClass);
+} catch (Exception $e) {
+       var_dump(get_class($e));
+       echo $e->getMessage() . "\n";
+}
+?>
+===DONE===
+--CLEAN--
+<?php 
+unlink(dirname(__FILE__) . '/buildfromiterator.phar');
+__HALT_COMPILER();
+?>
+--EXPECTF--
+Warning: Phar::buildFromIterator() expects parameter 2 to be string, object given in %sphar_buildfromiterator3.php on line %d
+===DONE===

diff --git a/ext/phar/tests/phar_buildfromiterator4.phpt b/ext/phar/tests/phar_buildfromiterator4.phpt
new file mode 100644 (file)
index 0000000..d0af185
--- /dev/null
+++ b/ext/phar/tests/phar_buildfromiterator4.phpt
@@ -0,0 +1,64 @@
+--TEST--
+Phar::buildFromIterator() iterator, nothng passed in
+--SKIPIF--
+<?php if (!extension_loaded("phar")) print "skip"; ?>
+--INI--
+phar.require_hash=0
+phar.readonly=0
+--FILE--
+<?php
+class myIterator implements Iterator
+{
+    var $a;
+    function __construct(array $a)
+    {
+        $this->a = $a;
+    }
+    function next() {
+        echo "next\n";
+        return next($this->a);
+    }
+    function current() {
+        echo "current\n";
+        return current($this->a);
+    }
+    function key() {
+        echo "key\n";
+        return key($this->a);
+    }
+    function valid() {
+        echo "valid\n";
+        return current($this->a);
+    }
+    function rewind() {
+        echo "rewind\n";
+        return reset($this->a);
+    }
+}
+try {
+       chdir(dirname(__FILE__));
+       $phar = new Phar(dirname(__FILE__) . '/buildfromiterator.phar');
+       var_dump($phar->buildFromIterator(new myIterator(array('a' => basename(__FILE__, 'php') . 'phpt'))));
+} catch (Exception $e) {
+       var_dump(get_class($e));
+       echo $e->getMessage() . "\n";
+}
+?>
+===DONE===
+--CLEAN--
+<?php 
+unlink(dirname(__FILE__) . '/buildfromiterator.phar');
+__HALT_COMPILER();
+?>
+--EXPECTF--
+rewind
+valid
+current
+key
+next
+valid
+array (1) {
+  ["a"] =>
+  string(%d) "%sphar_buildfromiterator4.phpt"
+}
+===DONE===