apache / apache/lucene

Fix replaceIgnoreCase method bug in EscapeQuerySyntaxImpl [LUCENE-8698]

Open
#9,744 0 comments 0 reactions 0 assignees View on GitHub
legacy-jira-priority:Major module:core/queryparser type:bug
Dominant language
Java
Stars
3.6k
Forks
1.4k
Avg merge
2d 11h
Merged PRs (30d)
88

Description

It is a patch of #9618 issue from @tonicava.

 

There is a serious bug in the replaceIgnoreCase method of the EscapeQuerySyntaxImpl class.

This issue can affect QueryNode. (StringIndexOutOfBoundsException)

As I mentioned in comment of the issue, the String#toLowerCase() causes the array to grow in size.

```java
private static CharSequence replaceIgnoreCase(CharSequence string,
CharSequence sequence1, CharSequence escapeChar, Locale locale) {
// string = "İpone " [304, 112, 111, 110, 101, 32], size = 6
...
while (start < count) {
// Convert by toLowerCase as follows.
// string = "i'̇pone " [105, 775, 112, 111, 110, 101, 32], size = 7
// firstIndex will be set 6.
if ((firstIndex = string.toString().toLowerCase(locale).indexOf(first,
start)) == -1)
break;
boolean found = true;
...
if (found) {
// In this line, String.toString() will only have a range of 0 to 5.
// So here we get a StringIndexOutOfBoundsException.
result.append(string.toString().substring(copyStart, firstIndex));
...
} else {
start = firstIndex + 1;
}
}
...
}
```

Maintaining the overall structure and fixing bug is very simple.

If we change to the following code, the method works fine.

 

```java
// Line 135 \~ 136
// BEFORE
if ((firstIndex = string.toString().toLowerCase(locale).indexOf(first, start)) == -1)

// AFTER
if ((firstIndex = string.toString().indexOf(first, start)) == -1)
```

 

 

But I wonder if this is the best way.

How do you think about using String#replace() instead?

 

```java
// SAMPLE : escapeWhiteChar (escapeChar and escapeQuoted are same)
// BEFORE
private static final CharSequence escapeWhiteChar(CharSequence str,
Locale locale) {
...
for (int i = 0; i < escapableWhiteChars.length; i++) {
buffer = replaceIgnoreCase(buffer, escapableWhiteChars[i].toLowerCase(locale),
"\\", locale);
}
...
}

// AFTER
private static final CharSequence escapeWhiteChar(CharSequence str,
Locale locale) {
...
for (int i = 0; i < escapableWhiteChars.length; i++) {
buffer = buffer.toString().replace(escapableWhiteChars[i], "\\" + escapableWhiteChars[i]);
}
...
}
```

 

First, I upload the patch using String#replace().
If you give me some feedback, I will check it :D

---
Migrated from [LUCENE-8698](https://issues.apache.org/jira/browse/LUCENE-8698) by Namgyu Kim (@danmuzi), updated Mar 26 2019
Attachments: [LUCENE-8698.patch](https://apache.github.io/lucene-jira-archive/attachments/LUCENE-8698/LUCENE-8698.patch)

Contributor guide

Open the contributing guide

Research direction

Start by reviewing the replaceIgnoreCase method in EscapeQuerySyntaxImpl and the attached LUCENE-8698.patch. Reproduce the Turkish-locale case involving "İpone" and verify that QueryNode processing no longer raises StringIndexOutOfBoundsException; the chosen fix should preserve the intended case-insensitive escaping behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
search
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.