uschindler commented on a change in pull request #590:
URL: https://github.com/apache/lucene/pull/590#discussion_r780260497
##########
File path: lucene/core/src/test/org/apache/lucene/util/automaton/TestRegExp.java
##########
@@ -99,6 +99,7 @@ private static int randomInt(int bound) {
return bound == 0 ? 0 : random().nextInt(bound);
}
+ @SuppressWarnings("BareDotMetacharacter")
Review comment:
I think we can keep it in. I would generally put String.replaceAll on
the forbidden APIs list as this causes so many also security bugs. In Solr I
have seen this exact case where somebody wanted to replace dots in class names
by slashes.
Most people don't know that replace all uses a regex, which needs to be
compiled, too. So in production code I would prefer to always compile patterns
up front.
So as this is only one place we should keep it in. Maybe it catches other
shit.
Especially this piece of code on the first view looks crazy. Maybe rewrite
to be explicit using a pattern.
Maybe I should do this and remove the suppression.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
