Overall cleanup of codec-dns

Related issue: #2688

- DnsClass and DnsType
  - Make DnsClass and DnsType implement Comparable
  - Optimize the message generation of IllegalArgumentException,
    by pre-populating the list of the expected parameters
  - Move the static methods up
  - Relax the validation rule of DnsClass so that a user can define a
    CLASS which is not listed in the RFC 1035
  - valueOf(int) does not throw IllegalArgumentException anymore as long
    as the specified value is an unsigned short.
  - Rename create() and forName() to valueOf() so that they look like a
    real enum
  - Rename type() and clazz() to intValue() so that they conform to our
    naming convention
- Add missing null checks in DnsEntry
This commit is contained in:
Trustin Lee 2014-07-23 14:33:35 -07:00
parent 9734170b7d
commit 4bde044957
7 changed files with 187 additions and 206 deletions

View File

@ -15,12 +15,10 @@
*/ */
package io.netty.handler.codec.dns; package io.netty.handler.codec.dns;
import java.util.Arrays;
/** /**
* Represents a class field in DNS protocol * Represents a class field in DNS protocol
*/ */
public class DnsClass { public final class DnsClass implements Comparable<DnsClass> {
/** /**
* Default class for DNS entries. * Default class for DNS entries.
@ -32,32 +30,55 @@ public class DnsClass {
public static final DnsClass NONE = new DnsClass(0x00fe, "NONE"); public static final DnsClass NONE = new DnsClass(0x00fe, "NONE");
public static final DnsClass ANY = new DnsClass(0x00ff, "ANY"); public static final DnsClass ANY = new DnsClass(0x00ff, "ANY");
/** private static final String EXPECTED =
* The protocol value of this DNS class " (expected: " +
*/ IN + '(' + IN.intValue() + "), " +
private final int clazz; CSNET + '(' + CSNET.intValue() + "), " +
/** CHAOS + '(' + CHAOS.intValue() + "), " +
* The name of this DNS class HESIOD + '(' + HESIOD.intValue() + "), " +
*/ NONE + '(' + NONE.intValue() + "), " +
private final String name; ANY + '(' + ANY.intValue() + "))";
DnsClass(int clazz, String name) { public static DnsClass valueOf(String name) {
this.clazz = clazz; if (IN.name().equals(name)) {
this.name = name; return IN;
}
if (NONE.name().equals(name)) {
return NONE;
}
if (ANY.name().equals(name)) {
return ANY;
}
if (CSNET.name().equals(name)) {
return CSNET;
}
if (CHAOS.name().equals(name)) {
return CHAOS;
}
if (HESIOD.name().equals(name)) {
return HESIOD;
}
throw new IllegalArgumentException("name: " + name + EXPECTED);
} }
/** public static DnsClass valueOf(int intValue) {
* Returns the name of this class as used in bind config files switch (intValue) {
*/ case 0x0001:
public final String name() { return IN;
return name; case 0x0002:
} return CSNET;
case 0x0003:
/** return CHAOS;
* Returns the protocol value represented by this class case 0x0004:
*/ return HESIOD;
public final int clazz() { case 0x00fe:
return clazz; return NONE;
case 0x00ff:
return ANY;
default:
return new DnsClass(intValue, "UNKNOWN");
}
} }
/** /**
@ -66,71 +87,60 @@ public class DnsClass {
* @param clazz The class * @param clazz The class
* @param name The name * @param name The name
*/ */
public static DnsClass create(int clazz, String name) { public static DnsClass valueOf(int clazz, String name) {
return new DnsClass(clazz, name); return new DnsClass(clazz, name);
} }
/** /**
* Returns true if this class is valid with respect to DNS protocol * The protocol value of this DNS class
*/ */
public boolean isValid() { private final int intValue;
if (clazz < 1 || clazz > 4 && clazz != NONE.clazz && clazz != ANY.clazz) {
return false; /**
* The name of this DNS class
*/
private final String name;
private DnsClass(int intValue, String name) {
if ((intValue & 0xffff) != intValue) {
throw new IllegalArgumentException("intValue: " + intValue + " (expected: 0 ~ 65535)");
} }
return true;
this.intValue = intValue;
this.name = name;
} }
public static DnsClass forName(String name) { /**
if (IN.name.equals(name)) { * Returns the name of this class as used in bind config files
return IN; */
} else if (NONE.name().equals(name)) { public String name() {
return NONE; return name;
} else if (ANY.name().equals(name)) {
return ANY;
} else if (CSNET.name().equals(name)) {
return CSNET;
} else if (CHAOS.name().equals(name)) {
return CHAOS;
} else if (HESIOD.name().equals(name)) {
return HESIOD;
}
throw new IllegalArgumentException("name: " + name + " (expected: "
+ "IN, ANY, CSNET, CHAOS, HESIOD)");
} }
public static DnsClass valueOf(int clazz) { /**
switch (clazz) { * Returns the protocol value represented by this class
case 0x0001: */
return IN; public int intValue() {
case 0x0002: return intValue;
return CSNET;
case 0x0003:
return CHAOS;
case 0x0004:
return HESIOD;
case 0x00fe:
return NONE;
case 0x00ff:
return ANY;
default:
throw new IllegalArgumentException("clazz: " + clazz + " (expected: "
+ Arrays.asList(new Integer[] {IN.clazz, CSNET.clazz,
CHAOS.clazz, HESIOD.clazz, NONE.clazz, ANY.clazz}) + ")");
}
} }
@Override @Override
public final int hashCode() { public int hashCode() {
return clazz; return intValue;
} }
@Override @Override
public final boolean equals(Object o) { public boolean equals(Object o) {
return o instanceof DnsClass && ((DnsClass) o).clazz == clazz; return o instanceof DnsClass && ((DnsClass) o).intValue == intValue;
} }
@Override @Override
public final String toString() { public int compareTo(DnsClass o) {
return intValue() - o.intValue();
}
@Override
public String toString() {
return name; return name;
} }
} }

