Wrap strings passed to libzip with zip_source_function_create()

Wrap strings passed to libzip with zip_source_function_create()
instead of using zip_source_buffer_create(). This allows us to make
the string writable, and simplifies memory management.

Author: tstarling

ext/zip/config.m4

@@ -49,7 +49,7 @@ if test "$PHP_ZIP" != "no"; then
   AC_DEFINE([HAVE_ZIP], [1],
     [Define to 1 if the PHP extension 'zip' is available.])
 
-  PHP_NEW_EXTENSION([zip], [php_zip.c zip_stream.c], [$ext_shared])
+  PHP_NEW_EXTENSION([zip], [php_zip.c zip_source.c zip_stream.c], [$ext_shared])
   PHP_ADD_EXTENSION_DEP(zip, pcre)
 
   PHP_SUBST([ZIP_SHARED_LIBADD])

ext/zip/config.w32

@@ -8,7 +8,7 @@ if (PHP_ZIP != "no") {
 		(PHP_ZIP_SHARED && CHECK_LIB("libzip.lib", "zip", PHP_ZIP) ||
 		 CHECK_LIB("libzip_a.lib", "zip", PHP_ZIP) && CHECK_LIB("libbz2_a.lib", "zip", PHP_ZIP) && CHECK_LIB("zlib_a.lib", "zip", PHP_ZIP) && CHECK_LIB("liblzma_a.lib", "zip", PHP_ZIP))
 	) {
-		EXTENSION('zip', 'php_zip.c zip_stream.c');
+		EXTENSION('zip', 'php_zip.c zip_source.c zip_stream.c');
 		ADD_EXTENSION_DEP('zip', 'pcre');
 
 		if (get_define("LIBS_ZIP").match("libzip_a(?:_debug)?\.lib")) {

ext/zip/php_zip.c

@@ -575,28 +575,6 @@ static char * php_zipobj_get_zip_comment(ze_zip_object *obj, int *len) /* {{{ */
 }
 /* }}} */
 
-/* Add a string to the list of buffers to be released when the object is destroyed.*/
-static void php_zipobj_add_buffer(ze_zip_object *obj, zend_string *str) /* {{{ */
-{
-	size_t pos = obj->buffers_cnt++;
-	obj->buffers = safe_erealloc(obj->buffers, sizeof(*obj->buffers), obj->buffers_cnt, 0);
-	obj->buffers[pos] = zend_string_copy(str);
-}
-/* }}} */
-
-static void php_zipobj_release_buffers(ze_zip_object *obj) /* {{{ */
-{
-	if (obj->buffers_cnt > 0) {
-		for (size_t i = 0; i < obj->buffers_cnt; i++) {
-			zend_string_release(obj->buffers[i]);
-		}
-		efree(obj->buffers);
-		obj->buffers = NULL;
-	}
-	obj->buffers_cnt = 0;
-}
-/* }}} */
-
 /* Close and free the zip_t */
 static bool php_zipobj_close(ze_zip_object *obj) /* {{{ */
 {
@@ -630,8 +608,6 @@ static bool php_zipobj_close(ze_zip_object *obj) /* {{{ */
 		obj->filename_len = 0;
 	}
 
-	php_zipobj_release_buffers(obj);
-
 	obj->za = NULL;
 	return success;
 }
@@ -1531,10 +1507,12 @@ PHP_METHOD(ZipArchive, openString)
 
 	ze_zip_object *ze_obj = Z_ZIP_P(self);
 
+	php_zipobj_close(ze_obj);
+
 	zip_error_t err;
 	zip_error_init(&err);
 
-	zip_source_t * zip_source = zip_source_buffer_create(ZSTR_VAL(buffer), ZSTR_LEN(buffer), 0, &err);
+	zip_source_t * zip_source = php_zip_create_string_source(buffer, NULL, &err);
 
 	if (!zip_source) {
 		ze_obj->err_zip = zip_error_code_zip(&err);
@@ -1543,8 +1521,6 @@ PHP_METHOD(ZipArchive, openString)
 		RETURN_LONG(ze_obj->err_zip);
 	}
 
-	php_zipobj_close(ze_obj);
-
 	struct zip *intern = zip_open_from_source(zip_source, ZIP_RDONLY, &err);
 	if (!intern) {
 		ze_obj->err_zip = zip_error_code_zip(&err);
@@ -1554,7 +1530,6 @@ PHP_METHOD(ZipArchive, openString)
 		RETURN_LONG(ze_obj->err_zip);
 	}
 
-	php_zipobj_add_buffer(ze_obj, buffer);
 	ze_obj->za = intern;
 	zip_error_fini(&err);
 	RETURN_TRUE;
@@ -1597,7 +1572,7 @@ PHP_METHOD(ZipArchive, close)
 }
 /* }}} */
 
