Description Dimitriye Danilovic 2021-06-04 08:48:18 EDT

```
java.lang.StringIndexOutOfBoundsException: String index out of range: -2
	at java.base/java.lang.StringLatin1.charAt(StringLatin1.java:48)
	at java.base/java.lang.String.charAt(String.java:712)
	at org.cadixdev.mercury.jdt.rewrite.imports.ImportRewrite.compareImport(ImportRewrite.java:611)
	at org.cadixdev.mercury.jdt.rewrite.imports.ImportRewrite.findInImports(ImportRewrite.java:630)
	at org.cadixdev.mercury.jdt.rewrite.imports.ImportRewrite$1.findInContext(ImportRewrite.java:448)
	at com.dimitriye.filigree.remapper.RemapperVisitor$ImportContext.findInContext(RemapperVisitor.java:469)
```

The class in question is in another library, but it's copied wholesale over from JDT by their build process and after debugging and (I think) identifying the line causing the problem, the line in question is in `compareImport` in `org.eclipse.jdt.core.dom.rewrite.ImportRewrite`.

I'm not well versed enough with JDT to write a minimal example depending directly on JDT, I'll try to do so as soon as I can.

The key thing is that it seems to be an issue when the `name` parameter begins with `n`, in this case the `name` parameter was `nx` due to working with obfuscated code.

This allows the flow of execution to escape the guard at the beginning
```
if (curr.charAt(0) != prefix || !curr.endsWith(name)) {
```
as according to the debugger, `prefix` in this case was `n` and `curr` was `nx` just like `name`. Thus the execution proceeded as follows:
```
// Start at line 596 in org.eclipse.jdt.core.dom.rewrite.ImportRewrite
// curr: nx, name: nx, prefix: n
// Guard not triggered as both if conditions evaluate to false
if (curr.charAt(0) != prefix || !curr.endsWith(name)) {
  return ImportRewriteContext.RES_NAME_UNKNOWN;
}

curr= curr.substring(1); // removes the prefix
// curr: x, name: nx, prefix: n

// if branch gets skipped, `curr.length(): 1 != name.length(): 2`
if (curr.length() == name.length()) {
  if (qualifier.length() == 0) {
    return ImportRewriteContext.RES_NAME_FOUND;
  }
  return ImportRewriteContext.RES_NAME_CONFLICT;
}
// at this place: curr.length > name.length
// !!!!!!! CONTRACT VIOLATED !!!!!!!

int dotPos= curr.length() - name.length() - 1; // dotPos: -1
if (curr.charAt(dotPos) != '.') { // Crash
```

The call in question also came internally from JDT at
```
// Start at line 624 in same file
boolean allowAmbiguity=  (kind == ImportRewriteContext.KIND_STATIC_METHOD) || (name.length() == 1 && name.charAt(0) == '*');
List imports= this.existingImports;
char prefix= (kind == ImportRewriteContext.KIND_TYPE) ? NORMAL_PREFIX : STATIC_PREFIX;

for (int i= imports.size() - 1; i >= 0 ; i--) {
  String curr= (String) imports.get(i);
  int res= compareImport(prefix, qualifier, name, curr);
```

As best as I can tell, what triggered the bug was a preexisting import named `x`.

I *think* that the solution is simply to change
```
if (curr.charAt(0) != prefix || !curr.endsWith(name)) {
```
to
```
if (curr.charAt(0) != prefix || !curr.substring(1).endsWith(name)) {
```
but I'd rather someone more well versed in the codebase look at the issue before I submit a pull request.