Have constructing relative Name with length() == MAXNAME fail

nresare · nresare · commit 0697e5b9482a · 2021-02-16T16:14:12.000Z
The Name() constructor will currently fail if it's length() would
end up being longer than MAXNAME (255). However, there is an edge
case where a relative name can be exactly MAXNAME long. This is
legal but the lookup mechanism will then fail to make the name
absolute by appending an empty label to be able to use it for
lookups.
diff --git a/src/main/java/org/xbill/DNS/Name.java b/src/main/java/org/xbill/DNS/Name.java
@@ -271,6 +271,14 @@ public Name(String s, Name origin) throws TextParseException {
     if (origin != null && !absolute) {
       appendFromString(s, origin.name, origin.offset(0), origin.labels);
     }
+    // A relative name that is MAXNAME octets long is a strange and wonderful thing.
+    // Not technically in violation, but it can not be used for queries as it needs
+    // to be made absolute by appending at the very least the an empty label at the
+    // end, which there is no room for. To make life easier for everyone, let's only
+    // allow Names that are MAXNAME long if they are absolute.
+    if (!absolute && length() == MAXNAME) {
+      throw parseException(s, "Name too long");
+    }
   }
 
   /**
diff --git a/src/test/java/org/xbill/DNS/NameTest.java b/src/test/java/org/xbill/DNS/NameTest.java
@@ -34,6 +34,7 @@
 //
 package org.xbill.DNS;
 
+import static java.lang.String.format;
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -45,6 +46,8 @@
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.io.IOException;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -286,15 +289,26 @@ void ctor_toobig_label() {
     }
 
     @Test
-    void ctor_max_length_rel() throws TextParseException {
-      // relative name with three 63-char labels and a 62-char label
+    void ctor_length_too_long_rel() {
+      // We want to fail to crate this as there is no way to make a Name that long absolute,
+      // which means that it can not be used for lookups or updates.
+      String label63 = IntStream.range(0, 63).mapToObj(i -> "a").collect(Collectors.joining());
+      String label62 = IntStream.range(0, 62).mapToObj(i -> "a").collect(Collectors.joining());
+      assertThrows(
+          TextParseException.class,
+          () -> new Name(format("%s.%s.%s.%s", label63, label63, label63, label62)));
+    }
+
+    @Test
+    void ctor_longest_allowed_rel() throws TextParseException {
+      // relative name with three 63-char labels and a 61-char label
       Name n =
           new Name(
-              "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.ccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc.dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd");
+              "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb.ccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc.ddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd");
       assertFalse(n.isAbsolute());
       assertFalse(n.isWild());
       assertEquals(4, n.labels());
-      assertEquals(255, n.length());
+      assertEquals(254, n.length());
     }
 
     @Test

Original file line number	Diff line number	Diff line change
`@@ -271,6 +271,14 @@ public Name(String s, Name origin) throws TextParseException {`
`271`	`271`	`if (origin != null && !absolute) {`
`272`	`272`	`appendFromString(s, origin.name, origin.offset(0), origin.labels);`
`273`	`273`	`}`
	`274`	`+ // A relative name that is MAXNAME octets long is a strange and wonderful thing.`
	`275`	`+ // Not technically in violation, but it can not be used for queries as it needs`
	`276`	`+ // to be made absolute by appending at the very least the an empty label at the`
	`277`	`+ // end, which there is no room for. To make life easier for everyone, let's only`
	`278`	`+ // allow Names that are MAXNAME long if they are absolute.`
	`279`	`+ if (!absolute && length() == MAXNAME) {`
	`280`	`+ throw parseException(s, "Name too long");`
	`281`	`+ }`
`274`	`282`	`}`
`275`	`283`
`276`	`284`	`/**`