chore: probe for librrd's argv constness

.github/workflows/ci.yml

@@ -13,10 +13,16 @@ jobs:
   build:
     name: PHP ${{ matrix.php }}
     runs-on: ubuntu-latest
+    continue-on-error: ${{ matrix.experimental || false }}
     strategy:
       fail-fast: false
       matrix:
         php: ['8.1', '8.2', '8.3', '8.4', '8.5']
+        include:
+          # nightly build of php-src master; allowed to fail so an unrelated
+          # upstream break does not gate pull requests
+          - php: '8.6'
+            experimental: true
 
     steps:
       - name: Checkout
@@ -25,7 +31,7 @@ jobs:
       - name: Install librrd
         run: |
           sudo apt-get update
-          sudo apt-get install -y librrd-dev pkg-config
+          sudo apt-get install -y librrd-dev pkg-config rrdtool
 
       - name: Setup PHP
         uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # v2.37.2
@@ -40,6 +46,9 @@ jobs:
           ./configure --with-rrd
           make -j"$(nproc)"
 
+      - name: Generate test fixtures
+        run: make -C tests/data all
+
       - name: Test
         run: make test
         env:

.gitignore

@@ -29,3 +29,19 @@ modules
 run-tests.php
 tmp-php.ini
 tests/rrdtool-bin.inc