-/* {{{ close the zip archive */
+/* {{{ get the number of entries */
 PHP_METHOD(ZipArchive, count)
 {
 	struct zip *intern;
@@ -1911,9 +1886,7 @@ PHP_METHOD(ZipArchive, addFromString)
 	ZIP_FROM_OBJECT(intern, self);
 
 	ze_obj = Z_ZIP_P(self);
-	php_zipobj_add_buffer(ze_obj, buffer);
-
-	zs = zip_source_buffer(intern, ZSTR_VAL(buffer), ZSTR_LEN(buffer), 0);
+	zs = php_zip_create_string_source(buffer, NULL, NULL);
 
 	if (zs == NULL) {
 		RETURN_FALSE;

ext/zip/php_zip.h

@@ -68,11 +68,9 @@ typedef struct _ze_zip_read_rsrc {
 /* Extends zend object */
 typedef struct _ze_zip_object {
 	struct zip *za;
-	zend_string **buffers;
 	HashTable *prop_handler;
 	char *filename;
 	size_t filename_len;
-	size_t buffers_cnt;
 	zip_int64_t last_id;
 	int err_zip;
 	int err_sys;
@@ -96,6 +94,8 @@ php_stream *php_stream_zip_open(struct zip *arch, struct zip_stat *sb, const cha
 
 extern const php_stream_wrapper php_stream_zip_wrapper;
 
+zip_source_t * php_zip_create_string_source(zend_string *str, zend_string **dest, zip_error_t *err);
+
 #define LIBZIP_ATLEAST(m,n,p) (((m<<16) + (n<<8) + p) <= ((LIBZIP_VERSION_MAJOR<<16) + (LIBZIP_VERSION_MINOR<<8) + LIBZIP_VERSION_MICRO))
 
 #endif	/* PHP_ZIP_H */

ext/zip/tests/checkcons.zip

@@ -0,0 +1,188 @@
+#ifdef HAVE_CONFIG_H
+#   include "config.h"
+#endif
+#include "php.h"
+#include "php_zip.h"
+
+typedef struct _php_zip_string_source {
+	/* The current string being read from */
+	zend_string *in_str;
+	/* The offset into in_str of the current read position */
+	size_t in_offset;
+	/* The modification time returned in stat calls */
+	time_t mtime;
+	/* The current string being written to */
+	zend_string *out_str;
+	/* The offset into out_str of the current write position */
+	size_t out_offset;
+	/* A place to copy the result to when the archive is closed, or NULL */
+	zend_string **dest;
+	/* The error to be returned when libzip asks for the last error code */
+	zip_error_t error;
+} php_zip_string_source;
+
+static zip_int64_t php_zip_string_cb(void *userdata, void *data, zip_uint64_t len, zip_source_cmd_t cmd)
+{
+	php_zip_string_source *ctx = userdata;
+	switch (cmd) {
+		case ZIP_SOURCE_SUPPORTS:
+			return zip_source_make_command_bitmap(
+				ZIP_SOURCE_FREE,
+#if LIBZIP_VERSION_MAJOR > 1 || LIBZIP_VERSION_MINOR >= 10
+				ZIP_SOURCE_SUPPORTS_REOPEN,
+#endif
+				ZIP_SOURCE_OPEN,
+				ZIP_SOURCE_READ,
+				ZIP_SOURCE_CLOSE,
+				ZIP_SOURCE_STAT,
+				ZIP_SOURCE_ERROR,
+				ZIP_SOURCE_SEEK,
+				ZIP_SOURCE_TELL,
+				ZIP_SOURCE_BEGIN_WRITE,
+				ZIP_SOURCE_WRITE,
+				ZIP_SOURCE_COMMIT_WRITE,
+				ZIP_SOURCE_ROLLBACK_WRITE,
+				ZIP_SOURCE_SEEK_WRITE,
+				ZIP_SOURCE_TELL_WRITE,
+				ZIP_SOURCE_REMOVE,
+				-1
+			);
+
+		case ZIP_SOURCE_FREE:
+			zend_string_release(ctx->out_str);
+			zend_string_release(ctx->in_str);
+			efree(ctx);
+			return 0;
+
+		/* Read ops */
+
+		case ZIP_SOURCE_OPEN:
+			ctx->in_offset = 0;
+			return 0;
+
+		case ZIP_SOURCE_READ: {
+			size_t remaining = ZSTR_LEN(ctx->in_str) - ctx->in_offset;
+			len = MIN(len, remaining);
+			if (len) {
+				memcpy(data, ZSTR_VAL(ctx->in_str) + ctx->in_offset, len);
+				ctx->in_offset += len;
+			}
+			return len;
+		}
+
+		case ZIP_SOURCE_CLOSE:
+			return 0;
+
+		case ZIP_SOURCE_STAT: {
+			zip_stat_t *st;
+			if (len < sizeof(*st)) {
+				zip_error_set(&ctx->error, ZIP_ER_INVAL, 0);
+				return -1;
+			}
+
+			st = (zip_stat_t *)data;
+			zip_stat_init(st);
+			st->mtime = ctx->mtime;
+			st->size = ZSTR_LEN(ctx->in_str);
+			st->comp_size = st->size;
+			st->comp_method = ZIP_CM_STORE;
+			st->encryption_method = ZIP_EM_NONE;
+			st->valid = ZIP_STAT_MTIME | ZIP_STAT_SIZE | ZIP_STAT_COMP_SIZE | ZIP_STAT_COMP_METHOD | ZIP_STAT_ENCRYPTION_METHOD;
+
+			return sizeof(*st);
+		}
+
+		case ZIP_SOURCE_ERROR:
+			return zip_error_to_data(&ctx->error, data, len);
+
+		/* Seekable read ops */
+
+		case ZIP_SOURCE_SEEK: {
+			zip_int64_t new_offset = zip_source_seek_compute_offset(
+					ctx->in_offset, ZSTR_LEN(ctx->in_str), data, len, &ctx->error);
+			if (new_offset < 0) {
+				return -1;
+			}
+			ctx->in_offset = (size_t)new_offset;
+			return 0;
+		}
+
+		case ZIP_SOURCE_TELL:
+			if (ctx->in_offset > ZIP_INT64_MAX) {
+				zip_error_set(&ctx->error, ZIP_ER_TELL, EOVERFLOW);
+				return -1;
+			}
+			return (zip_int64_t)ctx->in_offset;
+
+		/* Write ops */
+
+		case ZIP_SOURCE_BEGIN_WRITE:
+			zend_string_release(ctx->out_str);
+			ctx->out_str = ZSTR_EMPTY_ALLOC();
+			return 0;
+
+		case ZIP_SOURCE_WRITE:
+			if (ctx->out_offset > SIZE_MAX - len) {
+				zip_error_set(&ctx->error, ZIP_ER_MEMORY, 0);
+				return -1;
+			}
+			if (ctx->out_offset + len > ZSTR_LEN(ctx->out_str)) {
+				ctx->out_str = zend_string_realloc(ctx->out_str, ctx->out_offset + len, false);
+			}
+			memcpy(ZSTR_VAL(ctx->out_str) + ctx->out_offset, data, len);
+			ctx->out_offset += len;
+			return len;
+
+		case ZIP_SOURCE_COMMIT_WRITE:
+			ZSTR_VAL(ctx->out_str)[ZSTR_LEN(ctx->out_str)] = '\0';
+			zend_string_release(ctx->in_str);
+			ctx->in_str = ctx->out_str;
+			ctx->out_str = ZSTR_EMPTY_ALLOC();
+			if (ctx->dest) {
+				*(ctx->dest) = zend_string_copy(ctx->in_str);
+			}
+			return 0;
+
+		case ZIP_SOURCE_ROLLBACK_WRITE:
+			zend_string_release(ctx->out_str);
+			ctx->out_str = ZSTR_EMPTY_ALLOC();
+			return 0;
+
+		case ZIP_SOURCE_SEEK_WRITE: {
+			zip_int64_t new_offset = zip_source_seek_compute_offset(
+					ctx->out_offset, ZSTR_LEN(ctx->out_str), data, len, &ctx->error);
+			if (new_offset < 0) {
+				return -1;
+			}
+			ctx->out_offset = new_offset;
+			return 0;
+		}
+
+		case ZIP_SOURCE_TELL_WRITE:
+			if (ctx->out_offset > ZIP_INT64_MAX) {
+				zip_error_set(&ctx->error, ZIP_ER_TELL, EOVERFLOW);
+				return -1;
+			}
+			return (zip_int64_t)ctx->out_offset;
+
+		case ZIP_SOURCE_REMOVE:
+			zend_string_release(ctx->in_str);
+			ctx->in_str = ZSTR_EMPTY_ALLOC();
+			ctx->in_offset = 0;
+			return 0;
+
+		default:
+			zip_error_set(&ctx->error, ZIP_ER_OPNOTSUPP, 0);
+			return -1;
+	}
+}
+
+zip_source_t * php_zip_create_string_source(zend_string *str, zend_string **dest, zip_error_t *err) /* {{{ */
+{
+	php_zip_string_source *ctx = ecalloc(1, sizeof(php_zip_string_source));
+	ctx->in_str = zend_string_copy(str);
+	ctx->out_str = ZSTR_EMPTY_ALLOC();
+	ctx->dest = dest;
+	ctx->mtime = time(NULL);
+	return zip_source_function_create(php_zip_string_cb, (void*)ctx, err);
+}