View File

@ -15,6 +15,8 @@
*/ */
package io.netty.handler.codec.dns; package io.netty.handler.codec.dns;
import io.netty.util.internal.StringUtil;
/** /**
* A class representing entries in a DNS packet (questions, and all resource * A class representing entries in a DNS packet (questions, and all resource
* records). Contains data shared by entries such as name, type, and class. * records). Contains data shared by entries such as name, type, and class.
@ -30,9 +32,13 @@ public class DnsEntry {
if (name == null) { if (name == null) {
throw new NullPointerException("name"); throw new NullPointerException("name");
} }
if (!dnsClass.isValid()) { if (type == null) {
throw new IllegalArgumentException("an invalid class has been supplied."); throw new NullPointerException("type");
} }
if (dnsClass == null) {
throw new NullPointerException("dnsClass");
}
this.name = name; this.name = name;
this.type = type; this.type = type;
this.dnsClass = dnsClass; this.dnsClass = dnsClass;
@ -66,8 +72,11 @@ public class DnsEntry {
@Override @Override
public String toString() { public String toString() {
return new StringBuilder(32).append(getClass().getSimpleName()).append("(domain name: ").append(name) return new StringBuilder(128).append(StringUtil.simpleClassName(this))
.append(", type: ").append(type).append(", class: ").append(dnsClass).append(')').toString(); .append("(name: ").append(name)
.append(", type: ").append(type)
.append(", class: ").append(dnsClass)
.append(')').toString();
} }
@Override @Override
@ -75,10 +84,13 @@ public class DnsEntry {
if (this == o) { if (this == o) {
return true; return true;
} }
if (o instanceof DnsEntry) { if (!(o instanceof DnsEntry)) {
DnsEntry other = (DnsEntry) o; return false;
return other.type() == type && other.dnsClass() == dnsClass && other.name().equals(name);
} }
return false;
DnsEntry that = (DnsEntry) o;
return type().intValue() == that.type().intValue() &&
dnsClass().intValue() == that.dnsClass().intValue() &&
name().equals(that.name());
} }
} }

View File

@ -85,7 +85,7 @@ public class DnsQueryEncoder extends MessageToMessageEncoder<DnsQuery> {
buf.writeBytes(part.getBytes(charset)); buf.writeBytes(part.getBytes(charset));
} }
buf.writeByte(0); // marks end of name field buf.writeByte(0); // marks end of name field
buf.writeShort(question.type().type()); buf.writeShort(question.type().intValue());
buf.writeShort(question.dnsClass().clazz()); buf.writeShort(question.dnsClass().intValue());
} }
} }

View File

@ -1,40 +0,0 @@
/*
* Copyright 2014 The Netty Project
*
* The Netty Project 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 io.netty.handler.codec.dns;
import io.netty.handler.codec.DecoderException;
public final class DnsResponseException extends DecoderException {
private static final long serialVersionUID = -8519053051363525286L;
private final DnsResponseCode code;
public DnsResponseException(DnsResponseCode code) {
if (code == null) {
throw new NullPointerException("code");
}
this.code = code;
}
/**
* The {@link DnsResponseCode} which caused this {@link DnsResponseException} to be created.
*/
public DnsResponseCode responseCode() {
return code;
}
}

