From dev-return-191014-archive-asf-public=cust-asf.ponee.io@tomcat.apache.org Wed May 23 22:35:28 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id 57EFD180645 for ; Wed, 23 May 2018 22:35:27 +0200 (CEST) Received: (qmail 14249 invoked by uid 500); 23 May 2018 20:35:26 -0000 Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: "Tomcat Developers List" Delivered-To: mailing list dev@tomcat.apache.org Received: (qmail 14238 invoked by uid 99); 23 May 2018 20:35:26 -0000 Received: from mail-relay.apache.org (HELO mailrelay1-lw-us.apache.org) (207.244.88.152) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 23 May 2018 20:35:26 +0000 Received: from [192.168.23.12] (host81-156-46-215.range81-156.btcentralplus.com [81.156.46.215]) by mailrelay1-lw-us.apache.org (ASF Mail Server at mailrelay1-lw-us.apache.org) with ESMTPSA id 20FF6C3D for ; Wed, 23 May 2018 20:35:24 +0000 (UTC) Subject: Re: svn commit: r1832124 - in /tomcat/trunk: java/org/apache/catalina/util/LocalStrings.properties java/org/apache/catalina/util/NetMask.java test/org/apache/catalina/util/TestNetMask.java From: Mark Thomas To: dev@tomcat.apache.org Reply-To: Tomcat Developers List References: <20180523203336.1D1BD3A0044@svn01-us-west.apache.org> Message-ID: <7f2776ad-6573-8bdd-c8c0-b138609e8d7c@apache.org> Date: Wed, 23 May 2018 21:35:23 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180523203336.1D1BD3A0044@svn01-us-west.apache.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit On 23/05/18 21:33, markt@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. > + * > + *

> + * 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. > + *

> + * > + *

> + * As byte arrays as returned by InetAddress.getByName() 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). > + *

> + * > + *

> + * As a bonus, if no '/' is found in the input, it is assumed that an exact > + * address match is required. > + *

> + */ > +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 inputs() { > + List 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-unsubscribe@tomcat.apache.org > For additional commands, e-mail: dev-help@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org For additional commands, e-mail: dev-help@tomcat.apache.org