diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2021-01-11 18:34:22 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2021-01-11 18:34:22 +0000 |
commit | 55b8a24439e9dc41122a98bd7537968983e8a9ec (patch) | |
tree | 3d5339fc449acc2e791bd1722a98939185ed266f /document/src | |
parent | 9043ed4ac5f1c8a4af91b1e2b2ff602bc1963c27 (diff) |
Apply a 64k limit to documentids.
Diffstat (limited to 'document/src')
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()); } } |