DeepLinkDispatch icon indicating copy to clipboard operation
DeepLinkDispatch copied to clipboard

performance issue: containsInvalidHostnameAsciiCodes

Open OleksandrKucherenko opened this issue 7 years ago • 4 comments

Original code:

private static boolean containsInvalidHostnameAsciiCodes(String hostnameAscii) {
      for (int i = 0; i < hostnameAscii.length(); i++) {
        char c = hostnameAscii.charAt(i);
        // The WHATWG Host parsing rules accepts some character codes which are invalid by
        // definition for OkHttp's host header checks (and the WHATWG Host syntax definition). Here
        // we rule out characters that would cause problems in host headers.
        if (c <= '\u001f' || c >= '\u007f') {
          return true;
        }
        // Check for the characters mentioned in the WHATWG Host parsing spec:
        // U+0000, U+0009, U+000A, U+000D, U+0020, "#", "%", "/", ":", "?", "@", "[", "\", and "]"
        // (excluding the characters covered above).
        if (" #%/:?@[\\]".indexOf(c) != -1) {
          return true;
        }
      }
      return false;
    }

line with IndexOf call is not optimal.

screen shot 2017-06-15 at 14 24 51

OleksandrKucherenko avatar Jun 15 '17 12:06 OleksandrKucherenko

possible ways to solve:

  • compose SparseArray or Map for quick lookup
  • sort array of chars and use binarySearch algorithm for validation
  • make a switch case for values (compiler will create a lookup map for us than in most affective way): 0x20, 0x23, 0x25, 0x2f, 0x3a, 0x3f, 0x40, 0x5b, 0x5c, 0x5d
  • rewrite code as a regex expression

OleksandrKucherenko avatar Jun 15 '17 12:06 OleksandrKucherenko

switch/case - gives the best performance

OleksandrKucherenko avatar Jun 15 '17 13:06 OleksandrKucherenko

    static boolean containsInvalidHostnameAsciiCodes_v3(String hostnameAscii) {
        for (int i = 0; i < hostnameAscii.length(); ++i) {
            char c = hostnameAscii.charAt(i);
            if (c <= 31 || c >= 127) {
                return true;
            }

            switch ((byte) c) {
                case 0x20:
                case 0x23:
                case 0x25:
                case 0x2f:
                case 0x3a:
                case 0x3f:
                case 0x40:
                case 0x5b:
                case 0x5c:
                case 0x5d:
                    return true;
            }
        }

        return false;
    }

OleksandrKucherenko avatar Jun 15 '17 13:06 OleksandrKucherenko

@OleksandrKucherenko please feel free to send a PR!

felipecsl avatar Jun 15 '17 16:06 felipecsl