-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix regex catastrophic backtracking #269
fix regex catastrophic backtracking #269
Conversation
7b6ee32
to
5e7e3b9
Compare
Note to self. Algorithm idea to fix this last case ("someone puts a category in the wrong place (not at the bottom of the article)"). Will code this up later and add to patch (and will add some more test cases): let textBetweenFirstCategoryAndEndOfFile = wikitext.match(/\[\[:?Category:.*$/);
// delete categories from sampled text
textBetweenFirstCategoryAndEndOfFile = textBetweenFirstCategoryAndEndOfFile.replace(/\[\[:?Category:[^\]]+\]\]/g, '');
// does the non-category sample text have anything except whitespace?
let hasNonWhitespace = textBetweenFirstCategoryAndEndOfFile.match(/\S/);
if ( hasNonWhitespace ) {
return;
} |
Note to self: maybe I can fix this by tweaking the existing regex. Look into some of the ideas in this article, in the "Possessive Quantifiers and Atomic Grouping to The Rescue" section: |
Fix wikimedia-gadgets#245 This new code is vulnerable to deleting the wrong heading if someone puts a category in the wrong place (not at the bottom of the article), but I think that's an acceptable tradeoff for now. If it actually affects someone we can make the solution more complex in a future patch.
b9dd73d
to
51e7f73
Compare
OK, I rewrote this and solved all the issues. Ready for review. This alg should be identical to the old alg, but is iterative instead of using regex, so no catastrophic backtracking problems. |
Fix #245
This new code is vulnerable to deleting the wrong heading if someone puts a category in the wrong place (not at the bottom of the article), but I think that's an acceptable tradeoff for now. If it actually affects someone we can make the solution more complex in a future patch.