On 5 November 2015 at 11:38, Benedikt Ritter <brit...@apache.org> wrote: > 2015-11-05 1:42 GMT+01:00 <s...@apache.org>: > >> Author: sebb >> Date: Thu Nov 5 00:42:37 2015 >> New Revision: 1712699 >> >> URL: http://svn.apache.org/viewvc?rev=1712699&view=rev >> Log: >> VALIDATOR-341 Make TLD list configurable >> >> Modified: >> commons/proper/validator/trunk/src/changes/changes.xml >> >> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java >> >> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/DomainValidatorTest.java >> >> Modified: commons/proper/validator/trunk/src/changes/changes.xml >> URL: >> http://svn.apache.org/viewvc/commons/proper/validator/trunk/src/changes/changes.xml?rev=1712699&r1=1712698&r2=1712699&view=diff >> >> ============================================================================== >> --- commons/proper/validator/trunk/src/changes/changes.xml (original) >> +++ commons/proper/validator/trunk/src/changes/changes.xml Thu Nov 5 >> 00:42:37 2015 >> @@ -43,6 +43,10 @@ The <action> type attribute can be add,u >> <body> >> >> <release version="1.5.0" date="tba" description="tba"> >> + <action issue="VALIDATOR-341" type="add" dev="sebb"> >> + Make TLD list configurable; >> + both generic and country-code now support addition and removal >> + </action> >> <action issue="VALIDATOR-374" type="fix"> >> Email Validator : .school domain is being rejected >> Add Unit test to show it has been fixed >> >> Modified: >> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java >> URL: >> http://svn.apache.org/viewvc/commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java?rev=1712699&r1=1712698&r2=1712699&view=diff >> >> ============================================================================== >> --- >> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java >> (original) >> +++ >> commons/proper/validator/trunk/src/main/java/org/apache/commons/validator/routines/DomainValidator.java >> Thu Nov 5 00:42:37 2015 >> @@ -63,6 +63,8 @@ import java.util.Locale; >> */ >> public class DomainValidator implements Serializable { >> >> + private static final String[] EMPTY_STRING_ARRAY = new String[0]; >> + >> private static final long serialVersionUID = -4407125112880174009L; >> >> // Regular expression strings for hostnames (derived from RFC2396 and >> RFC 1123) >> @@ -219,7 +221,8 @@ public class DomainValidator implements >> */ >> public boolean isValidGenericTld(String gTld) { >> final String key = >> chompLeadingDot(unicodeToASCII(gTld).toLowerCase(Locale.ENGLISH)); >> - return arrayContains(GENERIC_TLDS, key); >> + return (arrayContains(GENERIC_TLDS, key) || >> arrayContains(GENERIC_TLDS_PLUS, key)) >> + && !arrayContains(GENERIC_TLDS_MINUS, key); >> } >> >> /** >> @@ -231,7 +234,8 @@ public class DomainValidator implements >> */ >> public boolean isValidCountryCodeTld(String ccTld) { >> final String key = >> chompLeadingDot(unicodeToASCII(ccTld).toLowerCase(Locale.ENGLISH)); >> - return arrayContains(COUNTRY_CODE_TLDS, key); >> + return (arrayContains(COUNTRY_CODE_TLDS, key) || >> arrayContains(COUNTRY_CODE_TLDS_PLUS, key)) >> + && !arrayContains(COUNTRY_CODE_TLDS_MINUS, key); >> } >> >> /** >> @@ -1385,6 +1389,68 @@ public class DomainValidator implements >> "localhost", // RFC2606 defined >> }; >> >> + // Additional arrays to supplement or override the built in ones. >> + // The PLUS arrays are valid keys, the MINUS arrays are invalid keys >> + // The arrays are marked volatile because they are not immutable >> + >> + // WARNING: this array MUST be sorted, otherwise it cannot be >> searched reliably using binary search >> + private static volatile String[] COUNTRY_CODE_TLDS_PLUS = >> EMPTY_STRING_ARRAY; >> + >> + // WARNING: this array MUST be sorted, otherwise it cannot be >> searched reliably using binary search >> + private static volatile String[] GENERIC_TLDS_PLUS = >> EMPTY_STRING_ARRAY; >> + >> + // WARNING: this array MUST be sorted, otherwise it cannot be >> searched reliably using binary search >> + private static volatile String[] COUNTRY_CODE_TLDS_MINUS = >> EMPTY_STRING_ARRAY; >> + >> + // WARNING: this array MUST be sorted, otherwise it cannot be >> searched reliably using binary search >> + private static volatile String[] GENERIC_TLDS_MINUS = >> EMPTY_STRING_ARRAY; >> + >> + enum ArrayType { >> + GENERIC_PLUS, >> + GENERIC_MINUS, >> + COUNTRY_CODE_PLUS, >> + COUNTRY_CODE_MINUS; >> + }; >> + >> + // For use by unit test code >> + static void clearTLDOverrides() { >> + COUNTRY_CODE_TLDS_PLUS = EMPTY_STRING_ARRAY; >> + COUNTRY_CODE_TLDS_MINUS = EMPTY_STRING_ARRAY; >> + GENERIC_TLDS_PLUS = EMPTY_STRING_ARRAY; >> + GENERIC_TLDS_MINUS = EMPTY_STRING_ARRAY; >> + } >> + /** >> + * Update one of the TLD override arrays. >> + * This should normally only be done at program startup. >> + * >> + * To clear an override array, provide an empty array. >> + * >> + * @param table the table to update >> + * @param tlds the array of TLDs, must not be null >> + */ >> + public static void updateTLDOverride(ArrayType table, String [] tlds) >> { >> + String [] copy = new String[tlds.length]; >> + // Comparisons are always done with lower-case entries >> + for (int i = 0; i < tlds.length; i++) { >> + copy[i] = tlds[i].toLowerCase(Locale.ENGLISH); >> + } >> + Arrays.sort(copy); >> + switch(table) { >> + case COUNTRY_CODE_MINUS: >> + COUNTRY_CODE_TLDS_MINUS = copy; >> + break; >> + case COUNTRY_CODE_PLUS: >> + COUNTRY_CODE_TLDS_PLUS = copy; >> + break; >> + case GENERIC_MINUS: >> + GENERIC_TLDS_MINUS = copy; >> + break; >> + case GENERIC_PLUS: >> + GENERIC_TLDS_PLUS = copy; >> + break; >> + } >> + } >> + >> /** >> * Converts potentially Unicode input to punycode. >> * If conversion fails, returns the original input. >> >> Modified: >> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/DomainValidatorTest.java >> URL: >> http://svn.apache.org/viewvc/commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/DomainValidatorTest.java?rev=1712699&r1=1712698&r2=1712699&view=diff >> >> ============================================================================== >> --- >> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/DomainValidatorTest.java >> (original) >> +++ >> commons/proper/validator/trunk/src/test/java/org/apache/commons/validator/routines/DomainValidatorTest.java >> Thu Nov 5 00:42:37 2015 >> @@ -42,6 +42,8 @@ import java.util.TreeMap; >> import java.util.regex.Matcher; >> import java.util.regex.Pattern; >> >> +import org.apache.commons.validator.routines.DomainValidator.ArrayType; >> + >> import junit.framework.TestCase; >> >> /** >> @@ -55,6 +57,7 @@ public class DomainValidatorTest extends >> >> public void setUp() { >> validator = DomainValidator.getInstance(); >> + DomainValidator.clearTLDOverrides(); >> } >> >> public void testValidDomains() { >> @@ -296,6 +299,34 @@ public class DomainValidatorTest extends >> assertTrue(sorted); >> } >> >> + public void testUpdateCountryCode() { >> + assertFalse(validator.isValidCountryCodeTld("com")); // cannot be >> valid >> + DomainValidator.updateTLDOverride(ArrayType.COUNTRY_CODE_PLUS, >> new String[]{"com"}); >> + assertTrue(validator.isValidCountryCodeTld("com")); // it is now! >> + DomainValidator.updateTLDOverride(ArrayType.COUNTRY_CODE_MINUS, >> new String[]{"com"}); >> + assertFalse(validator.isValidCountryCodeTld("com")); // show that >> minus overrides the rest >> + >> + assertTrue(validator.isValidCountryCodeTld("ch")); >> + DomainValidator.updateTLDOverride(ArrayType.COUNTRY_CODE_MINUS, >> new String[]{"ch"}); >> + assertFalse(validator.isValidCountryCodeTld("ch")); >> + DomainValidator.updateTLDOverride(ArrayType.COUNTRY_CODE_MINUS, >> new String[]{"xx"}); >> + assertTrue(validator.isValidCountryCodeTld("ch")); >> + } >> + >> + public void testUpdateGeneric() { >> + assertFalse(validator.isValidGenericTld("ch")); // cannot be valid >> + DomainValidator.updateTLDOverride(ArrayType.GENERIC_PLUS, new >> String[]{"ch"}); >> + assertTrue(validator.isValidGenericTld("ch")); // it is now! >> + DomainValidator.updateTLDOverride(ArrayType.GENERIC_MINUS, new >> String[]{"ch"}); >> + assertFalse(validator.isValidGenericTld("ch")); // show that >> minus overrides the rest >> + >> + assertTrue(validator.isValidGenericTld("com")); >> + DomainValidator.updateTLDOverride(ArrayType.GENERIC_MINUS, new >> String[]{"com"}); >> + assertFalse(validator.isValidGenericTld("com")); >> + DomainValidator.updateTLDOverride(ArrayType.GENERIC_MINUS, new >> String[]{"xx"}); // change the minus list >> + assertTrue(validator.isValidGenericTld("com")); >> + } >> + >> // Download and process local copy of >> http://data.iana.org/TLD/tlds-alpha-by-domain.txt >> // Check if the internal TLD table is up to date >> // Check if the internal TLD tables have any spurious entries >> >> >> > I don't think it is a good idea to implement this based on static fields. > It would be better to make DomainValidator instances configurable via the > constructor. Has this been implemented this way, because other validators > use DomainValidator via Singleton access?
Yes, the existing code uses static methods on the class. The patch just allows the user to update the static data which is used for the validation. In any case, I don't think it makes sense for different threads to have different base data - either "xyz" is a valid domain or it is not, and this applies for all users of the code, until such time as a new domain is registered (or dropped). So I cannot see a use case for using multiple instances. As the Javadoc says, the intention is to allow the user to update the base data set at the start of an application. The alternative is to wait for the next release of the class, or rebuild/replace the class locally. [The code uses a pre-sorted database - rather than parsing a separate datafile - because that speeds up startup. But that is orthogonal to the question of whether the class should be a singleton.] > Benedikt > > > > -- > http://people.apache.org/~britter/ > http://www.systemoutprintln.de/ > http://twitter.com/BenediktRitter > http://github.com/britter --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
