ext/phar: assert function are not passed NULL pointers

This simplifies some of the logic and makes the assumptions clear

Author: Girgias

ext/phar/phar.c

@@ -404,11 +404,11 @@ void phar_request_initialize(void);
 void phar_object_init(void);
 void phar_destroy_phar_data(phar_archive_data *phar);
 
-zend_result phar_postprocess_file(phar_entry_data *idata, uint32_t crc32, char **error, int process_zip);
+ZEND_ATTRIBUTE_NONNULL zend_result phar_postprocess_file(phar_entry_data *idata, uint32_t crc32, char **error, int process_zip);
 zend_result phar_open_from_filename(char *fname, size_t fname_len, char *alias, size_t alias_len, uint32_t options, phar_archive_data** pphar, char **error);
-zend_result phar_open_or_create_filename(char *fname, size_t fname_len, char *alias, size_t alias_len, bool is_data, uint32_t options, phar_archive_data** pphar, char **error);
+ZEND_ATTRIBUTE_NONNULL_ARGS(1, 7, 8) zend_result phar_open_or_create_filename(char *fname, size_t fname_len, char *alias, size_t alias_len, bool is_data, uint32_t options, phar_archive_data** pphar, char **error);
 zend_result phar_create_or_parse_filename(char *fname, size_t fname_len, char *alias, size_t alias_len, bool is_data, uint32_t options, phar_archive_data** pphar, char **error);
-zend_result phar_open_executed_filename(char *alias, size_t alias_len, char **error);
+ZEND_ATTRIBUTE_NONNULL_ARGS(3) zend_result phar_open_executed_filename(char *alias, size_t alias_len, char **error);
 zend_result phar_free_alias(phar_archive_data *phar, char *alias, size_t alias_len);
 zend_result phar_get_archive(phar_archive_data **archive, char *fname, size_t fname_len, char *alias, size_t alias_len, char **error);
 zend_result phar_verify_signature(php_stream *fp, size_t end_of_phar, uint32_t sig_type, char *sig, size_t sig_len, char *fname, char **signature, size_t *signature_len, char **error);
