RFR 8027687: The constructors of URLPermission class do not behave as described in javadoc

[Chris Hegarty]

The latest changes look better to me.

Trivially ( and in agreement with Vitaly ), I would, but you don't have 
to, change:

     static final int CAP_DIFF = 0x20; // 'a' - 'A';

     static String toLowerCase(String s) {
         final int len = s.length();
         StringBuilder sb = null;

         for (int i=0; i<len; i++) {
             char c = s.charAt(i);
             if ((c >= 'a' && c <= 'z') || (c == '.')) {
                 if (sb != null)
                     sb.append(c);
             } else if ((c >= '0' && c <= '9') || (c == '-')) {
                 if (sb != null)
                     sb.append(c);
             } else if (c >= 'A' && c <= 'Z') {
                 if (sb == null) {
                     sb = new StringBuilder(len);
                     sb.append(s, 0, i);      // <<<<<<< HERE
                 }
                 sb.append((char)(c + CAP_DIFF));
             } else {
                 throw new IllegalArgumentException("Invalid characters 
in hostname");
             }
         }
         return sb == null ? s : sb.toString();

-Chris.

On 04/11/2013 13:35, Michael McMahon wrote:
> Vitaly,
>
> Good points, I agree. I think the optimisation is worthwhile
> and not costly. I also found a small casting issue with
> StringBuilder.append(char)
> being interpreted as StringBuilder.append(int)
>
> Here is the updated webrev:
>
> http://cr.openjdk.java.net/~michaelm/8027687/webrev.2/
>
> Thanks,
> Michael
>
>
> On 02/11/13 01:03, Vitaly Davidovich wrote:
>>
>> The size to use is the length of the argument, which you're already
>> using for the loop.
>>
>> On a separate note, is toLowerCase in a perf sensitive area? It makes
>> an assumption that the lowering will need to happen (by always
>> allocating the stringbuilder) but is that a common case? If this isn't
>> perf sensitive then disregard.
>>
>> Thanks
>>
>> Sent from my phone
>>
>> On Nov 1, 2013 4:28 PM, "Michael McMahon"
>> <michael.x.mcmahon at oracle.com <mailto:michael.x.mcmahon at oracle.com>>
>> wrote:
>>
>>     On 01/11/13 18:06, Mike Duigou wrote:
>>
>>         A couple minor quibbles
>>
>>         - Since the length is know the StringBuildiler can be created
>>         with a size.
>>
>>
>>     Right, 255 is probably a good size to use.
>>
>>         - sb.toString() is probably more efficient than new String(sb)
>>
>>
>>     Since Chris also suggests it, I'm curious why this is. Is there
>>     some clever sharing going on between
>>     StringBuilder and String?
>>
>>         - I would like to see some IDN URL cases in the tests.
>>
>>
>>     The first version of this class doesn't support Unicode strings in
>>     the hostname labels.
>>     So, I'm guessing you mean cases of IDNs that have been already
>>     converted
>>     into the ascii encoded form (eg xn--blahblah.xn-blah.com
>>     <http://xn--blahblah.xn-blah.com>). Something I'd like to do
>>     for JDK 9 will be to allow transparent Unicode in classes like
>>     URLPermission with
>>     automatic IDN conversions taking place in the http protocol handler.
>>     So, I can add some cases of encoded IDNs in the test okay.
>>
>>     Thanks!
>>
>>     Michael
>>
>>         Mike
>>
>>         On Nov 1 2013, at 07:46 , Michael McMahon
>>         <michael.x.mcmahon at oracle.com
>>         <mailto:michael.x.mcmahon at oracle.com>> wrote:
>>
>>             Simple bug fix to new URLPermission class, caused by
>>             insufficient parameter checking
>>             of the constructor.
>>
>>             webrev:
>>             http://cr.openjdk.java.net/~michaelm/8027687/webrev.1/
>>             <http://cr.openjdk.java.net/%7Emichaelm/8027687/webrev.1/>
>>
>>             Thanks,
>>             Michael
>>
>>
>