Skip to content

Commit ec93ad3

Browse files
authored
Fix not escaping special characters in search pattern (#5938)
* Fix not escaping special characters in search pattern fixes #5892 * add method to get search pattern for searched words with escaped javascript regexp special characters (for search without regular expressions) * in preview viewer use search pattern with escaped javascript regexp special characters * Refactoring and performance improvement * use enum to specify special characters escape mode * use compiled regex pattern instead of string * Refactoring: braces in if..else * Refactoring, minor changes: names, comments * Add tests of escaping special characters in search patterns
1 parent 741630b commit ec93ad3

File tree

3 files changed

+81
-3
lines changed

3 files changed

+81
-3
lines changed

src/main/java/org/jabref/gui/preview/PreviewViewer.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public class PreviewViewer extends ScrollPane implements InvalidationListener {
8585
private boolean registered;
8686

8787
private ChangeListener<Optional<SearchQuery>> listener = (queryObservable, queryOldValue, queryNewValue) -> {
88-
searchHighlightPattern = queryNewValue.flatMap(SearchQuery::getPatternForWords);
88+
searchHighlightPattern = queryNewValue.flatMap(SearchQuery::getJavaScriptPatternForWords);
8989
highlightSearchPattern();
9090
};
9191

@@ -131,7 +131,7 @@ public void setTheme(String theme) {
131131

132132
private void highlightSearchPattern() {
133133
if (searchHighlightPattern.isPresent()) {
134-
String pattern = searchHighlightPattern.get().pattern().replace("\\Q", "").replace("\\E", "");
134+
String pattern = searchHighlightPattern.get().pattern();
135135

136136
previewView.getEngine().executeScript(
137137
"var markInstance = new Mark(document.getElementById(\"content\"));" +

src/main/java/org/jabref/logic/search/SearchQuery.java

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,25 @@
1818

1919
public class SearchQuery implements SearchMatcher {
2020

21+
/**
22+
* Regex pattern for escaping special characters in javascript regular expressions
23+
*/
24+
public static final Pattern JAVASCRIPT_ESCAPED_CHARS_PATTERN = Pattern.compile("[\\.\\*\\+\\?\\^\\$\\{\\}\\(\\)\\|\\[\\]\\\\/]");
25+
26+
/**
27+
* The mode of escaping special characters in regular expressions
28+
*/
29+
private enum EscapeMode {
30+
/**
31+
* using \Q and \E marks
32+
*/
33+
JAVA,
34+
/**
35+
* escaping all javascript regex special characters separately
36+
*/
37+
JAVASCRIPT
38+
}
39+
2140
private final String query;
2241
private final boolean caseSensitive;
2342
private final boolean regularExpression;
@@ -124,6 +143,18 @@ public List<String> getSearchWords() {
124143

125144
// Returns a regular expression pattern in the form (w1)|(w2)| ... wi are escaped if no regular expression search is enabled
126145
public Optional<Pattern> getPatternForWords() {
146+
return joinWordsToPattern(EscapeMode.JAVA);
147+
}
148+
149+
// Returns a regular expression pattern in the form (w1)|(w2)| ... wi are escaped for javascript if no regular expression search is enabled
150+
public Optional<Pattern> getJavaScriptPatternForWords() {
151+
return joinWordsToPattern(EscapeMode.JAVASCRIPT);
152+
}
153+
154+
/** Returns a regular expression pattern in the form (w1)|(w2)| ... wi are escaped if no regular expression search is enabled
155+
* @param escapeMode the mode of escaping special characters in wi
156+
*/
157+
private Optional<Pattern> joinWordsToPattern(EscapeMode escapeMode) {
127158
List<String> words = getSearchWords();
128159

129160
if ((words == null) || words.isEmpty() || words.get(0).isEmpty()) {
@@ -133,7 +164,20 @@ public Optional<Pattern> getPatternForWords() {
133164
// compile the words to a regular expression in the form (w1)|(w2)|(w3)
134165
StringJoiner joiner = new StringJoiner(")|(", "(", ")");
135166
for (String word : words) {
136-
joiner.add(regularExpression ? word : Pattern.quote(word));
167+
if (regularExpression) {
168+
joiner.add(word);
169+
} else {
170+
switch (escapeMode) {
171+
case JAVA:
172+
joiner.add(Pattern.quote(word));
173+
break;
174+
case JAVASCRIPT:
175+
joiner.add(JAVASCRIPT_ESCAPED_CHARS_PATTERN.matcher(word).replaceAll("\\\\$0"));
176+
break;
177+
default:
178+
throw new IllegalArgumentException("Unknown special characters escape mode: " + escapeMode);
179+
}
180+
}
137181
}
138182
String searchPattern = joiner.toString();
139183

src/test/java/org/jabref/logic/search/SearchQueryTest.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,4 +203,38 @@ public void testGetPattern() {
203203
//We can't directly compare the pattern objects
204204
assertEquals(Optional.of(pattern.toString()), result.getPatternForWords().map(Pattern::toString));
205205
}
206+
207+
@Test
208+
public void testGetRegexpPattern() {
209+
String queryText = "[a-c]\\d* \\d*";
210+
SearchQuery regexQuery = new SearchQuery(queryText, false, true);
211+
Pattern pattern = Pattern.compile("([a-c]\\d* \\d*)");
212+
assertEquals(Optional.of(pattern.toString()), regexQuery.getPatternForWords().map(Pattern::toString));
213+
}
214+
215+
@Test
216+
public void testGetRegexpJavascriptPattern() {
217+
String queryText = "[a-c]\\d* \\d*";
218+
SearchQuery regexQuery = new SearchQuery(queryText, false, true);
219+
Pattern pattern = Pattern.compile("([a-c]\\d* \\d*)");
220+
assertEquals(Optional.of(pattern.toString()), regexQuery.getJavaScriptPatternForWords().map(Pattern::toString));
221+
}
222+
223+
@Test
224+
public void testEscapingInPattern() {
225+
//first word contain all java special regex characters
226+
String queryText = "<([{\\\\^-=$!|]})?*+.> word1 word2.";
227+
SearchQuery textQueryWithSpecialChars = new SearchQuery(queryText, false, false);
228+
String pattern = "(\\Q<([{\\^-=$!|]})?*+.>\\E)|(\\Qword1\\E)|(\\Qword2.\\E)";
229+
assertEquals(Optional.of(pattern), textQueryWithSpecialChars.getPatternForWords().map(Pattern::toString));
230+
}
231+
232+
@Test
233+
public void testEscapingInJavascriptPattern() {
234+
//first word contain all javascript special regex characters that should be escaped individually in text based search
235+
String queryText = "([{\\\\^$|]})?*+./ word1 word2.";
236+
SearchQuery textQueryWithSpecialChars = new SearchQuery(queryText, false, false);
237+
String pattern = "(\\(\\[\\{\\\\\\^\\$\\|\\]\\}\\)\\?\\*\\+\\.\\/)|(word1)|(word2\\.)";
238+
assertEquals(Optional.of(pattern), textQueryWithSpecialChars.getJavaScriptPatternForWords().map(Pattern::toString));
239+
}
206240
}

0 commit comments

Comments
 (0)