minor re-factoring of phar_open_url to remove one nested brackets, put fopen edge...

diff --git a/ext/phar/stream.c b/ext/phar/stream.c
index 0d73908d53029de56beb137c03ded82e7068291e..43937f4c019eadf460d3b96a9e8c04becd6aab5a 100644 (file)
--- a/ext/phar/stream.c
+++ b/ext/phar/stream.c
@@ -63,35 +63,37 @@ php_url* phar_open_url(php_stream_wrapper *wrapper, char *filename, char *mode,
        char *arch = NULL, *entry = NULL, *error;
        int arch_len, entry_len;
 
-       if (!strncasecmp(filename, "phar://", 7)) {
-               if (mode[0] == 'a') {
-                       if (!(options & PHP_STREAM_URL_STAT_QUIET)) {
-                               php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, "phar error: open mode append not supported");
-                       }
-                       return NULL;                    
-               }               
-               if (phar_split_fname(filename, strlen(filename), &arch, &arch_len, &entry, &entry_len TSRMLS_CC) == FAILURE) {
-                       if (!(options & PHP_STREAM_URL_STAT_QUIET)) {
-                               if (arch && !entry) {
-                                       php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, "phar error: no directory in \"%s\", must have at least phar://%s/ for root directory (always use full path to a new phar)", filename, arch);
-                                       arch = NULL;
-                               } else {
-                                       php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, "phar error: invalid url \"%s\" (cannot contain .phar.php and .phar.gz/.phar.bz2)", filename);
-                               }
-                       }
-                       if (arch) {
-                               efree(arch);
-                       }
-                       if (entry) {
-                               efree(entry);
+       if (strlen(filename) < 7 || strncasecmp(filename, "phar://", 7)) {
+               return NULL;
+       }
+       if (mode[0] == 'a') {
+               if (!(options & PHP_STREAM_URL_STAT_QUIET)) {
+                       php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, "phar error: open mode append not supported");
+               }
+               return NULL;                    
+       }               
+       if (phar_split_fname(filename, strlen(filename), &arch, &arch_len, &entry, &entry_len TSRMLS_CC) == FAILURE) {
+               if (!(options & PHP_STREAM_URL_STAT_QUIET)) {
+                       if (arch && !entry) {
+                               php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, "phar error: no directory in \"%s\", must have at least phar://%s/ for root directory (always use full path to a new phar)", filename, arch);
+                               arch = NULL;
+                       } else {
+                               php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, "phar error: invalid url \"%s\" (cannot contain .phar.php and .phar.gz/.phar.bz2)", filename);
                        }
-                       return NULL;
                }
-               resource = ecalloc(1, sizeof(php_url));
-               resource->scheme = estrndup("phar", 4);
-               resource->host = arch;
+               if (arch) {
+                       efree(arch);
+               }
+               if (entry) {
+                       efree(entry);
+               }
+               return NULL;
+       }
+       resource = ecalloc(1, sizeof(php_url));
+       resource->scheme = estrndup("phar", 4);
+       resource->host = arch;
 