View File

@ -22,7 +22,7 @@ import java.util.Map;
/** /**
* Represents a DNS record type. * Represents a DNS record type.
*/ */
public class DnsType { public final class DnsType implements Comparable<DnsType> {
/** /**
* Address record RFC 1035 Returns a 32-bit IPv4 address, most commonly used * Address record RFC 1035 Returns a 32-bit IPv4 address, most commonly used
@ -300,101 +300,99 @@ public class DnsType {
*/ */
public static final DnsType DLV = new DnsType(0x8001, "DLV"); public static final DnsType DLV = new DnsType(0x8001, "DLV");
private static final Map<String, DnsType> BY_NAME private static final Map<String, DnsType> BY_NAME = new HashMap<String, DnsType>();
= new HashMap<String, DnsType>(); private static final IntObjectHashMap<DnsType> BY_TYPE = new IntObjectHashMap<DnsType>();
private static final String EXPECTED;
private static final IntObjectHashMap<DnsType> BY_TYPE
= new IntObjectHashMap<DnsType>();
static { static {
DnsType[] all = {A, NS, CNAME, SOA, PTR, MX, DnsType[] all = {
TXT, RP, AFSDB, SIG, KEY, AAAA, LOC, A, NS, CNAME, SOA, PTR, MX, TXT, RP, AFSDB, SIG, KEY, AAAA, LOC, SRV, NAPTR, KX, CERT, DNAME, OPT, APL,
SRV, NAPTR, KX, CERT, DNAME, OPT, DS, SSHFP, IPSECKEY, RRSIG, NSEC, DNSKEY, DHCID, NSEC3, NSEC3PARAM, TLSA, HIP, SPF, TKEY, TSIG, IXFR,
APL, DS, SSHFP, IPSECKEY, RRSIG, NSEC, AXFR, ANY, CAA, TA, DLV
DNSKEY, DHCID, NSEC3, NSEC3PARAM, TLSA, };
HIP, SPF, TKEY, TSIG, IXFR, AXFR,
ANY, CAA, TA, DLV}; StringBuilder expected = new StringBuilder(512);
for (DnsType type : all) { expected.append(" (expected: ");
for (DnsType type: all) {
BY_NAME.put(type.name(), type); BY_NAME.put(type.name(), type);
BY_TYPE.put(type.type(), type); BY_TYPE.put(type.intValue(), type);
expected.append(type.name());
expected.append('(');
expected.append(type.intValue());
expected.append("), ");
} }
expected.setLength(expected.length() - 2);
expected.append(')');
EXPECTED = expected.toString();
} }
private final int type; public static DnsType valueOf(int intValue) {
DnsType result = BY_TYPE.get(intValue);
if (result == null) {
return new DnsType(intValue, "UNKNOWN");
}
return result;
}
public static DnsType valueOf(String name) {
DnsType result = BY_NAME.get(name);
if (result == null) {
throw new IllegalArgumentException("name: " + name + EXPECTED);
}
return result;
}
/**
* Returns a new instance.
*/
public static DnsType valueOf(int intValue, String name) {
return new DnsType(intValue, name);
}
private final int intValue;
private final String name; private final String name;
DnsType(int type, String name) { private DnsType(int intValue, String name) {
if ((type & 0xffff) != type) { if ((intValue & 0xffff) != intValue) {
throw new IllegalArgumentException("type: " + type throw new IllegalArgumentException("intValue: " + intValue + " (expected: 0 ~ 65535)");
+ " (expected: 0 ~ 65535)");
} }
this.type = type; this.intValue = intValue;
this.name = name; this.name = name;
} }
/** /**
* Returns the name of this type, as seen in bind config files * Returns the name of this type, as seen in bind config files
*/ */
public final String name() { public String name() {
return name; return name;
} }
/** /**
* Returns the value of this DnsType as it appears in DNS protocol * Returns the value of this DnsType as it appears in DNS protocol
*/ */
public final int type() { public int intValue() {
return type; return intValue;
}
/**
* Returns a new DnsType instance
*/
public static DnsType create(int type, String name) {
return new DnsType(type, name);
} }
@Override @Override
public final int hashCode() { public int hashCode() {
return type; return intValue;
} }
@Override @Override
public final boolean equals(Object o) { public boolean equals(Object o) {
return o instanceof DnsType && ((DnsType) o).type == type; return o instanceof DnsType && ((DnsType) o).intValue == intValue;
} }
@Override @Override
public final String toString() { public int compareTo(DnsType o) {
return intValue() - o.intValue();
}
@Override
public String toString() {
return name; return name;
} }
public static DnsType valueOf(int type) {
DnsType result = BY_TYPE.get(type);
if (result == null) {
throw new IllegalArgumentException("type: " + type
+ "(expected: " + keysString() + ")");
}
return result;
}
private static CharSequence keysString() {
// Replace with Objects.toString(BY_TYPE.keys()) when JDK 7 supported
StringBuilder sb = new StringBuilder();
for (int key : BY_TYPE.keys()) {
if (sb.length() > 0) {
sb.append(", ");
}
sb.append(key);
}
return sb;
}
public static DnsType forName(String name) {
DnsType result = BY_NAME.get(name);
if (result == null) {
throw new IllegalArgumentException("name: " + name
+ " (expected: " + BY_NAME.keySet() + ")");
}
return result;
}
} }

