From 23835c0ef5dfe290b3d08aa69a820bf268f5f7a0 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Fri, 11 Jan 2019 14:54:17 +0100 Subject: Disallow dash in search definition, document, field and function names --- config-model/src/main/javacc/SDParser.jj | 49 ++++++++++++++-------- config-model/src/test/examples/invalid-name.sd | 12 ++++++ .../src/test/examples/simple-with-weird-name.sd | 13 ------ .../SearchDefinitionsParsingTestCase.java | 35 ++++------------ .../model/container/xml/SearchBuilderTest.java | 13 +++--- .../ReservedDocumentTypeNameValidatorTest.java | 2 +- 6 files changed, 58 insertions(+), 66 deletions(-) create mode 100644 config-model/src/test/examples/invalid-name.sd delete mode 100644 config-model/src/test/examples/simple-with-weird-name.sd (limited to 'config-model') diff --git a/config-model/src/main/javacc/SDParser.jj b/config-model/src/main/javacc/SDParser.jj index 883dfc7ac72..60af7d7ab83 100644 --- a/config-model/src/main/javacc/SDParser.jj +++ b/config-model/src/main/javacc/SDParser.jj @@ -357,7 +357,8 @@ TOKEN : | < CONSTANTS: "constants" > | < FILE: "file" > | < URI: "uri" > -| < IDENTIFIER: ["a"-"z","A"-"Z", "_"] (["a"-"z","A"-"Z","0"-"9","_","-"])* > +| < IDENTIFIER: ["a"-"z","A"-"Z", "_"] (["a"-"z","A"-"Z","0"-"9","_"])* > +| < IDENTIFIER_WITH_DASH: ["a"-"z","A"-"Z", "_"] (["a"-"z","A"-"Z","0"-"9","_","-"])* > | < QUOTEDSTRING: "\"" ( ~["\""] )* "\"" > | < CONTEXT: ["a"-"z","A"-"Z"] (["a"-"z", "A"-"Z", "0"-"9"])* > | < DOUBLE: ("-")? (["0"-"9"])+ "." (["0"-"9"])+ > @@ -669,14 +670,15 @@ void fieldSetItem(String setName, Search search) : ( field=identifier() { search.fieldSets().addUserFieldSetItem(setName, field); } ( field=identifier() { search.fieldSets().addUserFieldSetItem(setName, field); } )* ) | - ( (queryCommand = identifier() | queryCommand = quotedString())) { search.fieldSets().userFieldSets().get(setName).queryCommands().add(queryCommand);} + ( (queryCommand = identifierWithDash() | queryCommand = quotedString())) { search.fieldSets().userFieldSets().get(setName).queryCommands().add(queryCommand);} | ( match(matchSettings) ) { matchSettings.applyOperations(); search.fieldSets().userFieldSets().get(setName).setMatching(matchSettings.getMatching());} } /** * This rule consumes a annotation block from within either a document element or a search element. - * @param search The search object to add content to. + + * @param search the search object to add content to. */ void annotationOutside(Search search) : { @@ -698,6 +700,7 @@ void annotationOutside(Search search) : /** * This rule consumes a annotation block from within either a document element. + * * @param document The document object to add content to. */ void annotation(Search search, SDDocumentType document) : @@ -1183,7 +1186,7 @@ Object sortingSetting(SortingOperation sorting, String attributeName) : | { sorting.setStrength(Sorting.Strength.QUATERNARY); } | { sorting.setStrength(Sorting.Strength.IDENTICAL); } ) - | locale = identifier() { sorting.setLocale(locale); } + | locale = identifierWithDash() { sorting.setLocale(locale); } ) { return null; } } @@ -1426,7 +1429,7 @@ void summaryProperty(SummaryInFieldLongOperation field) : String name, value; } { - name = identifier() (value = identifier() | value = quotedString()) + name = identifierWithDash() (value = identifierWithDash() | value = quotedString()) { field.addProperty(new SummaryField.Property(name, value)); } } @@ -1441,7 +1444,7 @@ void fieldStemming(FieldOperationContainer field) : StemmingOperation op = new StemmingOperation(); } { - setting = identifier() + setting = identifierWithDash() { op.setSetting(setting); field.addOperation(op); @@ -1458,7 +1461,7 @@ void searchStemming(Search search) : String setting; } { - setting = identifier() + setting = identifierWithDash() { search.setStemming(Stemming.get(setting)); } } @@ -1473,7 +1476,7 @@ void normalizing(FieldOperationContainer field) : String setting; } { - setting = identifier() + setting = identifierWithDash() { field.addOperation(new NormalizingOperation(setting)); } @@ -1527,7 +1530,7 @@ void queryCommand(FieldOperationContainer container) : QueryCommandOperation field = new QueryCommandOperation(); } { - command = identifier() + command = identifierWithDash() { field.addQueryCommand(command); container.addOperation(field); @@ -1540,7 +1543,7 @@ void alias(FieldOperationContainer container) : String alias; } { - [aliasedName = identifier()] alias = identifier() + [aliasedName = identifier()] alias = identifierWithDash() { AliasOperation op = new AliasOperation(aliasedName, alias); container.addOperation(op); @@ -1773,9 +1776,9 @@ Object indexBody(IndexOperation index) : double threshold; } { - ( { index.setPrefix(true); } - | str = identifier() { index.addAlias(str); } - | str = identifier() { index.setStemming(str); } + ( { index.setPrefix(true); } + | str = identifierWithDash() { index.addAlias(str); } + | str = identifierWithDash() { index.setStemming(str); } | arity = integer() { index.setArity(arity); } | num = consumeLong() { index.setLowerBound(num); } | num = consumeLong() { index.setUpperBound(num); } @@ -1855,7 +1858,7 @@ void rankProfile(Search search) : RankProfile profile; } { - ( name = identifier() + ( name = identifierWithDash() { if (documentsOnly) { profile = new DocumentsOnlyRankProfile(name, search, rankProfileRegistry); @@ -1912,7 +1915,7 @@ void inheritsRankProfile(RankProfile profile) : String str; } { - str = identifier() + str = identifierWithDash() { profile.setInherited(str); } } @@ -2211,9 +2214,9 @@ String rankPropertyItem() : String image, ret = ""; } { - ( ( image = identifier() { ret += image; } - | image = quotedString() { ret += image; } - | ( "(" | ")" | | ) { ret += token.image; } )+ ) + ( ( image = identifierWithDash() { ret += image; } + | image = quotedString() { ret += image; } + | ( "(" | ")" | | ) { ret += token.image; } )+ ) { return ret; } } @@ -2454,6 +2457,16 @@ String expression() : { return exp; } } +String identifierWithDash() : +{ + String identifier; +} +{ + ( identifier = identifier() { return identifier; } ) + | + ( { return token.image; } ) +} + /** * This rule consumes an identifier. This must be kept in sync with all word tokens that should be parseable as * identifiers. diff --git a/config-model/src/test/examples/invalid-name.sd b/config-model/src/test/examples/invalid-name.sd new file mode 100644 index 00000000000..f26fcc723f4 --- /dev/null +++ b/config-model/src/test/examples/invalid-name.sd @@ -0,0 +1,12 @@ +# Dashes in names are not allowed +search invalid-name { + + document invalid-name { + + field title type string { + + } + + } + +} \ No newline at end of file diff --git a/config-model/src/test/examples/simple-with-weird-name.sd b/config-model/src/test/examples/simple-with-weird-name.sd deleted file mode 100644 index 109f4f7bba7..00000000000 --- a/config-model/src/test/examples/simple-with-weird-name.sd +++ /dev/null @@ -1,13 +0,0 @@ -# Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -# A minimal doc with name which is incompatible with YQL+ -search simple-with-weird-name { - - document simple-with-weird-name { - - field title type string { - indexing: summary | index - } - - } - -} diff --git a/config-model/src/test/java/com/yahoo/searchdefinition/SearchDefinitionsParsingTestCase.java b/config-model/src/test/java/com/yahoo/searchdefinition/SearchDefinitionsParsingTestCase.java index 278471cb37a..a26154fc8da 100644 --- a/config-model/src/test/java/com/yahoo/searchdefinition/SearchDefinitionsParsingTestCase.java +++ b/config-model/src/test/java/com/yahoo/searchdefinition/SearchDefinitionsParsingTestCase.java @@ -73,35 +73,16 @@ public class SearchDefinitionsParsingTestCase extends SearchDefinitionTestCase { } } - private static class WarningCatcher extends Handler { - volatile boolean gotYqlWarning = false; - - @Override - public void publish(LogRecord record) { - if (record.getLevel() == Level.WARNING && record.getMessage().indexOf("YQL") >= 0) { - gotYqlWarning = true; + @Test + public void illegalSearchDefinitionName() throws IOException, ParseException { + try { + SearchBuilder.buildFromFile("src/test/examples/invalid-name.sd"); + fail("Name with dash passed"); + } catch (ParseException e) { + if ( ! e.getMessage().contains("invalid-name")) { + throw e; } } - - @Override - public void flush() { - // intentionally left blank - } - - @Override - public void close() throws SecurityException { - // intentionally left blank - } } - - @Test - public void requireYqlCompatibilityIsTested() throws Exception { - Logger log = Logger.getLogger("DeployLogger"); - WarningCatcher w = new WarningCatcher(); - log.addHandler(w); - assertNotNull(SearchBuilder.buildFromFile("src/test/examples/simple-with-weird-name.sd")); - log.removeHandler(w); - assertTrue(w.gotYqlWarning); - } } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/SearchBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/SearchBuilderTest.java index 30f1df6a394..4180f9f6de4 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/SearchBuilderTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/SearchBuilderTest.java @@ -26,7 +26,6 @@ import static org.junit.Assert.*; /** * @author gjoranv - * @since 5.1.10 */ public class SearchBuilderTest extends ContainerModelBuilderTestBase { @@ -35,7 +34,7 @@ public class SearchBuilderTest extends ContainerModelBuilderTestBase { } @Test - public void gui_search_handler_is_always_included_when_search_is_specified() throws Exception{ + public void gui_search_handler_is_always_included_when_search_is_specified() { Element clusterElem = DomBuilderTest.parse( "", " ", @@ -61,7 +60,7 @@ public class SearchBuilderTest extends ContainerModelBuilderTestBase { @Test - public void search_handler_bindings_can_be_overridden() throws Exception { + public void search_handler_bindings_can_be_overridden() { Element clusterElem = DomBuilderTest.parse( "", " ", @@ -80,7 +79,7 @@ public class SearchBuilderTest extends ContainerModelBuilderTestBase { } @Test - public void search_handler_bindings_can_be_disabled() throws Exception { + public void search_handler_bindings_can_be_disabled() { Element clusterElem = DomBuilderTest.parse( "", " ", @@ -111,7 +110,7 @@ public class SearchBuilderTest extends ContainerModelBuilderTestBase { assertThat(chainsConfig().chains(), hasItemWithMethod("vespa", "id")); } - private void createClusterWithOnlyDefaultChains() throws SAXException, IOException { + private void createClusterWithOnlyDefaultChains() { Element containerElem = DomBuilderTest.parse( "", " ", @@ -124,7 +123,7 @@ public class SearchBuilderTest extends ContainerModelBuilderTestBase { } @Test - public void manually_setting_up_search_handler_is_forbidden() throws IOException, SAXException { + public void manually_setting_up_search_handler_is_forbidden() { try { Element clusterElem = DomBuilderTest.parse( "", @@ -193,7 +192,7 @@ public class SearchBuilderTest extends ContainerModelBuilderTestBase { } - private VespaModel getVespaModelWithMusic(String hosts, String services) throws ParseException { + private VespaModel getVespaModelWithMusic(String hosts, String services) { return new VespaModelCreatorWithMockPkg(hosts, services, ApplicationPackageUtils.generateSearchDefinitions("music")).create(); } diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/ReservedDocumentTypeNameValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/ReservedDocumentTypeNameValidatorTest.java index 0ad5fb3b0bd..66f59717407 100644 --- a/config-model/src/test/java/com/yahoo/vespa/model/content/ReservedDocumentTypeNameValidatorTest.java +++ b/config-model/src/test/java/com/yahoo/vespa/model/content/ReservedDocumentTypeNameValidatorTest.java @@ -47,7 +47,7 @@ public class ReservedDocumentTypeNameValidatorTest { public void validation_is_case_insensitive() { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("The following document types conflict with reserved keyword names: " + - "'NULL', 'True', 'anD'."); + "'NULL', 'True', 'anD'."); ReservedDocumentTypeNameValidator validator = new ReservedDocumentTypeNameValidator(); Map orderedDocTypes = new TreeMap<>(asDocTypeMapping(Arrays.asList("NULL", "True", "anD"))); -- cgit v1.2.3