diff options
author | Jon Bratseth <bratseth@oath.com> | 2019-01-15 10:25:49 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-01-15 10:25:49 +0100 |
commit | cbe9b4e670834488c5b754d7aa3c8730d753ce4b (patch) | |
tree | f8ede0ad05a855b71c64fab093a1400409b7c576 | |
parent | 650467957862a1cc50b566f8841d8bffc7a60ff0 (diff) | |
parent | 27e1e811df19e5987c9126a3dadbceef9aa0ab58 (diff) |
Merge pull request #8110 from vespa-engine/bratseth/disallow-dash
Bratseth/disallow dash
13 files changed, 90 insertions, 91 deletions
diff --git a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java index 5ec30f71e7b..640e5156615 100644 --- a/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java +++ b/config-application-package/src/main/java/com/yahoo/config/model/application/provider/FilesApplicationPackage.java @@ -619,6 +619,7 @@ public class FilesApplicationPackage implements ApplicationPackage { @Override public Reader getRankingExpression(String name) { try { + File file = expressionFileNameToFile(name); return IOUtils.createReader(expressionFileNameToFile(name), "utf-8"); } catch (IOException e) { diff --git a/config-model/src/main/javacc/SDParser.jj b/config-model/src/main/javacc/SDParser.jj index e50cbabeb9f..66fa308f32f 100644 --- a/config-model/src/main/javacc/SDParser.jj +++ b/config-model/src/main/javacc/SDParser.jj @@ -360,7 +360,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"])+ > @@ -418,10 +419,7 @@ Search rootSearch(String dir) : Search search; } { - ( <SEARCH> name = identifier() { if (!name.matches("[a-zA-Z_][a-zA-Z_:0-9]*")) { - deployLogger.log(Level.WARNING, name + " can not be used in YQL+ expressions."); - } - search = new Search(name, app); + ( <SEARCH> name = identifier() { search = new Search(name, app); rankProfileRegistry.add(new DefaultRankProfile(search, rankProfileRegistry)); rankProfileRegistry.add(new UnrankedRankProfile(search, rankProfileRegistry));} lbrace() (rootSearchItem(search) (<NL>)*)* <RBRACE> (<NL>)* <EOF>) @@ -672,14 +670,15 @@ void fieldSetItem(String setName, Search search) : ( <FIELDS><COLON> field=identifier() { search.fieldSets().addUserFieldSetItem(setName, field); } ( <COMMA> field=identifier() { search.fieldSets().addUserFieldSetItem(setName, field); } )* ) | - ( <QUERYCOMMAND> <COLON> (queryCommand = identifier() | queryCommand = quotedString())) { search.fieldSets().userFieldSets().get(setName).queryCommands().add(queryCommand);} + ( <QUERYCOMMAND> <COLON> (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) : { @@ -701,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) : @@ -1186,7 +1186,7 @@ Object sortingSetting(SortingOperation sorting, String attributeName) : | <QUATERNARY> { sorting.setStrength(Sorting.Strength.QUATERNARY); } | <IDENTICAL> { sorting.setStrength(Sorting.Strength.IDENTICAL); } ) - | <LOCALE> <COLON> locale = identifier() { sorting.setLocale(locale); } + | <LOCALE> <COLON> locale = identifierWithDash() { sorting.setLocale(locale); } ) { return null; } } @@ -1438,7 +1438,7 @@ void summaryProperty(SummaryInFieldLongOperation field) : String name, value; } { - name = identifier() <COLON> (value = identifier() | value = quotedString()) + name = identifierWithDash() <COLON> (value = identifierWithDash() | value = quotedString()) { field.addProperty(new SummaryField.Property(name, value)); } } @@ -1453,7 +1453,7 @@ void fieldStemming(FieldOperationContainer field) : StemmingOperation op = new StemmingOperation(); } { - <STEMMING> <COLON> setting = identifier() + <STEMMING> <COLON> setting = identifierWithDash() { op.setSetting(setting); field.addOperation(op); @@ -1470,7 +1470,7 @@ void searchStemming(Search search) : String setting; } { - <STEMMING> <COLON> setting = identifier() + <STEMMING> <COLON> setting = identifierWithDash() { search.setStemming(Stemming.get(setting)); } } @@ -1485,7 +1485,7 @@ void normalizing(FieldOperationContainer field) : String setting; } { - <NORMALIZING> <COLON> setting = identifier() + <NORMALIZING> <COLON> setting = identifierWithDash() { field.addOperation(new NormalizingOperation(setting)); } @@ -1543,7 +1543,7 @@ void queryCommand(FieldOperationContainer container) : QueryCommandOperation field = new QueryCommandOperation(); } { - <QUERYCOMMAND> <COLON> command = identifier() + <QUERYCOMMAND> <COLON> command = identifierWithDash() { field.addQueryCommand(command); container.addOperation(field); @@ -1556,7 +1556,7 @@ void alias(FieldOperationContainer container) : String alias; } { - <ALIAS> [aliasedName = identifier()] <COLON> alias = identifier() + <ALIAS> [aliasedName = identifier()] <COLON> alias = identifierWithDash() { AliasOperation op = new AliasOperation(aliasedName, alias); container.addOperation(op); @@ -1790,9 +1790,9 @@ Object indexBody(IndexOperation index) : double threshold; } { - ( <PREFIX> { index.setPrefix(true); } - | <ALIAS> <COLON> str = identifier() { index.addAlias(str); } - | <STEMMING> <COLON> str = identifier() { index.setStemming(str); } + ( <PREFIX> { index.setPrefix(true); } + | <ALIAS> <COLON> str = identifierWithDash() { index.addAlias(str); } + | <STEMMING> <COLON> str = identifierWithDash() { index.setStemming(str); } | <RISE> { if (true) throw new ParseException("'index:rise' is no longer an option. Use 'indexing:attribute' instead. " + "If it is a weighted set field you should also add 'attribute:fast-search'." + @@ -1877,7 +1877,7 @@ void rankProfile(Search search) : RankProfile profile; } { - ( <RANKPROFILE> name = identifier() + ( <RANKPROFILE> name = identifierWithDash() { if (documentsOnly) { profile = new DocumentsOnlyRankProfile(name, search, rankProfileRegistry); @@ -1934,7 +1934,7 @@ void inheritsRankProfile(RankProfile profile) : String str; } { - <INHERITS> str = identifier() + <INHERITS> str = identifierWithDash() { profile.setInherited(str); } } @@ -2233,9 +2233,9 @@ String rankPropertyItem() : String image, ret = ""; } { - ( ( image = identifier() { ret += image; } - | image = quotedString() { ret += image; } - | ( "(" | ")" | <DOT> | <COMMA> ) { ret += token.image; } )+ ) + ( ( image = identifierWithDash() { ret += image; } + | image = quotedString() { ret += image; } + | ( "(" | ")" | <DOT> | <COMMA> ) { ret += token.image; } )+ ) { return ret; } } @@ -2476,6 +2476,16 @@ String expression() : { return exp; } } +String identifierWithDash() : +{ + String identifier; +} +{ + ( identifier = identifier() { return identifier; } ) + | + ( <IDENTIFIER_WITH_DASH> { 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( "<jdisc id='default' version='1.0'>", " <search />", @@ -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( "<jdisc id='default' version='1.0'>", " <search>", @@ -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( "<jdisc id='default' version='1.0'>", " <search>", @@ -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( "<jdisc id='default' version='1.0'>", " <search/>", @@ -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( "<jdisc id='default' version='1.0'>", @@ -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<String, NewDocumentType> orderedDocTypes = new TreeMap<>(asDocTypeMapping(Arrays.asList("NULL", "True", "anD"))); diff --git a/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java b/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java index ed80c0bf256..dae6b9f0fb9 100644 --- a/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java +++ b/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java @@ -308,6 +308,17 @@ public class QueryTestCase { } @Test + public void testLanguageSubstitution() { + QueryProfile profile = new QueryProfile("myProfile"); + profile.set("myField1", "Language: %{model.language}", null); + profile.set("myField2", "Locale: %{locale}", null); + Query q = new Query(QueryTestCase.httpEncode("/search?lang=en-us"), profile.compile(null)); + assertEquals("Language: ENGLISH", q.properties().get("myField1")); + q.properties().set("locale", q.getHttpRequest().propertyMap().get("lang")); + assertEquals("Locale: en-us", q.properties().get("myField2")); + } + + @Test public void testTimeoutInRequestOverridesQueryProfile() { QueryProfile profile = new QueryProfile("test"); profile.set("timeout", 318, (QueryProfileRegistry)null); diff --git a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzPrincipalFilter.java b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzPrincipalFilter.java index 5b79b806190..3c154fa4d89 100644 --- a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzPrincipalFilter.java +++ b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzPrincipalFilter.java @@ -69,7 +69,7 @@ public class AthenzPrincipalFilter extends CorsRequestFilterBase { if (!certificatePrincipal.isPresent() && !nTokenPrincipal.isPresent()) { String errorMessage = "Unable to authenticate Athenz identity. " + - "Either client certificate or principal token is required."; + "Either client certificate or principal token is required."; return createResponse(request, Response.Status.UNAUTHORIZED, errorMessage); } if (certificatePrincipal.isPresent() && nTokenPrincipal.isPresent() diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java index da76e288a2a..f7ab399574c 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/filter/DiscFilterRequest.java @@ -67,8 +67,7 @@ public abstract class DiscFilterRequest { public abstract void setUri(URI uri); public HttpRequest getParentRequest() { - throw new UnsupportedOperationException( - "getParentRequest is not supported for " + parent.getClass().getName()); + throw new UnsupportedOperationException("getParentRequest is not supported for " + parent.getClass().getName()); } /** @@ -340,8 +339,8 @@ public abstract class DiscFilterRequest { * is included in the specified logical "role". */ public boolean isUserInRole(String role) { - if(overrideIsUserInRole) { - if(roles != null) { + if (overrideIsUserInRole) { + if (roles != null) { for (String role1 : roles) { if (role1 != null && role1.trim().length() > 0) { String userRole = role1.trim(); @@ -400,7 +399,7 @@ public abstract class DiscFilterRequest { */ public void setCharacterEncoding(String encoding) { String charEncoding = setCharsetFromContentType(this.getContentType(), encoding); - if(charEncoding != null && !charEncoding.isEmpty()) { + if (charEncoding != null && !charEncoding.isEmpty()) { removeHeaders(HttpHeaders.Names.CONTENT_TYPE); setHeaders(HttpHeaders.Names.CONTENT_TYPE, charEncoding); } @@ -410,11 +409,11 @@ public abstract class DiscFilterRequest { * Can be called multiple times to add Cookies */ public void addCookie(JDiscCookieWrapper cookie) { - if(cookie != null) { - List<Cookie> cookies = new ArrayList<Cookie>(); - //Get current set of cookies first + if (cookie != null) { + List<Cookie> cookies = new ArrayList<>(); + // Get current set of cookies first List<Cookie> c = getCookies(); - if(c != null && !c.isEmpty()) { + if (c != null && !c.isEmpty()) { cookies.addAll(c); } cookies.add(cookie.getCookie()); @@ -426,7 +425,7 @@ public abstract class DiscFilterRequest { public JDiscCookieWrapper[] getWrappedCookies() { List<Cookie> cookies = getCookies(); - if(cookies == null) { + if (cookies == null) { return null; } List<JDiscCookieWrapper> cookieWrapper = new ArrayList<>(cookies.size()); @@ -508,12 +507,9 @@ public abstract class DiscFilterRequest { } protected static ThreadLocalSimpleDateFormat formats[] = { - new ThreadLocalSimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", - Locale.US), - new ThreadLocalSimpleDateFormat("EEEEEE, dd-MMM-yy HH:mm:ss zzz", - Locale.US), - new ThreadLocalSimpleDateFormat("EEE MMMM d HH:mm:ss yyyy", - Locale.US) }; + new ThreadLocalSimpleDateFormat("EEE, dd MMM yyyy HH:mm:ss zzz", Locale.US), + new ThreadLocalSimpleDateFormat("EEEEEE, dd-MMM-yy HH:mm:ss zzz", Locale.US), + new ThreadLocalSimpleDateFormat("EEE MMMM d HH:mm:ss yyyy", Locale.US) }; /** * The set of SimpleDateFormat formats to use in getDateHeader(). @@ -521,8 +517,8 @@ public abstract class DiscFilterRequest { * Notice that because SimpleDateFormat is not thread-safe, we can't declare * formats[] as a static variable. */ - protected static final class ThreadLocalSimpleDateFormat extends - ThreadLocal<SimpleDateFormat> { + protected static final class ThreadLocalSimpleDateFormat extends ThreadLocal<SimpleDateFormat> { + private final String format; private final Locale locale; @@ -541,6 +537,7 @@ public abstract class DiscFilterRequest { public Date parse(String value) throws ParseException { return get().parse(value); } + } } diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/servlet/ServletRequest.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/servlet/ServletRequest.java index e6bb99d4647..2eb7f432ec2 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/servlet/ServletRequest.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/servlet/ServletRequest.java @@ -33,10 +33,9 @@ import static com.yahoo.jdisc.http.core.HttpServletRequestUtils.getConnection; * You might find it tempting to remove e.g. the getParameter... methods, * but keep in mind that this IS-A servlet request and must provide the * full api of such a request for use outside the "JDisc filter world". - * - * @since 5.27 */ public class ServletRequest extends HttpServletRequestWrapper implements ServletOrJdiscHttpRequest { + public static final String JDISC_REQUEST_PRINCIPAL = "jdisc.request.principal"; public static final String JDISC_REQUEST_X509CERT = "jdisc.request.X509Certificate"; public static final String SERVLET_REQUEST_X509CERT = "javax.servlet.request.X509Certificate"; diff --git a/security-utils/src/main/java/com/yahoo/security/KeyUtils.java b/security-utils/src/main/java/com/yahoo/security/KeyUtils.java index 0d45a62f193..491178f8801 100644 --- a/security-utils/src/main/java/com/yahoo/security/KeyUtils.java +++ b/security-utils/src/main/java/com/yahoo/security/KeyUtils.java @@ -38,6 +38,7 @@ import static com.yahoo.security.KeyAlgorithm.RSA; * @author bjorncs */ public class KeyUtils { + private KeyUtils() {} public static KeyPair generateKeypair(KeyAlgorithm algorithm, int keySize) { diff --git a/security-utils/src/main/java/com/yahoo/security/X509CertificateUtils.java b/security-utils/src/main/java/com/yahoo/security/X509CertificateUtils.java index 33bd750bac5..fc7a6780b23 100644 --- a/security-utils/src/main/java/com/yahoo/security/X509CertificateUtils.java +++ b/security-utils/src/main/java/com/yahoo/security/X509CertificateUtils.java @@ -133,4 +133,5 @@ public class X509CertificateUtils { throw new UncheckedIOException(e); } } + } |