pcre2_string_utils: avoid segfault with strlen(NULL) #55
Conversation
|
I don't think that's what we want, see my comment here. |
pcre2(?:_dfa)?_match should return PCRE2_ERROR_NULL if the subject is NULL, but the order that is done is incorrect, leading to crashes. Workaround the issue by allowing strlen(NULL) to return a value of 0, so it wouldn't segfault while trying to access a NULL subject and therefore allowing the current check to be reached even if it is done after the length of the subject is evaluated because it was provided as PCRE2_ZERO_TERMINATED. As a side effect, this also prevents crashes in pcre2_substitute when the subject or the replacement string were NULL and the length was provided as PCRE2_ZERO_TERMINATED and that would come out handy if we want to be able to allow a NULL subject as valid when a length of 0 was also provided.
(NULL, PCRE2_ZERO_TERMINATED) is an special case of (NULL, 0) as they will be equivalent if you consider (or are forced to, because your language doesn't differenciate NULL) that NULL and "" are somehow representing an "empty string". since from the discussion in #54 it would seem that there is no point on accomodating for that in the library, this is likely not needed as well, and specially because it is likely that most languages interfacing with the library would hit this already and have a workaround like the one you have in PCRE.NET, and which will need to be replicated inside the library if we were to fully implement that equivalence. |
I disagree. Here is my interpretation:
I don't consider that. They are definitely different. We apparently still have a misunderstanding about that, but I don't think it's worth pursuing it further.
Yes, let's drop this. It's not really worth the time we all already spent on this. |
|
After some thought, I do have some sympathy with ignoring the value of the pointer when the string length is given as 0. When I get round to looking at the code - possibly in a couple of weeks as there is no hurry and I'm on another project - I will certainly put in Carlo's crash avoidance patches. To support "length=0 means empty string" is perhaps just a matter of inserting lines like if (length == 0) subject = (appropriate cast)""; in the right places. I would have to see if that's OK at all code unit sizes. |
As the first step towards allowing the use of NULL together with a length of 0 as equivalent to "" for a subject as suggested in #54, but also as an alternative fix for the failures addressed by #53.