From: Nikita Popov <nikita.ppv@gmail.com>
Date: Thu, 15 Nov 2018 20:05:05 +0000 (+0100)
Subject: Check for zero SplPriorityQueue extract flags earlier
X-Git-Tag: php-7.4.0alpha1~1533
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=022eea8be1d0a49446b1b52c3f4719717234ccd8;p=php

Check for zero SplPriorityQueue extract flags earlier

Generate an exception during the SplPriorityQueue::setExtracFlags()
call instead of generating E_RECOVERABLE_FATAL all over the place
later.
---

diff --git a/UPGRADING b/UPGRADING
index 2d9299f883..2e49c7d641 100644
--- a/UPGRADING
+++ b/UPGRADING
@@ -46,6 +46,9 @@ PHP 7.4 UPGRADE NOTES
     (array) casts are *not* affected. They will continue to return either the
     wrapped array, or the ArrayObject properties, depending on whether the
     STD_PROP_LIST flag is used.
+  . SplPriorityQueue::setExtractFlags() will throw an exception if zero is
+    passed. Previously this would generate a recoverable fatal error on the
+    next extraction operation.
 
 ========================================
 2. New Features
diff --git a/ext/spl/spl_heap.c b/ext/spl/spl_heap.c
index 1050b732bf..8e644df76e 100644
--- a/ext/spl/spl_heap.c
+++ b/ext/spl/spl_heap.c
@@ -137,27 +137,27 @@ static int spl_ptr_heap_cmp_cb_helper(zval *object, spl_heap_object *heap_object
 }
 /* }}} */
 
-static int spl_pqueue_extract_helper(zval *result, zval *value, int flags) /* {{{ */
+static void spl_pqueue_extract_helper(zval *result, zval *value, int flags) /* {{{ */
 {
 	spl_pqueue_elem *elem = Z_PTR_P(value);
 	if ((flags & SPL_PQUEUE_EXTR_BOTH) == SPL_PQUEUE_EXTR_BOTH) {
 		array_init(result);
 		add_assoc_zval_ex(result, "data", sizeof("data") - 1, &elem->data);
 		add_assoc_zval_ex(result, "priority", sizeof("priority") - 1, &elem->priority);
-		return SUCCESS;
+		return;
 	}
 
 	if (flags & SPL_PQUEUE_EXTR_DATA) {
 		ZVAL_COPY(result, &elem->data);
-		return SUCCESS;
+		return;
 	}
 
 	if (flags & SPL_PQUEUE_EXTR_PRIORITY) {
 		ZVAL_COPY(result, &elem->priority);
-		return SUCCESS;
+		return;
 	}
 
-	return FAILURE;
+	ZEND_ASSERT(0);
 }
 /* }}} */
 
@@ -701,11 +701,7 @@ SPL_METHOD(SplPriorityQueue, extract)
 		return;
 	}
 
-	if (spl_pqueue_extract_helper(return_value, &value, intern->flags) == FAILURE) {
-		zend_error(E_RECOVERABLE_ERROR, "Unable to extract from the PriorityQueue node");
-		spl_ptr_heap_pqueue_elem_dtor(&value);
-		return;
-	}
+	spl_pqueue_extract_helper(return_value, &value, intern->flags);
 }
 /* }}} */
 
@@ -734,9 +730,7 @@ SPL_METHOD(SplPriorityQueue, top)
 		return;
 	}
 
-	if (spl_pqueue_extract_helper(return_value, value, intern->flags) == FAILURE) {
-		zend_error(E_RECOVERABLE_ERROR, "Unable to extract from the PriorityQueue node");
-	}
+	spl_pqueue_extract_helper(return_value, value, intern->flags);
 }
 /* }}} */
 
@@ -752,10 +746,14 @@ SPL_METHOD(SplPriorityQueue, setExtractFlags)
 		return;
 	}
 
-	intern = Z_SPLHEAP_P(ZEND_THIS);
-
-	intern->flags = value & SPL_PQUEUE_EXTR_MASK;
+	value &= SPL_PQUEUE_EXTR_MASK;
+	if (!value) {
+		zend_throw_exception(spl_ce_RuntimeException, "Must specify at least one extract flag", 0);
+		return;
+	}
 
+	intern = Z_SPLHEAP_P(ZEND_THIS);
+	intern->flags = value;
 	RETURN_LONG(intern->flags);
 }
 /* }}} */
@@ -935,9 +933,7 @@ static zval *spl_pqueue_it_get_current_data(zend_object_iterator *iter) /* {{{ *
 		return NULL;
 	}
 	if (Z_ISUNDEF(user_it->value)) {
-		if (spl_pqueue_extract_helper(&user_it->value, element, object->flags) == FAILURE) {
-			zend_error(E_RECOVERABLE_ERROR, "Unable to extract from the PriorityQueue node");
-		}
+		spl_pqueue_extract_helper(&user_it->value, element, object->flags);
 	}
 	return &user_it->value;
 }
@@ -1057,10 +1053,7 @@ SPL_METHOD(SplPriorityQueue, current)
 	if (!intern->heap->count || Z_ISUNDEF_P(element)) {
 		RETURN_NULL();
 	} else {
-		if (spl_pqueue_extract_helper(return_value, element, intern->flags) == FAILURE) {
-			zend_error(E_RECOVERABLE_ERROR, "Unable to extract from the PriorityQueue node");
-			RETURN_NULL();
-		}
+		spl_pqueue_extract_helper(return_value, element, intern->flags);
 	}
 }
 /* }}} */
diff --git a/ext/spl/tests/SplPriorityQueue_setExtractFlags_zero.phpt b/ext/spl/tests/SplPriorityQueue_setExtractFlags_zero.phpt
new file mode 100644
index 0000000000..cf2d339914
--- /dev/null
+++ b/ext/spl/tests/SplPriorityQueue_setExtractFlags_zero.phpt
@@ -0,0 +1,15 @@
+--TEST--
+Setting SplPriorityQueue extract flags to zero generates an exception
+--FILE--
+<?php
+
+$queue = new SplPriorityQueue();
+$queue->setExtractFlags(0);
+
+?>
+--EXPECTF--
+Fatal error: Uncaught RuntimeException: Must specify at least one extract flag in %s:%d
+Stack trace:
+#0 %s(%d): SplPriorityQueue->setExtractFlags(0)
+#1 {main}
+  thrown in %s on line %d