-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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 not escaping special characters in search pattern #5938
Changes from 3 commits
2f68520
5c2925d
89c72c0
6f39cbc
266bfc8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,25 @@ | |
|
||
public class SearchQuery implements SearchMatcher { | ||
|
||
/** | ||
* Regex pattern for escaping special characters in javascript regular expressions | ||
*/ | ||
public static final Pattern JAVASCRIPT_ESCAPED_CHARS_PATTERN = Pattern.compile("[\\.\\*\\+\\?\\^\\$\\{\\}\\(\\)\\|\\[\\]\\\\/]"); | ||
|
||
/** | ||
* Metod for escaping special characters in regular expressions | ||
*/ | ||
private enum EscapeMode { | ||
/** | ||
* using \Q and \E marks | ||
*/ | ||
JAVA, | ||
/** | ||
* escaping all javascript regex special characters separately | ||
*/ | ||
JAVASCRIPT | ||
} | ||
|
||
private final String query; | ||
private final boolean caseSensitive; | ||
private final boolean regularExpression; | ||
|
@@ -124,6 +143,18 @@ public List<String> getSearchWords() { | |
|
||
// Returns a regular expression pattern in the form (w1)|(w2)| ... wi are escaped if no regular expression search is enabled | ||
public Optional<Pattern> getPatternForWords() { | ||
return joinWordsToPattern(EscapeMode.JAVA); | ||
} | ||
|
||
// Returns a regular expression pattern in the form (w1)|(w2)| ... wi are escaped for javascript if no regular expression search is enabled | ||
public Optional<Pattern> getJsPatternForWords() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMHO, although "Js" is quite obvious, I would reword that to getJavaScriptPatternForWords, since you are creating a new Method just for the sole purpose to make its use more obvious in it's name. So why not go all the way? |
||
return joinWordsToPattern(EscapeMode.JAVASCRIPT); | ||
} | ||
|
||
/** Returns a regular expression pattern in the form (w1)|(w2)| ... wi are escaped if no regular expression search is enabled | ||
* @param escapeMode method for escaping special characters in wi | ||
*/ | ||
private Optional<Pattern> joinWordsToPattern(EscapeMode escapeMode) { | ||
List<String> words = getSearchWords(); | ||
|
||
if ((words == null) || words.isEmpty() || words.get(0).isEmpty()) { | ||
|
@@ -133,7 +164,21 @@ public Optional<Pattern> getPatternForWords() { | |
// compile the words to a regular expression in the form (w1)|(w2)|(w3) | ||
StringJoiner joiner = new StringJoiner(")|(", "(", ")"); | ||
for (String word : words) { | ||
joiner.add(regularExpression ? word : Pattern.quote(word)); | ||
if (regularExpression) { | ||
joiner.add(word); | ||
} | ||
else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't checkstyle complaining about putting this in two lines? Has no effect, but looks somewhat odd... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it's a mistake, I'll change it. Also looks odd to me. Checkstyle wasn't complaining. |
||
switch (escapeMode) { | ||
case JAVA: | ||
joiner.add(Pattern.quote(word)); | ||
break; | ||
case JAVASCRIPT: | ||
joiner.add(JAVASCRIPT_ESCAPED_CHARS_PATTERN.matcher(word).replaceAll("\\\\$0")); | ||
break; | ||
default: | ||
throw new IllegalArgumentException("Unknown special characters escape method: " + escapeMode); | ||
} | ||
} | ||
} | ||
String searchPattern = joiner.toString(); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method