Skip to content

Commit 12e772f

Browse files
nikicnikic
committed
Promote substr_replace warnings
The implementation here was pretty confused. In reality the only error condition it has right now is that for a string input, from & length cannot be arrays. The fact that the array lengths are the same was probably supposed to be checked for the case of array input, as it wouldn't matter otherwise.
1 parent ade57e6 commit 12e772f

File tree

2 files changed

+73
-85
lines changed

2 files changed

+73
-85
lines changed

‎ext/standard/string.c

Lines changed: 60 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -2265,87 +2265,87 @@ PHP_FUNCTION(substr_replace)
22652265
}
22662266

22672267
if (str) {
2268-
if ((len_is_null && from_ht) || (!len_is_null && (from_ht == NULL) != (len_ht == NULL))) {
2269-
php_error_docref(NULL, E_WARNING, "'start' and 'length' should be of same type - numerical or array ");
2270-
RETURN_STR_COPY(str);
2268+
if (from_ht) {
2269+
zend_argument_type_error(3, "cannot be an array when working on a single string");
2270+
RETURN_THROWS();
22712271
}
2272-
if (!len_is_null && from_ht) {
2273-
if (zend_hash_num_elements(from_ht) != zend_hash_num_elements(len_ht)) {
2274-
php_error_docref(NULL, E_WARNING, "'start' and 'length' should have the same number of elements");
2275-
RETURN_STR_COPY(str);
2276-
}
2272+
if (len_ht) {
2273+
zend_argument_type_error(4, "cannot be an array when working on a single string");
2274+
RETURN_THROWS();
22772275
}
2278-
}
22792276

2280-
if (str) {
2281-
if (!from_ht) {
2282-
f = from_long;
2277+
f = from_long;
22832278

2284-
/* if "from" position is negative, count start position from the end
2285-
* of the string
2286-
*/
2279+
/* if "from" position is negative, count start position from the end
2280+
* of the string
2281+
*/
2282+
if (f < 0) {
2283+
f = (zend_long)ZSTR_LEN(str) + f;
22872284
if (f < 0) {
2288-
f = (zend_long)ZSTR_LEN(str) + f;
2289-
if (f < 0) {
2290-
f = 0;
2291-
}
2292-
} else if ((size_t)f > ZSTR_LEN(str)) {
2293-
f = ZSTR_LEN(str);
2285+
f = 0;
22942286
}
2295-
/* if "length" position is negative, set it to the length
2296-
* needed to stop that many chars from the end of the string
2297-
*/
2287+
} else if ((size_t)f > ZSTR_LEN(str)) {
2288+
f = ZSTR_LEN(str);
2289+
}
2290+
/* if "length" position is negative, set it to the length
2291+
* needed to stop that many chars from the end of the string
2292+
*/
2293+
if (l < 0) {
2294+
l = ((zend_long)ZSTR_LEN(str) - f) + l;
22982295
if (l < 0) {
2299-
l = ((zend_long)ZSTR_LEN(str) - f) + l;
2300-
if (l < 0) {
2301-
l = 0;
2302-
}
2296+
l = 0;
23032297
}
2298+
}
23042299

2305-
if ((size_t)l > ZSTR_LEN(str) || (l < 0 && (size_t)(-l) > ZSTR_LEN(str))) {
2306-
l = ZSTR_LEN(str);
2307-
}
2300+
if ((size_t)l > ZSTR_LEN(str) || (l < 0 && (size_t)(-l) > ZSTR_LEN(str))) {
2301+
l = ZSTR_LEN(str);
2302+
}
23082303

2309-
if ((f + l) > (zend_long)ZSTR_LEN(str)) {
2310-
l = ZSTR_LEN(str) - f;
2311-
}
2304+
if ((f + l) > (zend_long)ZSTR_LEN(str)) {
2305+
l = ZSTR_LEN(str) - f;
2306+
}
23122307

2313-
zend_string *tmp_repl_str = NULL;
2314-
if (repl_ht) {
2315-
repl_idx = 0;
2316-
while (repl_idx < repl_ht->nNumUsed) {
2317-
tmp_repl = &repl_ht->arData[repl_idx].val;
2318-
if (Z_TYPE_P(tmp_repl) != IS_UNDEF) {
2319-
break;
2320-
}
2321-
repl_idx++;
2322-
}
2323-
if (repl_idx < repl_ht->nNumUsed) {
2324-
repl_str = zval_get_tmp_string(tmp_repl, &tmp_repl_str);
2325-
} else {
2326-
repl_str = STR_EMPTY_ALLOC();
2308+
zend_string *tmp_repl_str = NULL;
2309+
if (repl_ht) {
2310+
repl_idx = 0;
2311+
while (repl_idx < repl_ht->nNumUsed) {
2312+
tmp_repl = &repl_ht->arData[repl_idx].val;
2313+
if (Z_TYPE_P(tmp_repl) != IS_UNDEF) {
2314+
break;
23272315
}
2316+
repl_idx++;
23282317
}
2318+
if (repl_idx < repl_ht->nNumUsed) {
2319+
repl_str = zval_get_tmp_string(tmp_repl, &tmp_repl_str);
2320+
} else {
2321+
repl_str = STR_EMPTY_ALLOC();
2322+
}
2323+
}
23292324

2330-
result = zend_string_safe_alloc(1, ZSTR_LEN(str) - l + ZSTR_LEN(repl_str), 0, 0);
2325+
result = zend_string_safe_alloc(1, ZSTR_LEN(str) - l + ZSTR_LEN(repl_str), 0, 0);
23312326

2332-
memcpy(ZSTR_VAL(result), ZSTR_VAL(str), f);
2333-
if (ZSTR_LEN(repl_str)) {
2334-
memcpy((ZSTR_VAL(result) + f), ZSTR_VAL(repl_str), ZSTR_LEN(repl_str));
2335-
}
2336-
memcpy((ZSTR_VAL(result) + f + ZSTR_LEN(repl_str)), ZSTR_VAL(str) + f + l, ZSTR_LEN(str) - f - l);
2337-
ZSTR_VAL(result)[ZSTR_LEN(result)] = '\0';
2338-
zend_tmp_string_release(tmp_repl_str);
2339-
RETURN_NEW_STR(result);
2340-
} else {
2341-
php_error_docref(NULL, E_WARNING, "Functionality of 'start' and 'length' as arrays is not implemented");
2342-
RETURN_STR_COPY(str);
2327+
memcpy(ZSTR_VAL(result), ZSTR_VAL(str), f);
2328+
if (ZSTR_LEN(repl_str)) {
2329+
memcpy((ZSTR_VAL(result) + f), ZSTR_VAL(repl_str), ZSTR_LEN(repl_str));
23432330
}
2331+
memcpy((ZSTR_VAL(result) + f + ZSTR_LEN(repl_str)), ZSTR_VAL(str) + f + l, ZSTR_LEN(str) - f - l);
2332+
ZSTR_VAL(result)[ZSTR_LEN(result)] = '\0';
2333+
zend_tmp_string_release(tmp_repl_str);
2334+
RETURN_NEW_STR(result);
23442335
} else { /* str is array of strings */
23452336
zend_string *str_index = NULL;
23462337
size_t result_len;
23472338
zend_ulong num_index;
23482339

2340+
/* TODO
2341+
if (!len_is_null && from_ht) {
2342+
if (zend_hash_num_elements(from_ht) != zend_hash_num_elements(len_ht)) {
2343+
php_error_docref(NULL, E_WARNING, "'start' and 'length' should have the same number of elements");
2344+
RETURN_STR_COPY(str);
2345+
}
2346+
}
2347+
*/
2348+
23492349
array_init(return_value);
23502350

23512351
from_idx = len_idx = repl_idx = 0;

‎ext/standard/tests/strings/substr_replace_error.phpt

Lines changed: 13 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -10,34 +10,22 @@ echo "*** Testing substr_replace() : error conditions ***\n";
1010

1111
$s1 = "Good morning";
1212

13-
echo "\n-- Testing substr_replace() function with start and length different types --\n";
14-
var_dump(substr_replace($s1, "evening", array(5)));
15-
var_dump(substr_replace($s1, "evening", 5, array(8)));
16-
17-
echo "\n-- Testing substr_replace() function with start and length with a different number of elements --\n";
18-
var_dump(substr_replace($s1, "evening", array(5, 1), array(8)));
19-
2013
echo "\n-- Testing substr_replace() function with start and length as arrays but string not--\n";
21-
var_dump(substr_replace($s1, "evening", array(5), array(8)));
14+
try {
15+
var_dump(substr_replace($s1, "evening", array(5)));
16+
} catch (TypeError $e) {
17+
echo $e->getMessage(), "\n";
18+
}
19+
try {
20+
var_dump(substr_replace($s1, "evening", 5, array(1)));
21+
} catch (TypeError $e) {
22+
echo $e->getMessage(), "\n";
23+
}
2224

2325
?>
24-
--EXPECTF--
26+
--EXPECT--
2527
*** Testing substr_replace() : error conditions ***
2628

27-
-- Testing substr_replace() function with start and length different types --
28-
29-
Warning: substr_replace(): 'start' and 'length' should be of same type - numerical or array in %s on line %d
30-
string(12) "Good morning"
31-
32-
Warning: substr_replace(): 'start' and 'length' should be of same type - numerical or array in %s on line %d
33-
string(12) "Good morning"
34-
35-
-- Testing substr_replace() function with start and length with a different number of elements --
36-
37-
Warning: substr_replace(): 'start' and 'length' should have the same number of elements in %s on line %d
38-
string(12) "Good morning"
39-
4029
-- Testing substr_replace() function with start and length as arrays but string not--
41-
42-
Warning: substr_replace(): Functionality of 'start' and 'length' as arrays is not implemented in %s on line %d
43-
string(12) "Good morning"
30+
substr_replace(): Argument #3 ($start) cannot be an array when working on a single string
31+
substr_replace(): Argument #4 ($length) cannot be an array when working on a single string

0 commit comments

Comments
 (0)