View File

@ -15,23 +15,22 @@
*/ */
package io.netty.handler.codec.dns; package io.netty.handler.codec.dns;
import org.junit.Test;
import java.lang.reflect.Field; import java.lang.reflect.Field;
import java.lang.reflect.Modifier; import java.lang.reflect.Modifier;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse; import static org.junit.Assert.*;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertSame;
import org.junit.Test;
public class DnsClassTest { public class DnsClassTest {
private List<DnsClass> allTypes() throws Exception { private static List<DnsClass> allTypes() throws Exception {
List<DnsClass> result = new ArrayList<DnsClass>(); List<DnsClass> result = new ArrayList<DnsClass>();
for (Field field : DnsClass.class.getDeclaredFields()) { for (Field field : DnsClass.class.getDeclaredFields()) {
if ((field.getModifiers() & Modifier.STATIC) != 0) { if ((field.getModifiers() & Modifier.STATIC) != 0 && field.getType() == DnsClass.class) {
result.add((DnsClass) field.get(null)); result.add((DnsClass) field.get(null));
} }
} }
@ -42,7 +41,7 @@ public class DnsClassTest {
@Test @Test
public void testSanity() throws Exception { public void testSanity() throws Exception {
assertEquals("More than one type has the same int value", assertEquals("More than one type has the same int value",
allTypes().size(), new HashSet(allTypes()).size()); allTypes().size(), new HashSet<DnsClass>(allTypes()).size());
} }
/** /**
@ -51,7 +50,7 @@ public class DnsClassTest {
@Test @Test
public void testHashCode() throws Exception { public void testHashCode() throws Exception {
for (DnsClass t : allTypes()) { for (DnsClass t : allTypes()) {
assertEquals(t.clazz(), t.hashCode()); assertEquals(t.intValue(), t.hashCode());
} }
} }
@ -75,9 +74,9 @@ public class DnsClassTest {
@Test @Test
public void testFind() throws Exception { public void testFind() throws Exception {
for (DnsClass t : allTypes()) { for (DnsClass t : allTypes()) {
DnsClass found = DnsClass.valueOf(t.clazz()); DnsClass found = DnsClass.valueOf(t.intValue());
assertSame(t, found); assertSame(t, found);
found = DnsClass.forName(t.toString()); found = DnsClass.valueOf(t.toString());
assertSame(t.toString(), t, found); assertSame(t.toString(), t, found);
} }
} }

View File

@ -15,20 +15,22 @@
*/ */
package io.netty.handler.codec.dns; package io.netty.handler.codec.dns;
import org.junit.Test;
import java.lang.reflect.Field; import java.lang.reflect.Field;
import java.lang.reflect.Modifier; import java.lang.reflect.Modifier;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
import org.junit.Test;
import static org.junit.Assert.*; import static org.junit.Assert.*;
public class DnsTypeTest { public class DnsTypeTest {
private List<DnsType> allTypes() throws Exception { private static List<DnsType> allTypes() throws Exception {
List<DnsType> result = new ArrayList<DnsType>(); List<DnsType> result = new ArrayList<DnsType>();
for (Field field : DnsType.class.getFields()) { for (Field field : DnsType.class.getFields()) {
if ((field.getModifiers() & Modifier.STATIC) != 0 && DnsType.class == field.getType()) { if ((field.getModifiers() & Modifier.STATIC) != 0 && field.getType() == DnsType.class) {
result.add((DnsType) field.get(null)); result.add((DnsType) field.get(null));
} }
} }
@ -39,7 +41,7 @@ public class DnsTypeTest {
@Test @Test
public void testSanity() throws Exception { public void testSanity() throws Exception {
assertEquals("More than one type has the same int value", assertEquals("More than one type has the same int value",
allTypes().size(), new HashSet(allTypes()).size()); allTypes().size(), new HashSet<DnsType>(allTypes()).size());
} }
/** /**
@ -48,7 +50,7 @@ public class DnsTypeTest {
@Test @Test
public void testHashCode() throws Exception { public void testHashCode() throws Exception {
for (DnsType t : allTypes()) { for (DnsType t : allTypes()) {
assertEquals(t.type(), t.hashCode()); assertEquals(t.intValue(), t.hashCode());
} }
} }
@ -72,9 +74,9 @@ public class DnsTypeTest {
@Test @Test
public void testFind() throws Exception { public void testFind() throws Exception {
for (DnsType t : allTypes()) { for (DnsType t : allTypes()) {
DnsType found = DnsType.valueOf(t.type()); DnsType found = DnsType.valueOf(t.intValue());
assertSame(t, found); assertSame(t, found);
found = DnsType.forName(t.toString()); found = DnsType.valueOf(t.toString());
assertSame(t.toString(), t, found); assertSame(t.toString(), t, found);
} }
} }