+*.dep
+configure.ac
+tests/*.diff
+tests/*.exp
+tests/*.log
+tests/*.out
+tests/*.php
+tests/*.sh
+tests/*.txt
+tests/*.png
+tests/*.rrd
+tests/data/*.png
+tests/data/*.rrd
+tests/data/*.txt
+tests/data/*.xml
+tests/data/Makefile

config.m4

@@ -30,6 +30,20 @@ if test "$PHP_RRD" != "no"; then
     AC_MSG_ERROR(pkgconfig and librrd in version >= 1.3.0 must be installed)
   fi
 
+  dnl librrd took char ** for argv before 1.9 and const char ** from 1.9 on
+  AC_MSG_CHECKING(whether librrd takes const char ** argv)
+  rrd_save_CFLAGS="$CFLAGS"
+  CFLAGS="$CFLAGS $LIBRRD_CFLAGS -Werror"
+  AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
+    [[#include <rrd.h>]],
+    [[const char *argv[2] = { "create", 0 }; rrd_create(1, argv);]])],
+    [
+      AC_MSG_RESULT(yes)
+      AC_DEFINE(HAVE_RRD_CONST_ARGV, 1, [librrd takes const char ** argv])
+    ],
+    [AC_MSG_RESULT(no)])
+  CFLAGS="$rrd_save_CFLAGS"
+
   dnl rrd_lastupdate_r available in 1.4.0+
   AC_CHECK_LIB([rrd], [rrd_lastupdate_r],
   [ 

php_rrd.h

@@ -24,6 +24,13 @@ extern zend_module_entry rrd_module_entry;
 # define zend_parse_parameters_none() zend_parse_parameters(ZEND_NUM_ARGS(), "")
 #endif 
 
+/* librrd took char ** for argv before 1.9 and const char ** from 1.9 on */
+#ifdef HAVE_RRD_CONST_ARGV
+# define RRD_ARGV(argv) ((const char **)(argv))
+#else
+# define RRD_ARGV(argv) (argv)
+#endif
+
 typedef struct _rrd_args {
 	int count;
 	char **args;

rrd.c

@@ -61,6 +61,7 @@ PHP_FUNCTION(rrd_fetch)
 	time_t start, end;
 	unsigned long step,
 	ds_cnt; /* count of data sources */
+	unsigned ds_counter;
 	char **ds_namv; /* list of data source names */
 	rrd_value_t *ds_data; /* all data from all sources */
 
@@ -80,7 +81,7 @@ PHP_FUNCTION(rrd_fetch)
 	if (rrd_test_error()) rrd_clear_error();
 
 	/* call rrd_fetch and test if fails */
-	if (rrd_fetch(argv->count - 1, &argv->args[1], &start, &end, &step, &ds_cnt,
+	if (rrd_fetch(argv->count - 1, RRD_ARGV(&argv->args[1]), &start, &end, &step, &ds_cnt,
 		&ds_namv, &ds_data) == -1 ) {
 		rrd_args_free(argv);
 		RETURN_FALSE;
@@ -95,11 +96,11 @@ PHP_FUNCTION(rrd_fetch)
 	/* add "ds_namv" and "data" array into return values if there is any
 	 * result data
 	 */
-	if (!ds_data || !ds_namv || !ds_cnt) {
+	if (!ds_data || !ds_namv || !ds_cnt || !step) {
 		add_assoc_null(return_value, "data");
 	} else {
 		rrd_value_t *datap = ds_data;
-		unsigned timestamp, ds_counter;
+		time_t timestamp;
 		/* final array for all data from all data sources */
 		zval zv_data_array;
 
@@ -124,20 +125,26 @@ PHP_FUNCTION(rrd_fetch)
 				/* pointer for one data source retrieved data */
 				zval *ds_data_array;
 				/* value for key (timestamp) in data array */
-				char str_timestamp[11];
-				ZEND_LTOA((zend_ulong)timestamp, str_timestamp, sizeof(str_timestamp));
+				/* time_t is wider than zend_long on 32-bit builds, so
+				   this cannot go through ZEND_LONG_FMT */
+				char str_timestamp[24];
+				snprintf(str_timestamp, sizeof(str_timestamp),
+					"%" PRId64, (int64_t)timestamp);
 
-				/* gets pointer for data source result array */
+					/* gets pointer for data source result array */
 				ds_data_array = zend_hash_get_current_data(Z_ARRVAL(zv_data_array));
+				if (!ds_data_array) break;
 
 				add_assoc_double(ds_data_array, str_timestamp, *(datap++));
 				zend_hash_move_forward(Z_ARRVAL(zv_data_array));
 			}
 		}
 		add_assoc_zval(return_value, "data", &zv_data_array);
+	}
 
-		/* free data from rrd_fetch */
-		free(ds_data);
+	/* free data from rrd_fetch */
+	free(ds_data);
+	if (ds_namv) {
 		for (ds_counter = 0; ds_counter < ds_cnt; ds_counter++) {
 			free(ds_namv[ds_counter]);
 		}
@@ -249,7 +256,7 @@ PHP_FUNCTION(rrd_lastupdate)
 	if (rrd_lastupdate_r(argv[2], &last_update, &ds_cnt, &ds_namv,
 		&last_ds) == -1) {
 #else
- 	if (rrd_lastupdate(2, &argv[1], &last_update, &ds_cnt, &ds_namv,
+ 	if (rrd_lastupdate(2, RRD_ARGV(&argv[1]), &last_update, &ds_cnt, &ds_namv,
  		&last_ds) == -1) {
 #endif
 		efree(argv[2]); efree(argv[1]);
@@ -334,19 +341,19 @@ PHP_FUNCTION(rrd_restore)
 	argv = rrd_args_init_by_phparray("restore", xml_filename, &zv_options);
 	if (!argv) {
 		zend_error(E_WARNING, "cannot allocate arguments options");
-		zval_dtor(&zv_options);
+		zval_ptr_dtor_nogc(&zv_options);
 		RETURN_FALSE;
 	}
 
 	if (rrd_test_error()) rrd_clear_error();
 
 	/* call rrd_ restore and test if fails */
-	if (rrd_restore(argv->count-1, &argv->args[1]) == -1) {
+	if (rrd_restore(argv->count-1, RRD_ARGV(&argv->args[1])) == -1) {
 		RETVAL_FALSE;
 	} else {
 		RETVAL_TRUE;
 	}
-	zval_dtor(&zv_options);
+	zval_ptr_dtor_nogc(&zv_options);
 	rrd_args_free(argv);
 }
 /* }}} */
@@ -381,7 +388,7 @@ PHP_FUNCTION(rrd_tune)
 	if (rrd_test_error()) rrd_clear_error();
 
 	/* call rrd_tune and test if fails */
-	if (rrd_tune(argv->count-1, &argv->args[1]) == -1 ) {
+	if (rrd_tune(argv->count-1, RRD_ARGV(&argv->args[1])) == -1 ) {
 		RETVAL_FALSE;
 	} else {
 		RETVAL_TRUE;
@@ -419,7 +426,7 @@ PHP_FUNCTION(rrd_xport)
 	if (rrd_test_error()) rrd_clear_error();
 
 	/* call rrd_xport and test if fails */
-	if (rrd_xport(argv->count-1, &argv->args[1], &xxsize, &start, &end, &step,
+	if (rrd_xport(argv->count-1, RRD_ARGV(&argv->args[1]), &xxsize, &start, &end, &step,
 		&outvar_count, &legend_v, &data) == -1) {
 		php_printf("rrd_xport failed");
 		rrd_args_free(argv);
@@ -436,8 +443,15 @@ PHP_FUNCTION(rrd_xport)
 	add_assoc_long(return_value, "step", step);
 
 	/* no data available */
-	if (!data) {
+	if (!data || !step) {
 		add_assoc_null(return_value, "data");
+		if (legend_v) {
+			for (outvar_index = 0; outvar_index < outvar_count; outvar_index++) {
+				free(legend_v[outvar_index]);
+			}
+			free(legend_v);
+		}
+		free(data);
 		return;
 	}
 
@@ -463,8 +477,9 @@ PHP_FUNCTION(rrd_xport)
 		data_ptr = data + outvar_index;
 		for (time_index = start + step; time_index <= end; time_index += step) {
 			/* value for key (timestamp) in data array */
-			char str_timestamp[11];
-			ZEND_LTOA((zend_ulong)time_index, str_timestamp, sizeof(str_timestamp));
+			char str_timestamp[24];
+			snprintf(str_timestamp, sizeof(str_timestamp),
+				"%" PRId64, (int64_t)time_index);
 
 			add_assoc_double(&time_data, str_timestamp, *data_ptr);
 			data_ptr += outvar_count;
@@ -664,13 +679,21 @@ rrd_args *rrd_args_init_by_phparray(const char *command_name, const char *filena
 	zend_hash_internal_pointer_reset(Z_ARRVAL_P(options));
 	for (i=0; i < option_count; i++) {
 		zval *item;
+		zend_string *item_str;
 		smart_string option = {0}; /* one argument option */
 
 		/* force using strings as array items */
 		item = zend_hash_get_current_data(Z_ARRVAL_P(options));
-		if (Z_TYPE_P(item) != IS_STRING) convert_to_string(item);
-		smart_string_appendl(&option, Z_STRVAL_P(item), Z_STRLEN_P(item));
+		item_str = zval_try_get_string(item);
+		if (!item_str) {
+			smart_string_free(&option);
+			result->count = args_counter;
+			rrd_args_free(result);
+			return NULL;
+		}
+		smart_string_appendl(&option, ZSTR_VAL(item_str), ZSTR_LEN(item_str));
 		smart_string_0(&option);
+		zend_string_release(item_str);
 
 		result->args[args_counter++] = estrdup(option.c);
 		smart_string_free(&option);

rrd_create.c

@@ -47,7 +47,7 @@ typedef struct _rrd_create_object {
  * fetch our custom object from user space object
  */
 static inline rrd_create_object *php_rrd_create_fetch_object(zend_object *obj) {
-	return (rrd_create_object *)((char*)(obj) - XtOffsetOf(rrd_create_object, std));
+	return (rrd_create_object *)((char*)(obj) - offsetof(rrd_create_object, std));
 } 
 
 /* {{{ rrd_create_object_dtor
@@ -63,11 +63,11 @@ static void rrd_create_object_dtor(zend_object *object)
 	if (intern_obj->start_time)
 		efree(intern_obj->start_time);
 	if (!Z_ISUNDEF(intern_obj->zv_step))
-		zval_dtor(&intern_obj->zv_step);
+		zval_ptr_dtor_nogc(&intern_obj->zv_step);
 	if (!Z_ISUNDEF(intern_obj->zv_arr_data_sources))
-		zval_dtor(&intern_obj->zv_arr_data_sources);
+		zval_ptr_dtor_nogc(&intern_obj->zv_arr_data_sources);
 	if (!Z_ISUNDEF(intern_obj->zv_arr_archives))
-		zval_dtor(&intern_obj->zv_arr_archives);
+		zval_ptr_dtor_nogc(&intern_obj->zv_arr_archives);
 
 	zend_object_std_dtor(&intern_obj->std);
 }
@@ -104,7 +104,7 @@ PHP_METHOD(RRDCreator, __construct)
 	rrd_create_object *intern_obj;
 	char *path; size_t path_length;
 	/* better to set defaults for optional parameters */
-	zend_string *start_time;
+	zend_string *start_time = NULL;
 	long step = 0;
 	int argc = ZEND_NUM_ARGS();
 
@@ -131,8 +131,27 @@ PHP_METHOD(RRDCreator, __construct)
 	}
 
 	intern_obj = php_rrd_create_fetch_object(Z_OBJ_P(getThis()));
+	if (intern_obj->file_path) efree(intern_obj->file_path);
+	intern_obj->file_path = NULL;
+	if (intern_obj->start_time) efree(intern_obj->start_time);
+	intern_obj->start_time = NULL;
+
+	/* a second __construct otherwise inherits the previous step and sources */
+	if (!Z_ISUNDEF(intern_obj->zv_step)) {
+		zval_ptr_dtor_nogc(&intern_obj->zv_step);
+		ZVAL_UNDEF(&intern_obj->zv_step);
+	}
+	if (!Z_ISUNDEF(intern_obj->zv_arr_data_sources)) {
+		zval_ptr_dtor_nogc(&intern_obj->zv_arr_data_sources);
+		ZVAL_UNDEF(&intern_obj->zv_arr_data_sources);
+	}
+	if (!Z_ISUNDEF(intern_obj->zv_arr_archives)) {
+		zval_ptr_dtor_nogc(&intern_obj->zv_arr_archives);
+		ZVAL_UNDEF(&intern_obj->zv_arr_archives);
+	}
+
 	intern_obj->file_path = estrdup(path);
-	if (start_time) intern_obj->start_time = estrdup(ZSTR_VAL(start_time));
+	if (start_time) intern_obj->start_time = estrndup(ZSTR_VAL(start_time), ZSTR_LEN(start_time));
 	if (step) {
 		ZVAL_LONG(&intern_obj->zv_step, step);
 	}
@@ -218,6 +237,15 @@ PHP_METHOD(RRDCreator, save)
 	zval zv_create_argv;
 	rrd_args *create_argv;
 
+	if (!intern_obj->file_path) {
+		zend_throw_exception(NULL, "the object was not constructed", 0);
+		return;
+	}
+
+	if (php_check_open_basedir(intern_obj->file_path)) {
+		RETURN_FALSE;
+	}
+
 	array_init(&zv_create_argv);
 
 	if (intern_obj->start_time) {
@@ -249,21 +277,25 @@ PHP_METHOD(RRDCreator, save)
 	}
 
 	/* add array of archive and data source strings into argument list */
-	php_array_merge(Z_ARRVAL(zv_create_argv), Z_ARRVAL(intern_obj->zv_arr_data_sources));
-	php_array_merge(Z_ARRVAL(zv_create_argv), Z_ARRVAL(intern_obj->zv_arr_archives));
+	if (!Z_ISUNDEF(intern_obj->zv_arr_data_sources)) {
+		php_array_merge(Z_ARRVAL(zv_create_argv), Z_ARRVAL(intern_obj->zv_arr_data_sources));
+	}
+	if (!Z_ISUNDEF(intern_obj->zv_arr_archives)) {
+		php_array_merge(Z_ARRVAL(zv_create_argv), Z_ARRVAL(intern_obj->zv_arr_archives));
+	}
 
 	create_argv = rrd_args_init_by_phparray("create", intern_obj->file_path, &zv_create_argv);
 	if (!create_argv) {
 		zend_error(E_WARNING, "cannot allocate arguments options");
-		zval_dtor(&zv_create_argv);
+		zval_ptr_dtor_nogc(&zv_create_argv);
 		RETURN_FALSE;
 	}
 
 	if (rrd_test_error()) rrd_clear_error();
 
 	/* call rrd_create and test if fails */
-	if (rrd_create(create_argv->count - 1, &create_argv->args[1]) == -1) {
-		zval_dtor(&zv_create_argv);
+	if (rrd_create(create_argv->count - 1, RRD_ARGV(&create_argv->args[1])) == -1) {
+		zval_ptr_dtor_nogc(&zv_create_argv);
 		rrd_args_free(create_argv);
 
 		/* throw exception with rrd error string */
@@ -272,7 +304,7 @@ PHP_METHOD(RRDCreator, save)
 		return;
 	}
 
-	zval_dtor(&zv_create_argv);
+	zval_ptr_dtor_nogc(&zv_create_argv);
 	rrd_args_free(create_argv);
 	RETURN_TRUE;
 }
@@ -303,7 +335,7 @@ PHP_FUNCTION(rrd_create)
 
 	if (rrd_test_error()) rrd_clear_error();
 
-	if (rrd_create(argv->count - 1, &argv->args[1]) == -1 ) {
+	if (rrd_create(argv->count - 1, RRD_ARGV(&argv->args[1])) == -1 ) {
 		RETVAL_FALSE;
 	} else {
 		RETVAL_TRUE;
@@ -345,6 +377,6 @@ void rrd_create_minit()
 
 	memcpy(&rrd_create_handlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers));
 	rrd_create_handlers.clone_obj = NULL;
-	rrd_create_handlers.offset = XtOffsetOf(rrd_create_object, std);
+	rrd_create_handlers.offset = offsetof(rrd_create_object, std);
 	rrd_create_handlers.free_obj = rrd_create_object_dtor; 
 }