Repository navigation
strcspn() odd behaviour with NUL bytes and empty mask #12592
Description
Description
The following code:
<?php
var_dump(strcspn("abc\0def", " "));
var_dump(strcspn("abc\0def", ""));Resulted in this output:
int(7)
int(3)
But I expected this output instead:
int(7)
int(7)
Looking at the description of strcspn:
Returns the length of the initial segment of string which does not contain any of the characters in characters.
IOW, if the mask is empty, the stop condition is never met so the total length of the string must be returned, no?
I discovered this while I was working on an optimization for this function similar to what I did for strspn.
This has been wrong since 1999: a15916a
There are actually tests that check this behaviour, but I think that's just incidental as the test date from way later than 1999.
The reason this happens is because php_strcspn accesses *s2 even if it is empty, thus reading a NUL byte out of bounds and thinking that the NUL is part of the mask.
Fixing this is trivial however, although I wouldn't recommend doing this in stable branches as it is a behaviour change, although a weird one:
diff --git a/ext/standard/string.c b/ext/standard/string.c
index 8beedfe818..b5deb20403 100644
--- a/ext/standard/string.c
+++ b/ext/standard/string.c
@@ -1640,16 +1640,16 @@ PHPAPI size_t php_strcspn(const char *s1, const char *s2, const char *s1_end, co
const char *p, *spanp;
char c = *s1;
- for (p = s1;;) {
- spanp = s2;
- do {
- if (*spanp == c || p == s1_end) {
+ for (p = s1; p < s1_end;) {
+ for (spanp = s2; spanp < s2_end; spanp++) {
+ if (*spanp == c) {
return p - s1;
}
- } while (spanp++ < (s2_end - 1));
+ }
c = *++p;
}
- /* NOTREACHED */
+
+ return p - s1;
}
/* }}} */
I'd say: let's fix it only in master?
PHP Version
Every version since 1999
Operating System
No response
Activity
SakiTakamachi commentedon Nov 2, 2023
Doing something similar with a regular expression results in something like this:
Python regular expressions produce similar results.
It may be a good idea to firmly decide on a policy on how to handle empty characters when searching for characters.
strstr,strpos... there are many other functions that behave "strangely".Upon closer inspection, I may have missed the point a bit. If we respect the results of regular expressions, maybe we should expect something like this:
ndossche commentedon Nov 2, 2023
Neither of the regular expressions you posted do the same thing as strcspn.
strcspn searches the first position that has a letter in the mask.
So to give a concrete example:
strcspn("hello?", "l?")should output 2 becauselcomes before?inhello?at index 2.Similarly, if the mask is empty:
strcspn("hello", "")then it should return the length of the string because the mask is empty, i.e. the conditionfirst position that has a letter in the maskis false. It should thus behave the same way as if the mask contains letters that never occur in the haystack.The bug report here shows that an empty mask is actually equal to
"\0"due to an out-of-bound read (well, not outside of the buffer, but outside of the string contents into the NUL terminator). A non-empty mask never includes\0. Looking at the implementation reveals this is a mistake in the code because it uses a do-while loop.SakiTakamachi commentedon Nov 2, 2023
I see now.
I was thinking of it as a mask for the "empty string", which makes sense considering there is "no mask".
(If we expect the mask to include empty strings, this function will only return 0. It's my stupid mistake.)
Please forget what I said.
Girgias commentedon Nov 2, 2023
I think fixing this in master makes sense, the current behaviour is clearly a bug but needs an UPGRADING entry.