Apply a 64k limit to documentids.

4 files changed, 59 insertions, 7 deletions

diff --git a/document/src/main/java/com/yahoo/document/DocumentId.java b/document/src/main/java/com/yahoo/document/DocumentId.java
index e7c904cc420..30ca9be8231 100644
--- a/document/src/main/java/com/yahoo/document/DocumentId.java
+++ b/document/src/main/java/com/yahoo/document/DocumentId.java
@@ -33,6 +33,11 @@ public class DocumentId extends Identifiable implements Serializable {
         if (id == null) {
             throw new IllegalArgumentException("Cannot create DocumentId from null id.");
         }
+        if (id.length() > IdString.MAX_LENGTH) {
+            throw new IllegalArgumentException("The document id(" + id.length() + ") is too long(65536). " +
+                    "However if you have already fed a document earlier on and want to remove it, you can do so by " +
+                    "calling new DocumentId(IdString.createIdStringLessStrict()) that will bypass this restriction.");
+        }
         this.id = IdString.createIdString(id);
         globalId = null;
     }
@@ -55,8 +60,7 @@ public class DocumentId extends Identifiable implements Serializable {
 
     @Override
     public DocumentId clone() {
-        DocumentId docId =  (DocumentId)super.clone();
-        return docId;
+        return  (DocumentId)super.clone();
     }
 
     public void setId(IdString id) {
diff --git a/document/src/main/java/com/yahoo/document/idstring/IdString.java b/document/src/main/java/com/yahoo/document/idstring/IdString.java
index 2114a480ec3..404db928f30 100644
--- a/document/src/main/java/com/yahoo/document/idstring/IdString.java
+++ b/document/src/main/java/com/yahoo/document/idstring/IdString.java
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.document.idstring;
 
+import com.google.common.annotations.Beta;
 import com.yahoo.text.Text;
 import com.yahoo.text.Utf8String;
 
@@ -43,7 +44,8 @@ public abstract class IdString {
     private final String namespaceSpecific;
     private Utf8String cache;
     // This max unsigned 16 bit integer - 1 as the offset will be length + 1
-    static final int MAX_LENGTH_EXCEPT_NAMESPACE_SPECIFIC = 0xfffe;
+    static final int MAX_LENGTH_EXCEPT_NAMESPACE_SPECIFIC = 0xff00;
+    public static final int MAX_LENGTH = 0x10000;
 
     /**
      * Creates a IdString based on the given document id string.
@@ -51,6 +53,19 @@ public abstract class IdString {
      * The document id string can only contain text characters.
      */
     public static IdString createIdString(String id) {
+        if (id.length() >= MAX_LENGTH) {
+            throw new IllegalArgumentException("Document id length " + id.length() + " is longer than max length of " + MAX_LENGTH);
+        }
+        validateTextString(id);
+        return parseAndCreate(id);
+    }
+
+    /**
+     * Creates a IdString based on the given document id string. This is a less strict variant
+     * for creating 'illegal' document ids for documents already fed. Only use when strictly needed.
+     */
+    @Beta
+    public static IdString createIdStringLessStrict(String id) {
         validateTextString(id);
         return parseAndCreate(id);
     }
diff --git a/document/src/test/java/com/yahoo/document/DocumentIdTestCase.java b/document/src/test/java/com/yahoo/document/DocumentIdTestCase.java
index a0c5da52ff7..4cc4e75ea78 100644
--- a/document/src/test/java/com/yahoo/document/DocumentIdTestCase.java
+++ b/document/src/test/java/com/yahoo/document/DocumentIdTestCase.java
@@ -2,6 +2,7 @@
 package com.yahoo.document;
 
 import com.yahoo.document.idstring.IdIdString;
+import com.yahoo.document.idstring.IdString;
 import com.yahoo.vespa.objects.BufferSerializer;
 import org.junit.Before;
 import org.junit.Rule;
@@ -256,4 +257,20 @@ public class DocumentIdTestCase {
         }
     }
 
+    @Test
+    public void testTooLongDocId() {
+        StringBuilder sb = new StringBuilder("id:ns:type::");
+        for(int i=0; i < 0x10000; i++) {
+            sb.append('x');
+        }
+        try {
+            new DocumentId(sb.toString());
+            fail("Expected an IllegalArgumentException to be thrown");
+        } catch (IllegalArgumentException ex) {
+            assertTrue(ex.getMessage().contains("However if you have already fed a document earlier on and want to remove it, " +
+                    "you can do so by calling new DocumentId(IdString.createIdStringLessStrict()) that will bypass this restriction."));
+        }
+        assertEquals(65548, new DocumentId(IdString.createIdStringLessStrict(sb.toString())).toString().length());
+    }
+
 }
diff --git a/document/src/test/java/com/yahoo/document/IdIdStringTest.java b/document/src/test/java/com/yahoo/document/IdIdStringTest.java
index 7915f2c8b45..a94c3445d27 100644
--- a/document/src/test/java/com/yahoo/document/IdIdStringTest.java
+++ b/document/src/test/java/com/yahoo/document/IdIdStringTest.java
@@ -67,9 +67,25 @@ public class IdIdStringTest {
     }
 
     @Test
+    public void requireTooLongIdThrowsWhileParsing() throws Exception {
+        StringBuilder builder = new StringBuilder("id:ns:type::namespacespecificpart_01");
+        for (int i = 0; i < 0x10000; i++) {
+            builder.append('n');
+        }
+        try {
+            IdString.createIdString(builder.toString());
+            fail();
+        } catch (IllegalArgumentException e) {
+            assertEquals("Document id length 65572 is longer than max length of 65536", e.getMessage());
+        }
+        // But there is a backdor
+        assertEquals(65572, IdString.createIdStringLessStrict(builder.toString()).toString().length());
+    }
+
+    @Test
     public void requireThatTooLongPreNamespaceSpecificThrowsWhileParsing() throws Exception {
         StringBuilder builder = new StringBuilder("id:");
-        for (int i = 0; i < 0x10000; i++) {
+        for (int i = 0; i < 0xff00; i++) {
             builder.append('n');
         }
         builder.append(":type::namespacespecificpart_01");
@@ -77,20 +93,20 @@ public class IdIdStringTest {
             IdString.createIdString(builder.toString());
             fail();
         } catch (IllegalArgumentException e) {
-            assertEquals("Document id prior to the namespace specific part, 65545, is longer than 65534", e.getMessage().substring(0, 77));
+            assertEquals("Document id prior to the namespace specific part, 65289, is longer than 65280", e.getMessage().substring(0, 77));
         }
     }
     @Test
     public void requireThatTooLongPreNamespaceSpecificThrowsOnConstruction() {
         StringBuilder builder = new StringBuilder();
-        for (int i = 0; i < 0x10000; i++) {
+        for (int i = 0; i < 0xff00; i++) {
             builder.append('n');
         }
         try {
             new IdIdString(builder.toString(), "type", "", "namespacespecificpart_01");
             fail();
         } catch (IllegalArgumentException e) {
-            assertEquals("Length of namespace(65536) + doctype(4) + key/values(0), is longer than 65529", e.getMessage());
+            assertEquals("Length of namespace(65280) + doctype(4) + key/values(0), is longer than 65275", e.getMessage());
         }
     }