CharacterEscaper - further optimizable? Bugs? I have been working on a character escaper for a production server that receives quite a few requests per second (sometimes in the range of 400).

We forward some "query" requests to apache Lucent/SOLR, and I need to escape characters.

Containing also feedback from programmers-stackexchange , the code below is the result.

Can anyone see any bugs or additional ways to optimize?

String query = "http://This+*is||a&amp;&amp;test(whatever!!!!!!)";
char[] queryCharArray = new char[query.length()*2];
char c;
int length = query.length();
int currentIndex = 0;
for (int i = 0; i &lt; length; i++) 
{
    c = query.charAt(i);
    if(mustBeEscaped[c]){        
      if('&amp;'==c || '|'==c){
        if(i+1 &lt; length &amp;&amp; query.charAt(i+1) == c){
            queryCharArray[currentIndex++] = '\\'; 
            queryCharArray[currentIndex++] = c; 
            queryCharArray[currentIndex++] = c; 
            i++;
        }
      }    
      else{
        queryCharArray[currentIndex++] = '\\'; 
        queryCharArray[currentIndex++] = c; 
      }     
    }
    else{
        queryCharArray[currentIndex++] = c;     
    }
}

query = new String(queryCharArray,0,currentIndex);

System.out.println("TEST="+query);

private static final boolean[] mustBeEscaped = new boolean[65536];
static{
mustBeEscaped[':']=  //
  for(char c: "\\?+-!(){}[]^\"~*&amp;|".toCharArray()){
     mustBeEscaped[c]=true; 
  }
}
First I have to be honest and say that this code puts me into WTF-Mode very early on, but that must not be a bad thing. But what took me by surprise is the lack of comments, there are some edge-cases in there I can not make out why they're the way they're.

On the latter, changing, suggestions I did only perform minor profiling by simply running it in a loop with 1 million iterations and seeing if the time differs by much.



Rule of thumb: Never roll your own security!



Your indentation/brace-style is a mess. Java normally uses a modified K&amp;R-Style, with the opening brace on the same line.

function test() {
    if (condition) {
        // Stuff
    } else {
        // Stuff
    }
}


You should fix that first.



Some of your names can be improved. F.e. queryCharArray might be better named escapedQuery.



You might want to extract all hard coded chars at least into static final variables for readability.



Your loop-beginning can be changed to:

char[] queryCharArray = new char[query.length() * 2];
int currentIndex = 0;
    for (int i = 0; i &lt; query.length(); i++) {
        char c = query.charAt(i);


This improves readability by a lot.



Your loop can be optimized to this:

char c = query.charAt(i);
if (mustBeEscaped[c]) {
    queryCharArray[currentIndex++] = '\\';
    if ('&amp;' == c || '|' == c) {
        if (i + 1 &lt; query.length() &amp;&amp; query.charAt(i + 1) == c) {
            queryCharArray[currentIndex++] = c;
            i++;
        }
    }
}
queryCharArray[currentIndex++] = c;


This removes unnecessary else-blocks.



if ('&amp;' == c || '|' == c) {
    if (i + 1 &lt; query.length() &amp;&amp; query.charAt(i + 1) == c) {


You should leave a comment here explaining why these characters are special. Extracting them into another array might be a good idea.

if (skipNextOccurence[c]) {




Putting all this together (and renaming the constants to fit the Java naming conventions), I ended up with this in my tests:

private static final boolean[] MUST_BE_ESCAPED = new boolean[65536];
private static final boolean[] SKIP_NEXT_OCCURENCE = new boolean[65536];
private static final char ESCAPE_CHARACTER = '\\';

static {
    for (char c : "\\?+-!(){}[]^\"~*&amp;|".toCharArray()) {
        MUST_BE_ESCAPED[c] = true;
    }
    for (char c : "&amp;|".toCharArray()) {
        SKIP_NEXT_OCCURENCE[c] = true;
    }
}

private static String escape(String query) {
    char[] escapedQuery = new char[query.length() * 2];
    int currentIndex = 0;

    for (int idx = 0; idx &lt; query.length(); idx++) {
        char c = query.charAt(idx);

        if (MUST_BE_ESCAPED[c]) {
            escapedQuery[currentIndex++] = ESCAPE_CHARACTER;

            if (SKIP_NEXT_OCCURENCE[c]) {
                // Check if the next char is the same, and if yes add it to
                // the escapedQuery and make sure that it is skipped.
                if (idx + 1 &lt; query.length() &amp;&amp; query.charAt(idx + 1) == c) {
                    escapedQuery[currentIndex++] = c;
                    idx++;
                }
            }
        }

        escapedQuery[currentIndex++] = c;
    }

    return new String(escapedQuery, 0, currentIndex);
}




To elaborate on the possibilities to use a StringBuilder, or a Set, or pretty much everything else I tried to come up with which looked more readable...damn that thing is fast! Even using a StringBuilder slowed it down considerably. So as long as you're aware that you are, a little bit at least, abusing memory with that 65k array (which will end up as 65k * 8Byte on most platforms) it's fine with me.

Also don't forget that you allocate twice the amount of whatever is passed into this function. So if somebody feels funny and passes a 1Mb query into that, it will at least consume 3Mb (1+1*2) on it's way. That is, if the JVM is not smarter then I think and does not allocate all that at once.

Just use more comments and JavaDoc.