@@ -424,7 +424,7 @@ void phar_add_virtual_dirs(phar_archive_data *phar, char *filename, size_t filen
 zend_result phar_mount_entry(phar_archive_data *phar, char *filename, size_t filename_len, char *path, size_t path_len);
 zend_string *phar_find_in_include_path(zend_string *file, phar_archive_data **pphar);
 char *phar_fix_filepath(char *path, size_t *new_len, int use_cwd);
-phar_entry_info * phar_open_jit(phar_archive_data *phar, phar_entry_info *entry, char **error);
+ZEND_ATTRIBUTE_NONNULL phar_entry_info * phar_open_jit(const phar_archive_data *phar, phar_entry_info *entry, char **error);
 void phar_parse_metadata_lazy(const char *buffer, phar_metadata_tracker *tracker, uint32_t zip_metadata_len, bool persistent);
 bool phar_metadata_tracker_has_data(const phar_metadata_tracker* tracker, bool persistent);
 /* If this has data, free it and set all values to undefined. */
@@ -436,22 +436,22 @@ zend_result phar_metadata_tracker_unserialize_or_copy(phar_metadata_tracker* tra
 void destroy_phar_manifest_entry(zval *zv);
 int phar_seek_efp(phar_entry_info *entry, zend_off_t offset, int whence, zend_off_t position, int follow_links);
 php_stream *phar_get_efp(phar_entry_info *entry, int follow_links);
-zend_result phar_copy_entry_fp(phar_entry_info *source, phar_entry_info *dest, char **error);
-zend_result phar_open_entry_fp(phar_entry_info *entry, char **error, int follow_links);
+ZEND_ATTRIBUTE_NONNULL zend_result phar_copy_entry_fp(phar_entry_info *source, phar_entry_info *dest, char **error);
+ZEND_ATTRIBUTE_NONNULL zend_result phar_open_entry_fp(phar_entry_info *entry, char **error, int follow_links);
 phar_entry_info *phar_get_link_source(phar_entry_info *entry);
 zend_result phar_open_archive_fp(phar_archive_data *phar);
 zend_result phar_copy_on_write(phar_archive_data **pphar);
 
 /* tar functions in tar.c */
 bool phar_is_tar(char *buf, char *fname);
 zend_result phar_parse_tarfile(php_stream* fp, char *fname, size_t fname_len, char *alias, size_t alias_len, phar_archive_data** pphar, uint32_t compression, char **error);
-zend_result phar_open_or_create_tar(char *fname, size_t fname_len, char *alias, size_t alias_len, int is_data, uint32_t options, phar_archive_data** pphar, char **error);
-void phar_tar_flush(phar_archive_data *phar, zend_string *user_stub, bool is_default_stub, char **error);
+ZEND_ATTRIBUTE_NONNULL_ARGS(1, 7, 8) zend_result phar_open_or_create_tar(char *fname, size_t fname_len, char *alias, size_t alias_len, int is_data, uint32_t options, phar_archive_data** pphar, char **error);
+ZEND_ATTRIBUTE_NONNULL_ARGS(1, 4) void phar_tar_flush(phar_archive_data *phar, zend_string *user_stub, bool is_default_stub, char **error);
 
 /* zip functions in zip.c */
 int phar_parse_zipfile(php_stream *fp, char *fname, size_t fname_len, char *alias, size_t alias_len, phar_archive_data** pphar, char **error);
-int phar_open_or_create_zip(char *fname, size_t fname_len, char *alias, size_t alias_len, int is_data, uint32_t options, phar_archive_data** pphar, char **error);
-void phar_zip_flush(phar_archive_data *archive, zend_string *user_stub, bool is_default_stub, char **error);
+ZEND_ATTRIBUTE_NONNULL_ARGS(1, 7, 8) zend_result phar_open_or_create_zip(char *fname, size_t fname_len, char *alias, size_t alias_len, int is_data, uint32_t options, phar_archive_data** pphar, char **error);
+ZEND_ATTRIBUTE_NONNULL_ARGS(1, 4) void phar_zip_flush(phar_archive_data *archive, zend_string *user_stub, bool is_default_stub, char **error);
 
 #ifdef PHAR_MAIN
 extern const php_stream_wrapper php_stream_phar_wrapper;
@@ -465,10 +465,10 @@ void phar_entry_delref(phar_entry_data *idata);
 
 phar_entry_info *phar_get_entry_info(phar_archive_data *phar, char *path, size_t path_len, char **error, int security);
 phar_entry_info *phar_get_entry_info_dir(phar_archive_data *phar, char *path, size_t path_len, char dir, char **error, int security);
-phar_entry_data *phar_get_or_create_entry_data(char *fname, size_t fname_len, char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security);
-zend_result phar_get_entry_data(phar_entry_data **ret, char *fname, size_t fname_len, char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security);
-void phar_flush_ex(phar_archive_data *archive, zend_string *user_stub, bool is_default_stub, char **error);
-void phar_flush(phar_archive_data *archive, char **error);
+ZEND_ATTRIBUTE_NONNULL phar_entry_data *phar_get_or_create_entry_data(char *fname, size_t fname_len, char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security);
+ZEND_ATTRIBUTE_NONNULL zend_result phar_get_entry_data(phar_entry_data **ret, char *fname, size_t fname_len, char *path, size_t path_len, const char *mode, char allow_dir, char **error, int security);
+ZEND_ATTRIBUTE_NONNULL_ARGS(1, 4) void phar_flush_ex(phar_archive_data *archive, zend_string *user_stub, bool is_default_stub, char **error);
+ZEND_ATTRIBUTE_NONNULL void phar_flush(phar_archive_data *archive, char **error);
 zend_result phar_detect_phar_fname_ext(const char *filename, size_t filename_len, const char **ext_str, size_t *ext_len, int executable, int for_create, int is_complete);
 zend_result phar_split_fname(const char *filename, size_t filename_len, char **arch, size_t *arch_len, char **entry, size_t *entry_len, int executable, int for_create);
 

ext/phar/phar_internal.h

@@ -2184,7 +2184,7 @@ static zend_object *phar_rename_archive(phar_archive_data **sphar, char *ext) /*
 		goto err_oldpath;
 	}
 
-	phar_flush_ex(phar, NULL, 1, &error);
+	phar_flush_ex(phar, NULL, true, &error);
 
 	if (error) {
 		zend_hash_str_del(&(PHAR_G(phar_fname_map)), newpath, phar->fname_len);
@@ -4133,7 +4133,7 @@ PHP_METHOD(Phar, delMetadata)
 }
 /* }}} */
 
-static zend_result phar_extract_file(bool overwrite, phar_entry_info *entry, char *dest, size_t dest_len, char **error) /* {{{ */
+ZEND_ATTRIBUTE_NONNULL static zend_result phar_extract_file(bool overwrite, phar_entry_info *entry, const zend_string *dest, char **error) /* {{{ */
 {
 	php_stream_statbuf ssb;
 	size_t len;
@@ -4161,7 +4161,7 @@ static zend_result phar_extract_file(bool overwrite, phar_entry_info *entry, cha
 	if (virtual_file_ex(&new_state, ZSTR_VAL(entry->filename), NULL, CWD_EXPAND) != 0 ||
 			new_state.cwd_length <= 1) {
 		if (EINVAL == errno && ZSTR_LEN(entry->filename) > 50) {
-			spprintf(error, 4096, "Cannot extract \"%.50s...\" to \"%s...\", extracted filename is too long for filesystem", ZSTR_VAL(entry->filename), dest);
+			spprintf(error, 4096, "Cannot extract \"%.50s...\" to \"%s...\", extracted filename is too long for filesystem", ZSTR_VAL(entry->filename), ZSTR_VAL(dest));
 		} else {
 			spprintf(error, 4096, "Cannot extract \"%s\", internal error", ZSTR_VAL(entry->filename));
 		}
@@ -4183,7 +4183,7 @@ static zend_result phar_extract_file(bool overwrite, phar_entry_info *entry, cha
 	}
 #endif
 
-	len = spprintf(&fullpath, 0, "%s/%s", dest, filename);
+	len = spprintf(&fullpath, 0, "%s/%s", ZSTR_VAL(dest), filename);
 
 	if (len >= MAXPATHLEN) {
 		/* truncate for error message */
@@ -4224,9 +4224,9 @@ static zend_result phar_extract_file(bool overwrite, phar_entry_info *entry, cha
 	slash = zend_memrchr(filename, '/', filename_len);
 
 	if (slash) {
-		fullpath[dest_len + (slash - filename) + 1] = '\0';
+		fullpath[ZSTR_LEN(dest) + (slash - filename) + 1] = '\0';
 	} else {
-		fullpath[dest_len] = '\0';
+		fullpath[ZSTR_LEN(dest)] = '\0';
 	}
 
 	if (FAILURE == php_stream_stat_path(fullpath, &ssb)) {
@@ -4248,9 +4248,9 @@ static zend_result phar_extract_file(bool overwrite, phar_entry_info *entry, cha
 	}
 
 	if (slash) {
-		fullpath[dest_len + (slash - filename) + 1] = '/';
+		fullpath[ZSTR_LEN(dest) + (slash - filename) + 1] = '/';
 	} else {
-		fullpath[dest_len] = '/';
+		fullpath[ZSTR_LEN(dest)] = '/';
 	}
 
 	filename = NULL;
@@ -4270,9 +4270,10 @@ static zend_result phar_extract_file(bool overwrite, phar_entry_info *entry, cha
 	}
 
 	if ((phar_get_fp_type(entry) == PHAR_FP && (entry->flags & PHAR_ENT_COMPRESSION_MASK)) || !phar_get_efp(entry, 0)) {
-		if (FAILURE == phar_open_entry_fp(entry, error, 1)) {
-			if (error) {
-				spprintf(error, 4096, "Cannot extract \"%s\" to \"%s\", unable to open internal file pointer: %s", ZSTR_VAL(entry->filename), fullpath, *error);
+		char *open_entry_error = NULL;
+		if (FAILURE == phar_open_entry_fp(entry, &open_entry_error, 1)) {
+			if (open_entry_error) {
+				spprintf(error, 4096, "Cannot extract \"%s\" to \"%s\", unable to open internal file pointer: %s", ZSTR_VAL(entry->filename), fullpath, open_entry_error);
 			} else {
 				spprintf(error, 4096, "Cannot extract \"%s\" to \"%s\", unable to open internal file pointer", ZSTR_VAL(entry->filename), fullpath);
 			}
@@ -4310,28 +4311,28 @@ static zend_result phar_extract_file(bool overwrite, phar_entry_info *entry, cha
 }
 /* }}} */
 
-static int extract_helper(phar_archive_data *archive, zend_string *search, char *pathto, size_t pathto_len, bool overwrite, char **error) { /* {{{ */
+ZEND_ATTRIBUTE_NONNULL_ARGS(1, 3, 5) static int extract_helper(const phar_archive_data *archive, zend_string *search, const zend_string *pathto, bool overwrite, char **error) { /* {{{ */
 	int extracted = 0;
 	phar_entry_info *entry;
 
 	if (!search) {
 		/* nothing to match -- extract all files */
 		ZEND_HASH_MAP_FOREACH_PTR(&archive->manifest, entry) {
-			if (FAILURE == phar_extract_file(overwrite, entry, pathto, pathto_len, error)) return -1;
+			if (FAILURE == phar_extract_file(overwrite, entry, pathto, error)) return -1;
 			extracted++;
 		} ZEND_HASH_FOREACH_END();
 	} else if (ZSTR_LEN(search) > 0 && '/' == ZSTR_VAL(search)[ZSTR_LEN(search) - 1]) {
 		/* ends in "/" -- extract all entries having that prefix */
 		ZEND_HASH_MAP_FOREACH_PTR(&archive->manifest, entry) {
 			if (!zend_string_starts_with(entry->filename, search)) continue;
-			if (FAILURE == phar_extract_file(overwrite, entry, pathto, pathto_len, error)) return -1;
+			if (FAILURE == phar_extract_file(overwrite, entry, pathto, error)) return -1;
 			extracted++;
 		} ZEND_HASH_FOREACH_END();
 	} else {
 		/* otherwise, looking for an exact match */
 		entry = zend_hash_find_ptr(&archive->manifest, search);
 		if (NULL == entry) return 0;
-		if (FAILURE == phar_extract_file(overwrite, entry, pathto, pathto_len, error)) return -1;
+		if (FAILURE == phar_extract_file(overwrite, entry, pathto, error)) return -1;
 		return 1;
 	}
 
@@ -4344,17 +4345,16 @@ PHP_METHOD(Phar, extractTo)
 {
 	php_stream *fp;
 	php_stream_statbuf ssb;
-	char *pathto;
+	zend_string *path_to;
 	zend_string *filename = NULL;
-	size_t pathto_len;
 	int ret;
 	zval *zval_file;
 	HashTable *files_ht = NULL;
 	bool overwrite = 0;
 	char *error = NULL;
 
 	ZEND_PARSE_PARAMETERS_START(1, 3)
-		Z_PARAM_PATH(pathto, pathto_len)
+		Z_PARAM_PATH_STR(path_to)
 		Z_PARAM_OPTIONAL
 		Z_PARAM_ARRAY_HT_OR_STR_OR_NULL(files_ht, filename)
 		Z_PARAM_BOOL(overwrite)
@@ -4372,30 +4372,30 @@ PHP_METHOD(Phar, extractTo)
 
 	php_stream_close(fp);
 
-	if (pathto_len < 1) {
+	if (ZSTR_LEN(path_to) < 1) {
 		zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0,
 			"Invalid argument, extraction path must be non-zero length");
 		RETURN_THROWS();
 	}
 
-	if (pathto_len >= MAXPATHLEN) {
-		char *tmp = estrndup(pathto, 50);
+	if (ZSTR_LEN(path_to) >= MAXPATHLEN) {
+		char *tmp = estrndup(ZSTR_VAL(path_to), 50);
 		/* truncate for error message */
 		zend_throw_exception_ex(spl_ce_InvalidArgumentException, 0, "Cannot extract to \"%s...\", destination directory is too long for filesystem", tmp);
 		efree(tmp);
 		RETURN_THROWS();
 	}
 
-	if (php_stream_stat_path(pathto, &ssb) < 0) {
-		ret = php_stream_mkdir(pathto, 0777,  PHP_STREAM_MKDIR_RECURSIVE, NULL);
+	if (php_stream_stat_path(ZSTR_VAL(path_to), &ssb) < 0) {
+		ret = php_stream_mkdir(ZSTR_VAL(path_to), 0777,  PHP_STREAM_MKDIR_RECURSIVE, NULL);
 		if (!ret) {
 			zend_throw_exception_ex(spl_ce_RuntimeException, 0,
-				"Unable to create path \"%s\" for extraction", pathto);
+				"Unable to create path \"%s\" for extraction", ZSTR_VAL(path_to));
 			RETURN_THROWS();
 		}
 	} else if (!(ssb.sb.st_mode & S_IFDIR)) {
 		zend_throw_exception_ex(spl_ce_RuntimeException, 0,
-			"Unable to use path \"%s\" for extraction, it is a file, must be a directory", pathto);
+			"Unable to use path \"%s\" for extraction, it is a file, must be a directory", ZSTR_VAL(path_to));
 		RETURN_THROWS();
 	}
 
@@ -4411,7 +4411,7 @@ PHP_METHOD(Phar, extractTo)
 					"Invalid argument, array of filenames to extract contains non-string value");
 				RETURN_THROWS();
 			}
-			switch (extract_helper(phar_obj->archive, Z_STR_P(zval_file), pathto, pathto_len, overwrite, &error)) {
+			switch (extract_helper(phar_obj->archive, Z_STR_P(zval_file), path_to, overwrite, &error)) {
 				case -1:
 					zend_throw_exception_ex(phar_ce_PharException, 0, "Extraction from phar \"%s\" failed: %s",
 						phar_obj->archive->fname, error);
@@ -4427,7 +4427,7 @@ PHP_METHOD(Phar, extractTo)
 		RETURN_TRUE;
 	}
 
-	ret = extract_helper(phar_obj->archive, filename, pathto, pathto_len, overwrite, &error);
+	ret = extract_helper(phar_obj->archive, filename, path_to, overwrite, &error);
 	if (-1 == ret) {
 		zend_throw_exception_ex(phar_ce_PharException, 0, "Extraction from phar \"%s\" failed: %s",
 			phar_obj->archive->fname, error);

ext/phar/phar_object.c

@@ -21,7 +21,7 @@ BEGIN_EXTERN_C()
 #include "ext/standard/url.h"
 
 php_url* phar_parse_url(php_stream_wrapper *wrapper, const char *filename, const char *mode, int options);
-void phar_entry_remove(phar_entry_data *idata, char **error);
+ZEND_ATTRIBUTE_NONNULL void phar_entry_remove(phar_entry_data *idata, char **error);
 
 static php_stream* phar_wrapper_open_url(php_stream_wrapper *wrapper, const char *path, const char *mode, int options, zend_string **opened_path, php_stream_context *context STREAMS_DC);
 static int phar_wrapper_rename(php_stream_wrapper *wrapper, const char *url_from, const char *url_to, int options, php_stream_context *context);