All characters except 0x0 byte is allowed during de-serialization of document ids.

This is to ensure that already stored document ids can be de-serialized.

5 files changed, 109 insertions, 10 deletions

diff --git a/document/src/main/java/com/yahoo/document/DocumentId.java b/document/src/main/java/com/yahoo/document/DocumentId.java
index 3ba977fec9e..e7c904cc420 100644
--- a/document/src/main/java/com/yahoo/document/DocumentId.java
+++ b/document/src/main/java/com/yahoo/document/DocumentId.java
@@ -25,10 +25,9 @@ public class DocumentId extends Identifiable implements Serializable {
     }
 
     /**
-     * Constructor. This constructor is used if the DocumentId is used outside of a Document object, but we have the
-     * URI.
+     * Creates a document id based on the given document id URI string.
      *
-     * @param id Associate with this URI, storage address etc. is not applicable.
+     * The document id string can only contain text characters.
      */
     public DocumentId(String id) {
         if (id == null) {
@@ -43,6 +42,17 @@ public class DocumentId extends Identifiable implements Serializable {
         globalId = null;
     }
 
+    /**
+     * Creates a document id based on a serialized document id URI string.
+     *
+     * The document id string is not allowed to contain 0x0 byte characters.
+     * Otherwise all characters are allowed to ensure that document ids
+     * already stored can be de-serialized.
+     */
+    public static DocumentId createFromSerialized(String id) {
+        return new DocumentId(IdString.createFromSerialized(id));
+    }
+
     @Override
     public DocumentId clone() {
         DocumentId docId =  (DocumentId)super.clone();
@@ -95,7 +105,7 @@ public class DocumentId extends Identifiable implements Serializable {
         if (data instanceof DocumentReader) {
             id = ((DocumentReader)data).readDocumentId().getScheme();
         } else {
-            id = IdString.createIdString(data.getString(null));
+            id = IdString.createFromSerialized(data.getString(null));
         }
     }
 
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 27cdf8d603e..c58f2070e31 100644
--- a/document/src/main/java/com/yahoo/document/idstring/IdString.java
+++ b/document/src/main/java/com/yahoo/document/idstring/IdString.java
@@ -77,6 +77,26 @@ public abstract class IdString {
         }
     }
 
+    /**
+     * Creates a IdString based on the given document id string.
+     *
+     * The document id string can only contain text characters.
+     */
+    public static IdString createIdString(String id) {
+        validateTextString(id);
+        return parseAndCreate(id);
+    }
+
+    /**
+     * Creates a IdString based on the given serialized document id string.
+     *
+     * The document id string can not contain 0x0 byte characters.
+     */
+    public static IdString createFromSerialized(String id) {
+        validateNoZeroBytes(id);
+        return parseAndCreate(id);
+    }
+
     private static void validateTextString(String id) {
         OptionalInt illegalCodePoint = Text.validateTextString(id);
         if (illegalCodePoint.isPresent()) {
@@ -85,8 +105,15 @@ public abstract class IdString {
         }
     }
 
-    public static IdString createIdString(String id) {
-        validateTextString(id);
+    private static void validateNoZeroBytes(String id) {
+        for (int i = 0; i < id.length(); i++) {
+            if (id.codePointAt(i) == 0) {
+                throw new IllegalArgumentException("Unparseable id '" + id + "': Contains illegal zero byte code point");
+            }
+        }
+    }
+
+    private static IdString parseAndCreate(String id) {
         String namespace;
         long userId;
         String group;
diff --git a/document/src/main/java/com/yahoo/document/serialization/VespaDocumentDeserializer42.java b/document/src/main/java/com/yahoo/document/serialization/VespaDocumentDeserializer42.java
index ff0e11bcdeb..a048ea349eb 100644
--- a/document/src/main/java/com/yahoo/document/serialization/VespaDocumentDeserializer42.java
+++ b/document/src/main/java/com/yahoo/document/serialization/VespaDocumentDeserializer42.java
@@ -600,7 +600,7 @@ public class VespaDocumentDeserializer42 extends VespaDocumentSerializer42 imple
 
     public DocumentId readDocumentId() {
         Utf8String uri = new Utf8String(parseNullTerminatedString(getBuf().getByteBuffer()));
-        return new DocumentId(uri.toString());
+        return DocumentId.createFromSerialized(uri.toString());
     }
 
     public DocumentType readDocumentType() {
diff --git a/document/src/test/java/com/yahoo/document/DocumentIdTestCase.java b/document/src/test/java/com/yahoo/document/DocumentIdTestCase.java
index da7d40752fd..79a10bc72e4 100644
--- a/document/src/test/java/com/yahoo/document/DocumentIdTestCase.java
+++ b/document/src/test/java/com/yahoo/document/DocumentIdTestCase.java
@@ -3,6 +3,7 @@ package com.yahoo.document;
 
 import com.yahoo.document.*;
 import com.yahoo.document.idstring.*;
+import com.yahoo.vespa.objects.BufferSerializer;
 
 import java.math.BigInteger;
 
@@ -295,13 +296,45 @@ public class DocumentIdTestCase extends junit.framework.TestCase {
     }
 
     public void testDocumentIdCanOnlyContainTextCharacters() throws UnsupportedEncodingException {
-        byte[] rawId = {105, 100, 58, 97, 58, 98, 58, 58, 0, 99}; // "id:a:b::0x0c"
+        assertExceptionWhenConstructing(new byte[]{105, 100, 58, 97, 58, 98, 58, 58, 0, 99}, // "id:a:b::0x0c"
+                "illegal code point 0x0");
+        assertExceptionWhenConstructing(new byte[]{105, 100, 58, 97, 58, 98, 58, 58, 7, 99}, // "id:a:b::0x7c"
+                "illegal code point 0x7");
+    }
+
+    private void assertExceptionWhenConstructing(byte[] rawId,
+                                                 String exceptionMsg) throws UnsupportedEncodingException {
         String strId = new String(rawId, "UTF-8");
         try {
             new DocumentId(strId);
             fail("Expected an IllegalArgumentException to be thrown");
         } catch (IllegalArgumentException ex) {
-            assertThat(ex.getMessage(), containsString("illegal code point 0x0"));
+            assertThat(ex.getMessage(), containsString(exceptionMsg));
+        }
+    }
+
+    public void testSerializedDocumentIdCanContainNonTextCharacter() throws UnsupportedEncodingException {
+        String strId = new String(new byte[]{105, 100, 58, 97, 58, 98, 58, 58, 7, 99}); // "id:a:b::0x7c"
+        DocumentId docId = DocumentId.createFromSerialized(strId);
+        {
+            assertEquals(strId, docId.toString());
+        }
+        {
+            BufferSerializer buf = new BufferSerializer();
+            docId.serialize(buf);
+            buf.flip();
+            DocumentId deserializedId = new DocumentId(buf);
+            assertEquals(strId, deserializedId.toString());
+        }
+    }
+
+    public void testSerializedDocumentIdCannotContainZeroByte() throws UnsupportedEncodingException {
+        String strId = new String(new byte[]{105, 100, 58, 97, 58, 98, 58, 58, 0, 99}); // "id:a:b::0x0c"
+        try {
+            DocumentId.createFromSerialized(strId);
+            fail("Expected an IllegalArgumentException to be thrown");
+        } catch (IllegalArgumentException ex) {
+            assertThat(ex.getMessage(), containsString("illegal zero byte code point"));
         }
     }
 
diff --git a/document/src/test/java/com/yahoo/document/DocumentTestCase.java b/document/src/test/java/com/yahoo/document/DocumentTestCase.java
index 47a4c377da0..44c82042d5c 100644
--- a/document/src/test/java/com/yahoo/document/DocumentTestCase.java
+++ b/document/src/test/java/com/yahoo/document/DocumentTestCase.java
@@ -23,6 +23,7 @@ import org.junit.Test;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.UnsupportedEncodingException;
 import java.nio.ByteBuffer;
 import java.util.Map;
 
@@ -30,7 +31,6 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -1404,4 +1404,33 @@ public class DocumentTestCase extends DocumentTestCaseBase {
         assertEquals(doc2Before, doc2After);
     }
 
+    private static class DocumentIdFixture {
+        private final DocumentTypeManager docMan = new DocumentTypeManager();
+        private final DocumentType docType = new DocumentType("b");
+        private final GrowableByteBuffer buffer = new GrowableByteBuffer();
+        public DocumentIdFixture() {
+            docMan.register(docType);
+        }
+        public void serialize(String docId) {
+            new Document(docType, DocumentId.createFromSerialized(docId))
+                    .serialize(DocumentSerializerFactory.createHead(buffer));
+            buffer.flip();
+        }
+        public Document deserialize() {
+            return new Document(DocumentDeserializerFactory.createHead(docMan, buffer));
+        }
+    }
+
+    @Test
+    public void testDocumentIdWithNonTextCharacterCanBeDeserialized() throws UnsupportedEncodingException {
+        DocumentIdFixture f = new DocumentIdFixture();
+
+        // Document id = "id:a:b::0x7c"
+        String docId = new String(new byte[]{105, 100, 58, 97, 58, 98, 58, 58, 7, 99}, "UTF-8");
+        f.serialize(docId);
+
+        Document result = f.deserialize();
+        assertEquals(docId, result.getId().toString());
+    }
+
 }