Capture original regex in non-matching group when an exact match is r…

[libklein]

Fixes #7776.

Tools::convetToRegex pre-/appends "^"/"$" to the built regex if an exact match is requested. This breaks expressions such as "v|", which, instead of matching v or the empty string, matches v or the end of line after substitution: "^v|$" is parsed as "^(v|$)".

Surrounding the original regex with an non-capturing group fixes this error.

See https://pcre.org/pcre.txt Sections "Vertical Bar" and "Subpatterns".

Testing strategy

Added test to TestFdoSecrets and TestTools.

Type of change

• ✅ Bug fix (non-breaking change that fixes an issue)

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 96.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 64.47%. Comparing base (6f20f0e) to head (b6d140c).
Report is 422 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/core/Tools.cpp	96.67%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #7778      +/-   ##
===========================================
+ Coverage    64.46%   64.47%   +0.01%     
===========================================
  Files          339      339              
  Lines        43610    43637      +27     
===========================================
+ Hits         28110    28133      +23     
- Misses       15500    15504       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

---

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests
• JS Bundle Analysis - Avoid shipping oversized bundles

[droidmonkey]

Was thinking this was where the error was, great change!

[Aetf]

Why not use QRegularExpression::escape to escape the string
if exact match is requested? The current fix breaks on inputs like )v|(?:

In some sense this is pretty similar to protecting against SQL injection...

Edit: there's also QRegularExpression::wildcardToRegularExpression.
But then I noticed both functions are only available in Qt 5.15...

[droidmonkey]

That is why lol, not available yet for our minimum distros. Does the Qt function choke on these "malicious" type strings as well?

[Aetf]

I haven't tried but I don't think so.

Here's the implementation, we should do something similar

https://code.woboq.org/qt5/qtbase/src/corelib/text/qregularexpression.cpp.html#1899

https://code.woboq.org/qt5/qtbase/src/corelib/text/qregularexpression.cpp.html#1818

[droidmonkey]

I think I prefer just copying the 5.15 source code into our tools with attribution.

[libklein]

I agree and will work on that and some further tests later tonight to hopefully make 2.7.1.

[libklein]

That is why lol, not available yet for our minimum distros. Does the Qt function choke on these "malicious" type strings as well?

Actually, the QT implementation of exact matches (https://doc.qt.io/qt-5/qregularexpression.html#anchoredPattern) does exactly the same thing were doing right now. The "malicious string" is malicious only because it is not a proper regular expression, which is something Tools::convertToRegex should require.

As we'd need the copy the existing implementation (https://github.com/qt/qtbase/blob/2eb7a92aa373f10f4e6828dad640b393279637a3/src/corelib/text/qregularexpression.cpp#L2053, available > qt 5.12) into our source anyway, i'd just leave the function as is.

I'd still implement QRegularExpression::escape (and expose it), simply because it allows us to granularize tests. See the latest commit. I'll add further tests if you agree with the approach.

Also, does someone know if the attribution is correct?

[droidmonkey]

Should invoke the GPL 3 statements from their license block at the top of the qregularexpression.cpp.

[libklein]

I've fixed some more bugs with overlapping flag values, added some further tests, and the QT license header.

After spending some time browsing through the codebase i'd suggest splitting convertToRegex into createWildcardRegex and createRegex (perhaps even get rid of the latter one) as these are orthogonal anyway.