phar: Simplify phar_open_archive_fp() (#20753)

By returning the stream directly, we avoid calling some helpers
functions and it becomes more clear on what stream the code actually
acts upon.

Author: ndossche

ext/phar/phar.c

@@ -2310,28 +2310,29 @@ ZEND_ATTRIBUTE_NONNULL zend_result phar_postprocess_file(phar_entry_data *idata,
 		/* verify local file header */
 		phar_zip_file_header local;
 		phar_zip_data_desc desc;
+		php_stream *stream = phar_open_archive_fp(idata->phar);
 
-		if (SUCCESS != phar_open_archive_fp(idata->phar)) {
+		if (!stream) {
 			spprintf(error, 0, "phar error: unable to open zip-based phar archive \"%s\" to verify local file header for file \"%s\"",
 				idata->phar->fname, ZSTR_VAL(entry->filename));
 			return FAILURE;
 		}
-		php_stream_seek(phar_get_entrypfp(idata->internal_file), entry->header_offset, SEEK_SET);
+		php_stream_seek(stream, entry->header_offset, SEEK_SET);
 
-		if (sizeof(local) != php_stream_read(phar_get_entrypfp(idata->internal_file), (char *) &local, sizeof(local))) {
+		if (sizeof(local) != php_stream_read(stream, (char *) &local, sizeof(local))) {
 			spprintf(error, 0, "phar error: internal corruption of zip-based phar \"%s\" (cannot read local file header for file \"%s\")",
 				idata->phar->fname, ZSTR_VAL(entry->filename));
 			return FAILURE;
 		}
 
 		/* check for data descriptor */
 		if (((PHAR_ZIP_16(local.flags)) & 0x8) == 0x8) {
-			php_stream_seek(phar_get_entrypfp(idata->internal_file),
+			php_stream_seek(stream,
 					entry->header_offset + sizeof(local) +
 					PHAR_ZIP_16(local.filename_len) +
 					PHAR_ZIP_16(local.extra_len) +
 					entry->compressed_filesize, SEEK_SET);
-			if (sizeof(desc) != php_stream_read(phar_get_entrypfp(idata->internal_file),
+			if (sizeof(desc) != php_stream_read(stream,
 							    (char *) &desc, sizeof(desc))) {
 				spprintf(error, 0, "phar error: internal corruption of zip-based phar \"%s\" (cannot read local data descriptor for file \"%s\")",
 					idata->phar->fname, ZSTR_VAL(entry->filename));

ext/phar/phar_internal.h

@@ -440,7 +440,7 @@ php_stream *phar_get_efp(phar_entry_info *entry, bool 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, bool follow_links);
 phar_entry_info *phar_get_link_source(phar_entry_info *entry);
-zend_result phar_open_archive_fp(phar_archive_data *phar);
+php_stream *phar_open_archive_fp(phar_archive_data *phar);
 zend_result phar_copy_on_write(phar_archive_data **pphar);
 
 /* tar functions in tar.c */

ext/phar/stream.c

@@ -254,13 +254,13 @@ static php_stream * phar_wrapper_open_url(php_stream_wrapper *wrapper, const cha
 			} else {
 				php_stream *stream = phar_get_pharfp(phar);
 				if (stream == NULL) {
-					if (UNEXPECTED(FAILURE == phar_open_archive_fp(phar))) {
+					stream = phar_open_archive_fp(phar);
+					if (UNEXPECTED(!stream)) {
 						php_stream_wrapper_log_error(wrapper, options, "phar error: could not reopen phar \"%s\"", ZSTR_VAL(resource->host));
 						efree(internal_file);
 						php_url_free(resource);
 						return NULL;
 					}
-					stream = phar_get_pharfp(phar);
 				}
 
 				phar_entry_info *entry;

ext/phar/util.c

@@ -107,11 +107,12 @@ php_stream *phar_get_efp(phar_entry_info *entry, bool follow_links) /* {{{ */
 	}
 
 	if (phar_get_fp_type(entry) == PHAR_FP) {
-		if (!phar_get_entrypfp(entry)) {
+		php_stream *stream = phar_get_entrypfp(entry);
+		if (!stream) {
 			/* re-open just in time for cases where our refcount reached 0 on the phar archive */
-			phar_open_archive_fp(entry->phar);
+			stream = phar_open_archive_fp(entry->phar);
 		}
-		return phar_get_entrypfp(entry);
+		return stream;
 	} else if (phar_get_fp_type(entry) == PHAR_UFP) {
 		return phar_get_entrypufp(entry);
 	} else if (entry->fp_type == PHAR_MOD) {
@@ -708,24 +709,23 @@ static inline void phar_set_pharfp(phar_archive_data *phar, php_stream *fp)
 	PHAR_G(cached_fp)[phar->phar_pos].fp = fp;
 }
 
-/* initialize a phar_archive_data's read-only fp for existing phar data */
-zend_result phar_open_archive_fp(phar_archive_data *phar) /* {{{ */
+/* Initialize a phar_archive_data's read-only fp for existing phar data.
+ * The stream is owned by the `phar` object and must not be closed manually. */
+php_stream *phar_open_archive_fp(phar_archive_data *phar) /* {{{ */
 {
-	if (phar_get_pharfp(phar)) {
-		return SUCCESS;
+	php_stream *stream = phar_get_pharfp(phar);
+	if (stream) {
+		return stream;
 	}
 
 	if (php_check_open_basedir(phar->fname)) {
-		return FAILURE;
+		return NULL;
 	}
 
-	phar_set_pharfp(phar, php_stream_open_wrapper(phar->fname, "rb", IGNORE_URL|STREAM_MUST_SEEK|0, NULL));
-
-	if (!phar_get_pharfp(phar)) {
-		return FAILURE;
-	}
+	stream = php_stream_open_wrapper(phar->fname, "rb", IGNORE_URL|STREAM_MUST_SEEK, NULL);
+	phar_set_pharfp(phar, stream);
 
-	return SUCCESS;
+	return stream;
 }
 /* }}} */
 
@@ -829,7 +829,7 @@ ZEND_ATTRIBUTE_NONNULL zend_result phar_open_entry_fp(phar_entry_info *entry, ch
 	}
 
 	if (!phar_get_pharfp(phar)) {
-		if (FAILURE == phar_open_archive_fp(phar)) {
+		if (!phar_open_archive_fp(phar)) {
 			spprintf(error, 4096, "phar error: Cannot open phar archive \"%s\" for reading", phar->fname);
 			return FAILURE;
 		}