In message <[email protected]>, 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, [email protected] 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);
> > }
> > }
> >
> >
> >
> >
>
