On 23/05/18 21:33, ma...@apache.org wrote: > Author: markt > Date: Wed May 23 20:33:35 2018 > New Revision: 1832124 > > URL: http://svn.apache.org/viewvc?rev=1832124&view=rev > Log: > First part of implementation for BZ 51953 > Add a NetMask utility class and some test cases
The code looks good to me but given how this is going to be used, I'd welcome additional eyes on this and especially some more test cases. Thanks, Mark > > Added: > tomcat/trunk/java/org/apache/catalina/util/NetMask.java (with props) > tomcat/trunk/test/org/apache/catalina/util/TestNetMask.java (with props) > Modified: > tomcat/trunk/java/org/apache/catalina/util/LocalStrings.properties > > Modified: tomcat/trunk/java/org/apache/catalina/util/LocalStrings.properties > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/util/LocalStrings.properties?rev=1832124&r1=1832123&r2=1832124&view=diff > ============================================================================== > --- tomcat/trunk/java/org/apache/catalina/util/LocalStrings.properties > (original) > +++ tomcat/trunk/java/org/apache/catalina/util/LocalStrings.properties Wed > May 23 20:33:35 2018 > @@ -38,6 +38,12 @@ lifecycleBase.stopFail=Failed to stop co > lifecycleMBeanBase.registerFail=Failed to register object [{0}] with name > [{1}] during component initialisation > lifecycleMBeanBase.unregisterFail=Failed to unregister MBean with name [{0}] > during component destruction > lifecycleMBeanBase.unregisterNoServer=No MBean server was available to > unregister the MBean [{0}] > + > +netmask.cidrNegative=The CIDR [{0}] is negative > +netmask.cidrNotNumeric=The CIDR [{0}] is not numeric > +netmask.cidrTooBig=The CIDR [{0}] is greater than the address length [{1}] > +netmask.invalidAddress=The address [{0}] is not valid > + > SecurityUtil.doAsPrivilege=An exception occurs when running the > PrivilegedExceptionAction block. > sessionIdGeneratorBase.createRandom=Creation of SecureRandom instance for > session ID generation using [{0}] took [{1}] milliseconds. > sessionIdGeneratorBase.random=Exception initializing random number generator > of class [{0}]. Falling back to java.secure.SecureRandom > > Added: tomcat/trunk/java/org/apache/catalina/util/NetMask.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/util/NetMask.java?rev=1832124&view=auto > ============================================================================== > --- tomcat/trunk/java/org/apache/catalina/util/NetMask.java (added) > +++ tomcat/trunk/java/org/apache/catalina/util/NetMask.java Wed May 23 > 20:33:35 2018 > @@ -0,0 +1,241 @@ > +/* > + * Licensed to the Apache Software Foundation (ASF) under one or more > + * contributor license agreements. See the NOTICE file distributed with > + * this work for additional information regarding copyright ownership. > + * The ASF licenses this file to You under the Apache License, Version 2.0 > + * (the "License"); you may not use this file except in compliance with > + * the License. You may obtain a copy of the License at > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > +package org.apache.catalina.util; > + > +import java.net.InetAddress; > +import java.net.UnknownHostException; > + > +import org.apache.catalina.tribes.util.StringManager; > + > +/** > + * A class representing a CIDR netmask. > + * > + * <p> > + * The constructor takes a string as an argument which represents a netmask, > as > + * per the CIDR notation -- whether this netmask be IPv4 or IPv6. It then > + * extracts the network address (before the /) and the CIDR prefix (after the > + * /), and tells through the #matches() method whether a candidate > + * {@link InetAddress} object fits in the recorded range. > + * </p> > + * > + * <p> > + * As byte arrays as returned by <code>InetAddress.getByName()</code> are > always > + * in network byte order, finding a match is therefore as simple as testing > + * whether the n first bits (where n is the CIDR) are the same in both byte > + * arrays (the one of the network address and the one of the candidate > address). > + * We do that by first doing byte comparisons, then testing the last bits if > any > + * (that is, if the remainder of the integer division of the CIDR by 8 is not > + * 0). > + * </p> > + * > + * <p> > + * As a bonus, if no '/' is found in the input, it is assumed that an exact > + * address match is required. > + * </p> > + */ > +public final class NetMask { > + > + private static final StringManager sm = > StringManager.getManager(NetMask.class); > + > + /** > + * The argument to the constructor, used for .toString() > + */ > + private final String expression; > + > + /** > + * The byte array representing the address extracted from the expression > + */ > + private final byte[] netaddr; > + > + /** > + * The number of bytes to test for equality (CIDR / 8) > + */ > + private final int nrBytes; > + > + /** > + * The right shift to apply to the last byte if CIDR % 8 is not 0; if it > is > + * 0, this variable is set to 0 > + */ > + private final int lastByteShift; > + > + > + /** > + * Constructor > + * > + * @param input the CIDR netmask > + * @throws IllegalArgumentException if the netmask is not correct > (invalid > + * address specification, malformed CIDR prefix, etc) > + */ > + public NetMask(final String input) { > + > + expression = input; > + > + final int idx = input.indexOf("/"); > + > + /* > + * Handle the "IP only" case first > + */ > + if (idx == -1) { > + try { > + netaddr = InetAddress.getByName(input).getAddress(); > + } catch (UnknownHostException e) { > + throw new > IllegalArgumentException(sm.getString("netmask.invalidAddress", input)); > + } > + nrBytes = netaddr.length; > + lastByteShift = 0; > + return; > + } > + > + /* > + * OK, we do have a netmask specified, so let's extract both the > address > + * and the CIDR. > + */ > + > + final String addressPart = input.substring(0, idx), cidrPart = > input.substring(idx + 1); > + > + try { > + /* > + * The address first... > + */ > + netaddr = InetAddress.getByName(addressPart).getAddress(); > + } catch (UnknownHostException e) { > + throw new > IllegalArgumentException(sm.getString("netmask.invalidAddress", addressPart)); > + } > + > + final int addrlen = netaddr.length * 8; > + final int cidr; > + > + try { > + /* > + * And then the CIDR. > + */ > + cidr = Integer.parseInt(cidrPart); > + } catch (NumberFormatException e) { > + throw new > IllegalArgumentException(sm.getString("netmask.cidrNotNumeric", cidrPart)); > + } > + > + /* > + * We don't want a negative CIDR, nor do we want a CIDR which is > greater > + * than the address length (consider 0.0.0.0/33, or ::/129) > + */ > + if (cidr < 0) { > + throw new > IllegalArgumentException(sm.getString("netmask.cidrNegative", cidrPart)); > + } > + if (cidr > addrlen) { > + throw new IllegalArgumentException( > + sm.getString("netmask.cidrTooBig", cidrPart, > Integer.valueOf(addrlen))); > + } > + > + nrBytes = cidr / 8; > + > + /* > + * These last two lines could be shortened to: > + * > + * lastByteShift = (8 - (cidr % 8)) & 7; > + * > + * But... It's not worth it. In fact, explaining why it could work > would > + * be too long to be worth the trouble, so let's do it the simple > way... > + */ > + > + final int remainder = cidr % 8; > + > + lastByteShift = (remainder == 0) ? 0 : 8 - remainder; > + } > + > + > + /** > + * Test if a given address matches this netmask. > + * > + * @param addr The {@link java.net.InetAddress} to test > + * @return true on match, false otherwise > + */ > + public boolean matches(final InetAddress addr) { > + final byte[] candidate = addr.getAddress(); > + > + /* > + * OK, remember that a CIDR prefix tells the number of BITS which > should > + * be equal between this NetMask's recorded address (netaddr) and the > + * candidate address. One byte is 8 bits, no matter what, and IP > + * addresses, whether they be IPv4 or IPv6, are big endian, aka MSB, > + * Most Significant Byte (first). > + * > + * We therefore need to get the byte array of the candidate address, > + * compare as many bytes of the candidate address with the recorded > + * address as the CIDR prefix tells us to (that is, CIDR / 8), and > then > + * deal with the remaining bits -- if any. > + * > + * But prior to that, a simple test can be done: we deal with IP > + * addresses here, which means IPv4 and IPv6. IPv4 addresses are > encoded > + * on 4 bytes, IPv6 addresses are encoded on 16 bytes. If the > candidate > + * address length is different than this NetMask's address, we don't > + * have a match. > + */ > + if (candidate.length != netaddr.length) { > + return false; > + } > + > + > + /* > + * Now do the byte-compare. The constructor has recorded the number > of > + * bytes to compare in nrBytes, use that. If any of the byte we have > to > + * compare is different than what we expect, we don't have a match. > + * > + * If, on the opposite, after this loop, all bytes have been deemed > + * equal, then the loop variable i will point to the byte right after > + * that -- which we will need... > + */ > + int i = 0; > + for (; i < nrBytes; i++) { > + if (netaddr[i] != candidate[i]) { > + return false; > + } > + } > + > + /* > + * ... if there are bits left to test. There aren't any if > lastByteShift > + * is set to 0. > + */ > + if (lastByteShift == 0) { > + return true; > + } > + > + /* > + * If it is not 0, however, we must test for the relevant bits in the > + * next byte (whatever is in the bytes after that doesn't matter). > We do > + * it this way (remember that lastByteShift contains the amount of > bits > + * we should _right_ shift the last byte): > + * > + * - grab both bytes at index i, both from the netmask address and > the > + * candidate address; - xor them both. > + * > + * After the xor, it means that all the remaining bits of the CIDR > + * should be set to 0... > + */ > + final int lastByte = netaddr[i] ^ candidate[i]; > + > + /* > + * ... Which means that right shifting by lastByteShift should be 0. > + */ > + return lastByte >> lastByteShift == 0; > + } > + > + > + @Override > + public String toString() { > + return expression; > + } > +} > > Propchange: tomcat/trunk/java/org/apache/catalina/util/NetMask.java > ------------------------------------------------------------------------------ > svn:eol-style = native > > Added: tomcat/trunk/test/org/apache/catalina/util/TestNetMask.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/util/TestNetMask.java?rev=1832124&view=auto > ============================================================================== > --- tomcat/trunk/test/org/apache/catalina/util/TestNetMask.java (added) > +++ tomcat/trunk/test/org/apache/catalina/util/TestNetMask.java Wed May 23 > 20:33:35 2018 > @@ -0,0 +1,108 @@ > +/* > + * Licensed to the Apache Software Foundation (ASF) under one or more > + * contributor license agreements. See the NOTICE file distributed with > + * this work for additional information regarding copyright ownership. > + * The ASF licenses this file to You under the Apache License, Version 2.0 > + * (the "License"); you may not use this file except in compliance with > + * the License. You may obtain a copy of the License at > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > +package org.apache.catalina.util; > + > +import java.net.InetAddress; > +import java.net.UnknownHostException; > +import java.util.ArrayList; > +import java.util.Collection; > +import java.util.List; > + > +import org.junit.Assert; > +import org.junit.Test; > +import org.junit.runner.RunWith; > +import org.junit.runners.Parameterized; > +import org.junit.runners.Parameterized.Parameter; > +import org.junit.runners.Parameterized.Parameters; > + > +@RunWith(Parameterized.class) > +public final class TestNetMask { > + > + @Parameter(0) > + public String mask; > + > + @Parameter(1) > + public String input; > + > + @Parameter(2) > + public Boolean valid; > + > + @Parameter(3) > + public Boolean matches; > + > + > + @Parameters(name="{index}: mask [{0}], input [{1}]") > + public static Collection<Object[]> inputs() { > + List<Object[]> result = new ArrayList<>(); > + > + // Invalid IPv4 netmasks > + result.add(new Object[] { "260.1.1.1", null, Boolean.FALSE, null }); > + result.add(new Object[] { "1.2.3.4/foo", null, Boolean.FALSE, null > }); > + result.add(new Object[] { "1.2.3.4/-1", null, Boolean.FALSE, null }); > + result.add(new Object[] { "1.2.3.4/33", null, Boolean.FALSE, null }); > + > + // Invalid IPv6 netmasks > + result.add(new Object[] { "fffff::/71", null, Boolean.FALSE, null }); > + result.add(new Object[] { "ae31::27:ef2:1/foo", null, Boolean.FALSE, > null }); > + result.add(new Object[] { "ae31::27:ef2:1/-1", null, Boolean.FALSE, > null }); > + result.add(new Object[] { "ae31::27:ef2:1/129", null, Boolean.FALSE, > null }); > + > + // IPv4 > + result.add(new Object[] { "1.2.3.4/32", "1.2.3.3", Boolean.TRUE, > Boolean.FALSE }); > + result.add(new Object[] { "1.2.3.4/32", "1.2.3.4", Boolean.TRUE, > Boolean.TRUE }); > + result.add(new Object[] { "1.2.3.4/32", "1.2.3.5", Boolean.TRUE, > Boolean.FALSE }); > + > + result.add(new Object[] { "1.2.3.4/31", "1.2.3.3", Boolean.TRUE, > Boolean.FALSE }); > + result.add(new Object[] { "1.2.3.4/31", "1.2.3.4", Boolean.TRUE, > Boolean.TRUE }); > + result.add(new Object[] { "1.2.3.4/31", "1.2.3.5", Boolean.TRUE, > Boolean.TRUE }); > + result.add(new Object[] { "1.2.3.4/31", "1.2.3.6", Boolean.TRUE, > Boolean.FALSE }); > + > + return result; > + } > + > + > + @Test > + public void testNetMask() { > + Exception exception = null; > + NetMask netMask = null; > + try { > + netMask = new NetMask(mask); > + } catch (Exception e) { > + exception =e; > + } > + > + if (valid.booleanValue()) { > + Assert.assertNull(exception); > + Assert.assertNotNull(netMask); > + } else { > + Assert.assertNotNull(exception); > + Assert.assertEquals(IllegalArgumentException.class.getName(), > + exception.getClass().getName()); > + return; > + } > + > + InetAddress inetAddress = null; > + try { > + inetAddress = InetAddress.getByName(input); > + } catch (UnknownHostException e) { > + e.printStackTrace(); > + Assert.fail(); > + } > + > + Assert.assertEquals(matches, > Boolean.valueOf(netMask.matches(inetAddress))); > + } > +} > > Propchange: tomcat/trunk/test/org/apache/catalina/util/TestNetMask.java > ------------------------------------------------------------------------------ > svn:eol-style = native > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
