search

PCRE2Project / pcre2

PCRE2 development is now based here.
Other
922 stars 194 forks source link

Inconsistent behaviour for NULL zero length strings #270

Closed jagprog5 closed 1 year ago

jagprog5 commented 1 year ago

Commit 4ef0c51d2bbe1c572840cbb14c3080b1192e83d2 fixed this issue for substitution and matching, however pcre2_compile on latest still treats a null argument with zero length as an error.

The same fix should be applied to pcre2_compile, as I don't think this should generate an error.

carenas commented 1 year ago

Do you have a use case where this will be required, or is just for consistency?

Note that, unlike the other cases, there is syntactical differences, as the empty pattern ("") that would be used would match ALL subjects, and a NULL pattern was never considered valid (ex: regcomp would segfault, while we will return an error instead)

jagprog5 commented 1 year ago

I have a specific use case:

In my project, this test case was failing. See my associated fix here.

Pre-refactor, the test case passes a zero length null terminating string that points to the pattern. This calls pcre2_compile with non-null pointer pattern, and length PCRE2_ZERO_TERMINATED. It is handled correctly.

Post-refactor, the test case passes a pointer to an empty vector containing the pattern. This calls pcre2_compile with null pointer pattern and length 0. It throws an error.

In either case, both were holding the same data. One contained in a c-string, the other in a vector\<char>, both empty. These two cases should be treated the same, but they are not.

jagprog5 commented 1 year ago

Friendly ping!

I'd like to quote @PhilipHazel from the commit I mentioned above:

Apparently a number of applications treat NULL/0 in this way.

I feel that my use case cleanly aligns with this same reasoning. That being said, you mentioned outside context; I don't know the specifics of regcomp, and I don't know if there is a considerable user-base that expects this specific behavior. From my point of view it doesn't make sense, but I'm not special.

Please let me know either way.

PhilipHazel commented 1 year ago

I have no problem with this; I've just been busy with other things recently. At some point I will implement it.

PhilipHazel commented 1 year ago

HEAD now supports this. It took longer to invent a way of testing it than to make the change - oops, just remembered I haven't updated the pcre2test doc. Will do so right now.