RuntimeException in Preferences on startup

This was reported in another part of this forum by a Windows user. A slightly different version of this patch was provided to them for testing.

There is the possibility that a StringIndexOutOfBoundsException can be thrown from Preferences.setColor() on startup, halting the app.

The problem is that the method assumes that the String it creates (based on the hexadecimal representation of the RGB colours for UI controls it retrieves from AWT) will always be at least two characters long. Clearly, for any RGB values less than 16 decimal this will not be the case.

While building up the “#rrggbb” String value from these RGB component Strings, this assumption means we can pass -1 to String.substring(), leading to the StringIndexOutOfBoundsException.

Ideally, we’d like to use something like java.util.Formatter to build up this string in a normalized manner, but this is only available in Java 5 or higher. It might also be a little heavyweight for the purpose.

I propose the following change to address this (sorry for the white space changes – Xcode seems to want to do this):

---    (revision 567)
+++    (working copy)
@@ -789,11 +789,21 @@
   static public void setColor(String attr, Color what) {
-    String r = Integer.toHexString(what.getRed());
-    String g = Integer.toHexString(what.getGreen());
-    String b = Integer.toHexString(what.getBlue());
-    set(attr, "#" + r.substring(r.length() - 2) +
-        g.substring(g.length() - 2) + b.substring(b.length() - 2));
+       String r = Integer.toHexString(what.getRed());
+       String g = Integer.toHexString(what.getGreen());
+       String b = Integer.toHexString(what.getBlue());
+       // Massage each RGB component so that it conforms to a reasonable
+       // "#rrggbb" format. Ideally we'd Like use something like
+       // java.util.Formatter from Java 5 for this.
+       if (r.length() < 2)
+               r = "0" + r;
+       if (g.length() < 2)
+               g = "0" + g;
+       if (r.length() < 2)
+               b = "0" + b;
+       set(attr, "#" + r + g + b);

We simply ensure that single character strings (I don’t think these strings can’t be zero-length under any circumstances where the VM would continue) are prepended with a ‘0’. This ensures that any hex R, G or B value we compute will be of the form “00” to “ff” and nothing else.

My rationale for removing the belt-and-suspenders substring munging is that I think we can go ahead and trust that Color.getRed() and friends will return a number between 0 and 255. There is little reason to not trust that the resulting String of hexadecimal digits we get from this int will not be at least one character long, and at most two.

Certainly the existing code doesn’t really guard against the extremely unlikely event that these Strings can have a length > 2 anyway. So, if we /really/ want to be belt-and-suspenders for some reason, we should change this to something like:

r = r.substring(0, 2);
g = g.substring(0, 2);
b = b.substring(0, 2);

(Or we bracket the decimal values we get from AWT to the range 0-255, and only then compute the hexadecimal values we use to build the format string.)

However, I see no compelling reason to do so.