-               resource->path = entry;
+       resource->path = entry;
 #if MBO_0
                if (resource) {
                        fprintf(stderr, "Alias:     %s\n", alias);
@@ -105,50 +107,47 @@ php_url* phar_open_url(php_stream_wrapper *wrapper, char *filename, char *mode,
 /*                     fprintf(stderr, "Fragment:  %s\n", resource->fragment);*/
                }
 #endif
-               if (PHAR_GLOBALS->request_init && PHAR_GLOBALS->phar_plain_map.arBuckets && zend_hash_exists(&(PHAR_GLOBALS->phar_plain_map), arch, arch_len+1)) {
-                       return resource;
-               }
-               if (mode[0] == 'w' || (mode[0] == 'r' && mode[1] == '+')) {
-                       phar_archive_data **pphar = NULL;
+       if (PHAR_GLOBALS->request_init && PHAR_GLOBALS->phar_plain_map.arBuckets && zend_hash_exists(&(PHAR_GLOBALS->phar_plain_map), arch, arch_len+1)) {
+               return resource;
+       }
+       if (mode[0] == 'w' || (mode[0] == 'r' && mode[1] == '+')) {
+               phar_archive_data **pphar = NULL;
 
-                       if (PHAR_GLOBALS->request_init && PHAR_GLOBALS->phar_fname_map.arBuckets && FAILURE == zend_hash_find(&(PHAR_GLOBALS->phar_fname_map), arch, arch_len, (void **)&pphar)) {
-                               pphar = NULL;
+               if (PHAR_GLOBALS->request_init && PHAR_GLOBALS->phar_fname_map.arBuckets && FAILURE == zend_hash_find(&(PHAR_GLOBALS->phar_fname_map), arch, arch_len, (void **)&pphar)) {
+                       pphar = NULL;
+               }
+               if (PHAR_G(readonly) && (!pphar || !(*pphar)->is_data)) {
+                       if (!(options & PHP_STREAM_URL_STAT_QUIET)) {
+                               php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, "phar error: write operations disabled by INI setting");
                        }
-                       if (PHAR_G(readonly) && (!pphar || !(*pphar)->is_data)) {
+                       php_url_free(resource);
+                       return NULL;
+               }
+               if (phar_open_or_create_filename(resource->host, arch_len, NULL, 0, NULL, options, NULL, &error TSRMLS_CC) == FAILURE)
+               {
+                       if (error) {
                                if (!(options & PHP_STREAM_URL_STAT_QUIET)) {
-                                       php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, "phar error: write operations disabled by INI setting");
-                               }
-                               php_url_free(resource);
-                               return NULL;
-                       }
-                       if (phar_open_or_create_filename(resource->host, arch_len, NULL, 0, NULL, options, NULL, &error TSRMLS_CC) == FAILURE)
-                       {
-                               if (error) {
-                                       if (!(options & PHP_STREAM_URL_STAT_QUIET)) {
-                                               php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, error);
-                                       }
-                                       efree(error);
+                                       php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, error);
                                }
-                               php_url_free(resource);
-                               return NULL;
+                               efree(error);
                        }
-               } else {
-                       if (phar_open_filename(resource->host, arch_len, NULL, 0, options, NULL, &error TSRMLS_CC) == FAILURE)
-                       {
-                               if (error) {
-                                       if (!(options & PHP_STREAM_URL_STAT_QUIET)) {
-                                               php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, error);
-                                       }
-                                       efree(error);
+                       php_url_free(resource);
+                       return NULL;
+               }
+       } else {
+               if (phar_open_filename(resource->host, arch_len, NULL, 0, options, NULL, &error TSRMLS_CC) == FAILURE)
+               {
+                       if (error) {
+                               if (!(options & PHP_STREAM_URL_STAT_QUIET)) {
+                                       php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, error);
                                }
-                               php_url_free(resource);
-                               return NULL;
+                               efree(error);
                        }
-               }       
-               return resource;
-       }
-
-       return NULL;
+                       php_url_free(resource);
+                       return NULL;
+               }
+       }       
+       return resource;
 }
 /* }}} */
 

