In message <4c0fbf28.3000...@gmail.com>, Tim Ellison writes: > > I've not benched it but this looks like a reasonable amount of code to > run each time the hostname is checked against this permission. > > Maybe not too bad for getting these addresses from the interface, but > if they are checked for each connect... > > Is it worth caching the full form of the IPv6 address in the > InetAddress itself instance?
Though not obvious from the commit diff, in this context, I don't think it is a problem ... since you probably don't lookup addresses associated with an interface very often. The question in my mind, that I was still thinking about is: Are there other calls to SecurityManager.checkConnect(...) that might have compressed hostnames? We probably need to audit them to check. Looking at the context is a little worrying... first I was puzzled by the (security != null) check in the loop when it could be moved outside: for (InetAddress element : addresses) { if (security != null) { try { /* * since we don't have a port in this case we pass in * NO_PORT */ String hostName = element.getHostName(); if(hostName.contains("::")){ hostName = getFullFormOfCompressedIPV6Address(hostName); } security.checkConnect(hostName, CHECK_CONNECT_NO_PORT); accessibleAddresses.add(element); } catch (SecurityException e) { } } } Then I noticed that the code immediately preceding this was: /* * get the security manager. If one does not exist just return the full * list */ SecurityManager security = System.getSecurityManager(); if (security == null) { return (new Vector<InetAddress>(Arrays.asList(addresses))).elements(); } /* * ok security manager exists so check each address and return those * that pass */ for (InetAddress element : addresses) { if (security != null) { ... } } So the security != null isn't needed anyway. I added an item to my todo list to take a better look at this code. I would not mind if someone beats me to it though. Regards, Mark. > On 09/Jun/2010 15:05, hinde...@apache.org wrote: > > Author: hindessm > > Date: Wed Jun 9 14:05:56 2010 > > New Revision: 953015 > > > > URL: http://svn.apache.org/viewvc?rev=953015&view=rev > > Log: > > Applying patches from "[#HARMONY-6532] [classlib][luni]SocketPermission > > does NOT support compressed IPV6 address, should pass in full form > > address". > > > > Modified: > > harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/ne > t/NetworkInterface.java > > harmony/enhanced/java/trunk/classlib/modules/luni/src/test/api/common/o > rg/apache/harmony/luni/tests/java/net/NetworkInterfaceTest.java > > > > Modified: harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/j > ava/net/NetworkInterface.java > > URL: http://svn.apache.org/viewvc/harmony/enhanced/java/trunk/classlib/modu > les/luni/src/main/java/java/net/NetworkInterface.java?rev=953015&r1=953014&r2 > =953015&view=diff > > =========================================================================== > === > > --- harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/ne > t/NetworkInterface.java (original) > > +++ harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/ne > t/NetworkInterface.java Wed Jun 9 14:05:56 2010 > > @@ -166,7 +166,11 @@ public final class NetworkInterface exte > > * since we don't have a port in this case we pass in > > * NO_PORT > > */ > > - security.checkConnect(element.getHostName(), > > + String hostName = element.getHostName(); > > + if(hostName.contains("::")){ > > + hostName = getFullFormOfCompressedIPV6Address(host > Name); > > + } > > + security.checkConnect(hostName, > > CHECK_CONNECT_NO_PORT); > > accessibleAddresses.add(element); > > } catch (SecurityException e) { > > @@ -182,6 +186,62 @@ public final class NetworkInterface exte > > > > return new Vector<InetAddress>(0).elements(); > > } > > + > > + private String getFullFormOfCompressedIPV6Address(String compressed) { > > + StringBuilder fullForm = new StringBuilder(39); > > + final int NUM_OF_IPV6_FIELDS = 8; > > + > > + String[] fields = compressed.split(":"); > > + // the number of compressed fields > > + int numOfCompressedFields; > > + > > + if (compressed.startsWith("::")) { // compressed head part > > + compressed = compressed.replace("::", ""); > > + fields = compressed.split(":"); > > + numOfCompressedFields = NUM_OF_IPV6_FIELDS - fields.length; > > + restoreCompressedFieldsWithZero(fullForm, numOfCompressedField > s); > > + appendNonZeroFields(fullForm, fields); > > + } else if (compressed.endsWith("::")) { // compressed tail part > > + compressed = compressed.replace("::", ""); > > + fields = compressed.split(":"); > > + numOfCompressedFields = NUM_OF_IPV6_FIELDS - fields.length; > > + appendNonZeroFields(fullForm, fields); > > + restoreCompressedFieldsWithZero(fullForm, numOfCompressedField > s); > > + } else { // compressed middle part > > + numOfCompressedFields = NUM_OF_IPV6_FIELDS - fields.length + 1 > ; > > + for (String field : fields) { > > + if (field.equals("")) { > > + // for compressed fields add 0 > > + restoreCompressedFieldsWithZero(fullForm, numOfCompres > sedFields); > > + } else { > > + fullForm.append(field); > > + // add colon > > + fullForm.append(":"); > > + } > > + } > > + } > > + // delete the excess colon > > + fullForm.deleteCharAt(fullForm.length() - 1); > > + > > + return fullForm.toString(); > > + } > > + > > + private void appendNonZeroFields(StringBuilder fullForm, > > + String[] fields) { > > + for (int i = 0; i < fields.length; i++) { > > + fullForm.append(fields[i]); > > + fullForm.append(":"); > > + } > > + } > > + > > + private void restoreCompressedFieldsWithZero(StringBuilder fullForm, > > + int numOfCompressedFields) { > > + for (int i = 0; i < numOfCompressedFields; i++) { > > + fullForm.append("0"); > > + // add colon > > + fullForm.append(":"); > > + } > > + } > > > > /** > > * Gets the human-readable name associated with this network interface > . > > > > Modified: harmony/enhanced/java/trunk/classlib/modules/luni/src/test/api/co > mmon/org/apache/harmony/luni/tests/java/net/NetworkInterfaceTest.java > > URL: http://svn.apache.org/viewvc/harmony/enhanced/java/trunk/classlib/modu > les/luni/src/test/api/common/org/apache/harmony/luni/tests/java/net/NetworkIn > terfaceTest.java?rev=953015&r1=953014&r2=953015&view=diff > > =========================================================================== > === > > --- harmony/enhanced/java/trunk/classlib/modules/luni/src/test/api/common/o > rg/apache/harmony/luni/tests/java/net/NetworkInterfaceTest.java (original) > > +++ harmony/enhanced/java/trunk/classlib/modules/luni/src/test/api/common/o > rg/apache/harmony/luni/tests/java/net/NetworkInterfaceTest.java Wed Jun 9 14 > :05:56 2010 > > @@ -376,12 +376,22 @@ public class NetworkInterfaceTest extend > > networkInterface1.toString()); > > assertFalse("validate that non-zero length string is ge > nerated", > > networkInterface1.toString().equals("") > ); > > + > > + SecurityManager backup = System.getSecurityManager(); > > + System.setSecurityManager(new SecurityManager()); > > + assertNotNull(networkInterface1.toString()); > > + System.setSecurityManager(backup); > > } > > if (atLeastTwoInterfaces) { > > assertFalse( > > "Validate strings are different for dif > ferent interfaces", > > networkInterface1.toString().equals( > > networkInterface2.toStr > ing())); > > + > > + SecurityManager backup = System.getSecurityManager(); > > + System.setSecurityManager(new SecurityManager()); > > + assertNotNull(networkInterface2.toString()); > > + System.setSecurityManager(backup); > > } > > } > > > > > > > > >