diff --git a/ext/phar/tests/fopen_edgecases.phpt b/ext/phar/tests/fopen_edgecases.phpt
new file mode 100644 (file)
index 0000000..3381984
--- /dev/null
+++ b/ext/phar/tests/fopen_edgecases.phpt
@@ -0,0 +1,43 @@
+--TEST--
+Phar: fopen/stat edge cases
+--SKIPIF--
+<?php if (!extension_loaded("phar")) die("skip"); ?>
+--INI--
+phar.readonly=0
+phar.require_hash=0
+--FILE--
+<?php
+$fname = dirname(__FILE__) . '/' . basename(__FILE__, '.php') . '.phar.php';
+$fname2 = dirname(__FILE__) . '/' . basename(__FILE__, '.php') . '.2.phar.php';
+$pname = 'phar://' . $fname;
+$pname2 = 'phar://' . $fname2;
+
+// append
+$a = fopen($pname . '/b/c.php', 'a');
+// invalid pharname
+$a = fopen($pname . '.phar.gz', 'r');
+// test phar_open_url() with quiet stat for code coverage
+var_dump(file_exists($pname . '.phar.gz/hi'));
+// test open for write with new phar
+$a = fopen($pname . '/hi', 'w');
+fclose($a);
+// test open for write with corrupted phar
+file_put_contents($fname2, '<?php oh crap __HALT_COMPILER();');
+$a = fopen($pname2 . '/hi', 'w');
+$a = fopen('phar://', 'r');
+$a = fopen('phar://foo.phar', 'r');
+?>
+
+===DONE===
+--CLEAN--
+<?php unlink(dirname(__FILE__) . '/' . basename(__FILE__, '.clean.php') . '.phar.php'); ?>
+<?php unlink(dirname(__FILE__) . '/' . basename(__FILE__, '.clean.php') . '.2.phar.php'); ?>
+--EXPECTF--
+Warning: fopen(phar://%sfopen_edgecases.phar.php/b/c.php): failed to open stream: phar error: open mode append not supported in %sfopen_edgecases.php on line %d
+
+Warning: fopen(phar://%sfopen_edgecases.phar.php.phar.gz): failed to open stream: phar error: invalid url "phar://%sfopen_edgecases.phar.php.phar.gz" (cannot contain .phar.php and .phar.gz/.phar.bz2) in %sfopen_edgecases.php on line %d
+bool(false)
+
+Warning: fopen(phar://%sfopen_edgecases.2.phar.php/hi): failed to open stream: internal corruption of phar "%sfopen_edgecases.2.phar.php" (truncated manifest at stub end) in %sfopen_edgecases.php on line %d
+
+===DONE===
\ No newline at end of file

diff --git a/ext/phar/tests/open_for_write_existing.phpt b/ext/phar/tests/open_for_write_existing.phpt
index a3313b80ef17224caf80c773ae595815baa414c8..2a3ec8b8245a832489e1f15d73b7c6fe952f7bd4 100644 (file)
--- a/ext/phar/tests/open_for_write_existing.phpt
+++ b/ext/phar/tests/open_for_write_existing.phpt
@@ -21,9 +21,6 @@ $fp = fopen($pname . '/b/c.php', 'wb');
 fwrite($fp, 'extra');
 fclose($fp);
 include $pname . '/b/c.php';
-
-// add edge case test for append
-$a = fopen($pname . '/b/c.php', 'a');
 ?>
 
 ===DONE===
@@ -31,6 +28,4 @@ $a = fopen($pname . '/b/c.php', 'a');
 <?php unlink(dirname(__FILE__) . '/' . basename(__FILE__, '.clean.php') . '.phar.php'); ?>
 --EXPECTF--
 extra
-Warning: fopen(phar://%sopen_for_write_existing.phar.php/b/c.php): failed to open stream: phar error: open mode append not supported in %sopen_for_write_existing.php on line %d
-
 ===DONE===

diff --git a/ext/phar/tests/phar_dir_iterate.phpt b/ext/phar/tests/phar_dir_iterate.phpt
index 1ca8639eaedd97557768abe7a34b6406a2c28da6..2ee2cb5529f58f916a028d7f3aec0c0feb939f8d 100644 (file)
--- a/ext/phar/tests/phar_dir_iterate.phpt
+++ b/ext/phar/tests/phar_dir_iterate.phpt
@@ -8,12 +8,13 @@ phar.readonly=0
 phar.require_hash=0
 --FILE--
 <?php
+$fname = dirname(__FILE__) . '/' . basename(__FILE__, '.php') . '.phar.php';
 
-$phar = new Phar(dirname(__FILE__) . '/test.phar');
+$phar = new Phar($fname);
 $phar['top.txt'] = 'hi';
 $phar['sub/top.txt'] = 'there';
 $phar['another.file.txt'] = 'wowee';
-$newphar = new Phar(dirname(__FILE__) . '/test.phar');
+$newphar = new Phar($fname);
 foreach (new RecursiveIteratorIterator($newphar) as $path => $obj) {
        var_dump($obj->getPathName());
 }
@@ -21,11 +22,11 @@ foreach (new RecursiveIteratorIterator($newphar) as $path => $obj) {
 ===DONE===
 --CLEAN--
 <?php 
-unlink(dirname(__FILE__) . '/test.phar');
+unlink(dirname(__FILE__) . '/' . basename(__FILE__, '.clean.php') . '.phar.php');
 __halt_compiler();
 ?>
 --EXPECTF--
-string(%d) "phar://%stest.phar%canother.file.txt"
-string(%d) "phar://%stest.phar%csub%ctop.txt"
-string(%d) "phar://%stest.phar%ctop.txt"
+string(%d) "phar://%sphar_dir_iterate.phar.php%canother.file.txt"
+string(%d) "phar://%sphar_dir_iterate.phar.php%csub%ctop.txt"
+string(%d) "phar://%sphar_dir_iterate.phar.php%ctop.txt"
 ===DONE===