diff options
15 files changed, 143 insertions, 34 deletions
diff --git a/cps-path-parser/src/main/antlr4/org/onap/cps/cpspath/parser/antlr4/CpsPath.g4 b/cps-path-parser/src/main/antlr4/org/onap/cps/cpspath/parser/antlr4/CpsPath.g4 index c88a822654..3aef120fed 100644 --- a/cps-path-parser/src/main/antlr4/org/onap/cps/cpspath/parser/antlr4/CpsPath.g4 +++ b/cps-path-parser/src/main/antlr4/org/onap/cps/cpspath/parser/antlr4/CpsPath.g4 @@ -1,6 +1,6 @@ /* * ============LICENSE_START======================================================= - * Copyright (C) 2021-2022 Nordix Foundation + * Copyright (C) 2021-2023 Nordix Foundation * Modifications Copyright (C) 2023 TechMahindra Ltd * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); @@ -19,6 +19,12 @@ * ============LICENSE_END========================================================= */ +/* + * Parser Rules + * Some of the parser rules below are inspired by + * https://github.com/antlr/grammars-v4/blob/master/xpath/xpath31/XPath31Parser.g4 + */ + grammar CpsPath ; cpsPath : ( prefix | descendant | incorrectPrefix ) multipleLeafConditions? textFunctionCondition? containsFunctionCondition? ancestorAxis? invalidPostFix?; @@ -60,7 +66,7 @@ invalidPostFix : (AT | CB | COLONCOLON | comparativeOperators ).+ ; /* * Lexer Rules * Most of the lexer rules below are inspired by - * https://raw.githubusercontent.com/antlr/grammars-v4/master/xpath/xpath31/XPath31.g4 + * https://github.com/antlr/grammars-v4/blob/master/xpath/xpath31/XPath31Lexer.g4 */ AT : '@' ; @@ -89,9 +95,9 @@ IntegerLiteral : FragDigits ; // Add below type definitions for leafvalue comparision in https://jira.onap.org/browse/CPS-440 DecimalLiteral : ('.' FragDigits) | (FragDigits '.' [0-9]*) ; DoubleLiteral : (('.' FragDigits) | (FragDigits ('.' [0-9]*)?)) [eE] [+-]? FragDigits ; -StringLiteral : ('"' (FragEscapeQuot | ~[^"])*? '"') | ('\'' (FragEscapeApos | ~['])*? '\'') ; +StringLiteral : '"' (~["] | FragEscapeQuot)* '"' | '\'' (~['] | FragEscapeApos)* '\'' ; fragment FragEscapeQuot : '""' ; -fragment FragEscapeApos : '\'' ; +fragment FragEscapeApos : '\'\''; fragment FragDigits : [0-9]+ ; QName : FragQName ; diff --git a/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathBuilder.java b/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathBuilder.java index 99135962f8..de261e64b3 100644 --- a/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathBuilder.java +++ b/cps-path-parser/src/main/java/org/onap/cps/cpspath/parser/CpsPathBuilder.java @@ -79,18 +79,13 @@ public class CpsPathBuilder extends CpsPathBaseListener { @Override public void exitLeafCondition(final LeafConditionContext ctx) { - Object comparisonValue; + final Object comparisonValue; if (ctx.IntegerLiteral() != null) { comparisonValue = Integer.valueOf(ctx.IntegerLiteral().getText()); } else if (ctx.StringLiteral() != null) { - final boolean wasWrappedInDoubleQuote = ctx.StringLiteral().getText().startsWith("\""); - comparisonValue = stripFirstAndLastCharacter(ctx.StringLiteral().getText()); - if (wasWrappedInDoubleQuote) { - comparisonValue = String.valueOf(comparisonValue).replace("'", "\\'"); - } + comparisonValue = unwrapQuotedString(ctx.StringLiteral().getText()); } else { - throw new PathParsingException( - "Unsupported comparison value encountered in expression" + ctx.getText()); + throw new PathParsingException("Unsupported comparison value encountered in expression" + ctx.getText()); } leafContext(ctx.leafName(), comparisonValue); } @@ -140,13 +135,13 @@ public class CpsPathBuilder extends CpsPathBaseListener { @Override public void exitTextFunctionCondition(final TextFunctionConditionContext ctx) { cpsPathQuery.setTextFunctionConditionLeafName(ctx.leafName().getText()); - cpsPathQuery.setTextFunctionConditionValue(stripFirstAndLastCharacter(ctx.StringLiteral().getText())); + cpsPathQuery.setTextFunctionConditionValue(unwrapQuotedString(ctx.StringLiteral().getText())); } @Override public void exitContainsFunctionCondition(final CpsPathParser.ContainsFunctionConditionContext ctx) { cpsPathQuery.setContainsFunctionConditionLeafName(ctx.leafName().getText()); - cpsPathQuery.setContainsFunctionConditionValue(stripFirstAndLastCharacter(ctx.StringLiteral().getText())); + cpsPathQuery.setContainsFunctionConditionValue(unwrapQuotedString(ctx.StringLiteral().getText())); } @Override @@ -173,10 +168,6 @@ public class CpsPathBuilder extends CpsPathBaseListener { return cpsPathQuery; } - private static String stripFirstAndLastCharacter(final String wrappedString) { - return wrappedString.substring(1, wrappedString.length() - 1); - } - @Override public void exitContainerName(final CpsPathParser.ContainerNameContext ctx) { final String containerName = ctx.getText(); @@ -207,11 +198,25 @@ public class CpsPathBuilder extends CpsPathBaseListener { .append(name) .append(getLastElement(comparativeOperators)) .append("'") - .append(value) + .append(value.toString().replace("'", "''")) .append("'"); } - private String getLastElement(final List<String> listOfStrings) { + private static String getLastElement(final List<String> listOfStrings) { return listOfStrings.get(listOfStrings.size() - 1); } + + private static String unwrapQuotedString(final String wrappedString) { + final boolean wasWrappedInSingleQuote = wrappedString.startsWith("'"); + final String value = stripFirstAndLastCharacter(wrappedString); + if (wasWrappedInSingleQuote) { + return value.replace("''", "'"); + } else { + return value.replace("\"\"", "\""); + } + } + + private static String stripFirstAndLastCharacter(final String wrappedString) { + return wrappedString.substring(1, wrappedString.length() - 1); + } } diff --git a/cps-path-parser/src/test/groovy/org/onap/cps/cpspath/parser/CpsPathQuerySpec.groovy b/cps-path-parser/src/test/groovy/org/onap/cps/cpspath/parser/CpsPathQuerySpec.groovy index 78963033da..0017242abe 100644 --- a/cps-path-parser/src/test/groovy/org/onap/cps/cpspath/parser/CpsPathQuerySpec.groovy +++ b/cps-path-parser/src/test/groovy/org/onap/cps/cpspath/parser/CpsPathQuerySpec.groovy @@ -46,6 +46,10 @@ class CpsPathQuerySpec extends Specification { 'spaces around =' | '/parent/child[@common-leaf-name-int = 5]' || '/parent/child' | 'common-leaf-name-int' | 5 'key in top container' | '/parent[@common-leaf-name-int=5]' || '/parent' | 'common-leaf-name-int' | 5 'parent list' | '/shops/shop[@id=1]/categories[@id=1]/book[@title="Dune"]' || "/shops/shop[@id='1']/categories[@id='1']/book" | 'title' | 'Dune' + "' in double quote" | '/parent[@common-leaf-name="leaf\'value"]' || '/parent' | 'common-leaf-name' | "leaf'value" + "' in single quote" | "/parent[@common-leaf-name='leaf''value']" || '/parent' | 'common-leaf-name' | "leaf'value" + '" in double quote' | '/parent[@common-leaf-name="leaf""value"]' || '/parent' | 'common-leaf-name' | 'leaf"value' + '" in single quote' | '/parent[@common-leaf-name=\'leaf"value\']' || '/parent' | 'common-leaf-name' | 'leaf"value' } def 'Parse cps path of type ends with a #scenario.'() { @@ -80,8 +84,8 @@ class CpsPathQuerySpec extends Specification { 'parent leaf of type Integer & child' | '/parent/child[@code=1]/child2' || "/parent/child[@code='1']/child2" 'parent leaf with double quotes' | '/parent/child[@code="1"]/child2' || "/parent/child[@code='1']/child2" 'parent leaf with double quotes inside single quotes' | '/parent/child[@code=\'"1"\']/child2' || "/parent/child[@code='\"1\"']/child2" - 'parent leaf with single quotes inside double quotes' | '/parent/child[@code="\'1\'"]/child2' || "/parent/child[@code='\\\'1\\\'']/child2" - 'leaf with single quotes inside double quotes' | '/parent/child[@code="\'1\'"]' || "/parent/child[@code='\\\'1\\\'']" + 'parent leaf with single quotes inside double quotes' | '/parent/child[@code="\'1\'"]/child2' || "/parent/child[@code='''1''']/child2" + 'leaf with single quotes inside double quotes' | '/parent/child[@code="\'1\'"]' || "/parent/child[@code='''1''']" 'leaf with more than one attribute' | '/parent/child[@key1=1 and @key2="abc"]' || "/parent/child[@key1='1' and @key2='abc']" 'parent & child with more than one attribute' | '/parent/child[@key1=1 and @key2="abc"]/child2' || "/parent/child[@key1='1' and @key2='abc']/child2" 'leaf with more than one attribute has OR operator' | '/parent/child[@key1=1 or @key2="abc"]' || "/parent/child[@key1='1' or @key2='abc']" diff --git a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentQueryBuilder.java b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentQueryBuilder.java index be06ebac03..e371035ba5 100644 --- a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentQueryBuilder.java +++ b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentQueryBuilder.java @@ -136,7 +136,7 @@ public class FragmentQueryBuilder { final String leafValueAsText = leaf.getValue().toString(); sqlStringBuilder.append("attributes ->> '").append(leaf.getName()).append("'"); sqlStringBuilder.append(" = '"); - sqlStringBuilder.append(leafValueAsText); + sqlStringBuilder.append(EscapeUtils.escapeForSqlStringLiteral(leafValueAsText)); sqlStringBuilder.append("'"); } else { throw new CpsPathException(" can use only " + nextComparativeOperator + " with integer "); diff --git a/cps-ri/src/main/java/org/onap/cps/spi/repository/TempTableCreator.java b/cps-ri/src/main/java/org/onap/cps/spi/repository/TempTableCreator.java index 139a8b3063..4c7971ead8 100644 --- a/cps-ri/src/main/java/org/onap/cps/spi/repository/TempTableCreator.java +++ b/cps-ri/src/main/java/org/onap/cps/spi/repository/TempTableCreator.java @@ -31,6 +31,7 @@ import javax.persistence.EntityManager; import javax.persistence.PersistenceContext; import lombok.AllArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.onap.cps.spi.utils.EscapeUtils; import org.springframework.stereotype.Component; import org.springframework.transaction.annotation.Transactional; @@ -86,7 +87,7 @@ public class TempTableCreator { final Collection<String> sqlInserts = new HashSet<>(sqlData.size()); for (final Collection<String> rowValues : sqlData) { final Collection<String> escapedValues = - rowValues.stream().map(it -> escapeSingleQuotesByDoublingThem(it)).collect(Collectors.toList()); + rowValues.stream().map(EscapeUtils::escapeForSqlStringLiteral).collect(Collectors.toList()); sqlInserts.add("('" + String.join("','", escapedValues) + "')"); } sqlStringBuilder.append("INSERT INTO "); @@ -98,8 +99,4 @@ public class TempTableCreator { sqlStringBuilder.append(";"); } - private static String escapeSingleQuotesByDoublingThem(final String value) { - return value.replace("'", "''"); - } - } diff --git a/cps-ri/src/main/java/org/onap/cps/spi/utils/EscapeUtils.java b/cps-ri/src/main/java/org/onap/cps/spi/utils/EscapeUtils.java index 3092b79051..2b61d39503 100644 --- a/cps-ri/src/main/java/org/onap/cps/spi/utils/EscapeUtils.java +++ b/cps-ri/src/main/java/org/onap/cps/spi/utils/EscapeUtils.java @@ -26,8 +26,12 @@ import lombok.NoArgsConstructor; @NoArgsConstructor(access = AccessLevel.PRIVATE) public class EscapeUtils { - public static String escapeForSqlLike(final String text) { - return text.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_"); + public static String escapeForSqlLike(final String value) { + return value.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_"); + } + + public static String escapeForSqlStringLiteral(final String value) { + return value.replace("'", "''"); } } diff --git a/cps-ri/src/main/resources/changelog/changelog-master.yaml b/cps-ri/src/main/resources/changelog/changelog-master.yaml index 4e6986e71f..f76c5ba3b9 100644 --- a/cps-ri/src/main/resources/changelog/changelog-master.yaml +++ b/cps-ri/src/main/resources/changelog/changelog-master.yaml @@ -56,3 +56,5 @@ databaseChangeLog: file: changelog/db/changes/19-delete-not-required-dataspace-id-from-fragment.yaml - include: file: changelog/db/changes/20-change-foreign-key-id-types-to-integer.yaml + - include: + file: changelog/db/changes/21-escape-quotes-in-xpath.yaml diff --git a/cps-ri/src/main/resources/changelog/db/changes/21-escape-quotes-in-xpath-forward.sql b/cps-ri/src/main/resources/changelog/db/changes/21-escape-quotes-in-xpath-forward.sql new file mode 100644 index 0000000000..9bf7f9a74e --- /dev/null +++ b/cps-ri/src/main/resources/changelog/db/changes/21-escape-quotes-in-xpath-forward.sql @@ -0,0 +1,19 @@ +/* + ============LICENSE_START======================================================= + Copyright (C) 2023 Nordix Foundation. + ================================================================================ + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + SPDX-License-Identifier: Apache-2.0 + ============LICENSE_END========================================================= +*/ + +-- replace \' with '' and "" with " +UPDATE fragment SET xpath = replace(replace(xpath, $$\'$$, $$''$$), '""', '"');
\ No newline at end of file diff --git a/cps-ri/src/main/resources/changelog/db/changes/21-escape-quotes-in-xpath-rollback.sql b/cps-ri/src/main/resources/changelog/db/changes/21-escape-quotes-in-xpath-rollback.sql new file mode 100644 index 0000000000..0fd1633a54 --- /dev/null +++ b/cps-ri/src/main/resources/changelog/db/changes/21-escape-quotes-in-xpath-rollback.sql @@ -0,0 +1,19 @@ +/* + ============LICENSE_START======================================================= + Copyright (C) 2023 Nordix Foundation. + ================================================================================ + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + SPDX-License-Identifier: Apache-2.0 + ============LICENSE_END========================================================= +*/ + +-- replace '' with \' and " with "" +UPDATE fragment SET xpath = replace(replace(xpath, $$''$$, $$\'$$), '"', '""');
\ No newline at end of file diff --git a/cps-ri/src/main/resources/changelog/db/changes/21-escape-quotes-in-xpath.yaml b/cps-ri/src/main/resources/changelog/db/changes/21-escape-quotes-in-xpath.yaml new file mode 100644 index 0000000000..7b5b1dbd07 --- /dev/null +++ b/cps-ri/src/main/resources/changelog/db/changes/21-escape-quotes-in-xpath.yaml @@ -0,0 +1,29 @@ +# ============LICENSE_START======================================================= +# Copyright (C) 2023 Nordix Foundation. +# ================================================================================ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# SPDX-License-Identifier: Apache-2.0 +# ============LICENSE_END========================================================= + +databaseChangeLog: + + - changeSet: + id: 21 + author: cps + changes: + - sqlFile: + path: changelog/db/changes/21-escape-quotes-in-xpath-forward.sql + rollback: + - sqlFile: + path: changelog/db/changes/21-escape-quotes-in-xpath-rollback.sql diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/utils/EscapeUtilsSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/utils/EscapeUtilsSpec.groovy index 7de9b97ba0..52330e6251 100644 --- a/cps-ri/src/test/groovy/org/onap/cps/spi/utils/EscapeUtilsSpec.groovy +++ b/cps-ri/src/test/groovy/org/onap/cps/spi/utils/EscapeUtilsSpec.groovy @@ -24,7 +24,7 @@ import spock.lang.Specification class EscapeUtilsSpec extends Specification { - def 'Escape text for using in SQL LIKE operation'() { + def 'Escape text for use in SQL LIKE operation.'() { expect: 'SQL LIKE special characters to be escaped with forward-slash' assert EscapeUtils.escapeForSqlLike(unescapedText) == escapedText where: @@ -33,4 +33,9 @@ class EscapeUtilsSpec extends Specification { 'Others (./?$) are not special' || 'Others (./?$) are not special' } + def 'Escape text for use in SQL string literal.'() { + expect: 'single quotes to be doubled' + assert EscapeUtils.escapeForSqlStringLiteral("I'm escaping!") == "I''m escaping!" + } + } diff --git a/cps-service/src/main/java/org/onap/cps/utils/YangUtils.java b/cps-service/src/main/java/org/onap/cps/utils/YangUtils.java index 7da4024156..f00f9442ce 100644 --- a/cps-service/src/main/java/org/onap/cps/utils/YangUtils.java +++ b/cps-service/src/main/java/org/onap/cps/utils/YangUtils.java @@ -253,7 +253,7 @@ public class YangUtils { final List<String> keyAttributes = nodeIdentifier.entrySet().stream().map( entry -> { final String name = entry.getKey().getLocalName(); - final String value = String.valueOf(entry.getValue()).replace("'", "\\'"); + final String value = String.valueOf(entry.getValue()).replace("'", "''"); return String.format("@%s='%s'", name, value); } ).collect(Collectors.toList()); diff --git a/docs/cps-path.rst b/docs/cps-path.rst index 796eb7f429..6611789544 100644 --- a/docs/cps-path.rst +++ b/docs/cps-path.rst @@ -177,6 +177,7 @@ General Notes ============= - String values must be wrapped in quotation marks ``"`` (U+0022) or apostrophes ``'`` (U+0027). +- Quotations marks and apostrophes can be escaped by doubling them in their respective quotes, for example ``'CPS ''Path'' Query' -> CPS 'Path' Query`` - String comparisons are case sensitive. Query Syntax @@ -247,7 +248,6 @@ leaf-conditions - The key should be supplied with correct data type for it to be queried from DB. In the last example above the attribute code is of type Integer so the cps query will not work if the value is passed as string. eg: ``//categories[@code="1"]`` or ``//categories[@code='1']`` will not work because the key attribute code is treated a string. - - Having '[' token in any index in any list will have a negative impact on this function. **Notes** - For performance reasons it does not make sense to query using key leaf as attribute. If the key value is known it is better to execute a get request with the complete xpath. @@ -272,7 +272,6 @@ The text()-condition can be added to any CPS path query. - Only string and integer values are supported, boolean and float values are not supported. - Since CPS cannot return individual leaves it will always return the container with all its leaves. Ancestor-axis can be used to specify a parent higher up the tree. - When querying a leaf value (instead of leaf-list) it is better, more performant to use a text value condition use @<leaf-name> as described above. - - Having '[' token in any index in any list will have a negative impact on this function. contains()-condition -------------------- diff --git a/docs/release-notes.rst b/docs/release-notes.rst index f56b34a9f5..25f6d22ac0 100755 --- a/docs/release-notes.rst +++ b/docs/release-notes.rst @@ -42,6 +42,7 @@ Bug Fixes Features -------- + - `CPS-1760 <https://jira.onap.org/browse/CPS-1760>`_ Improve handling of special characters in Cps Paths Version: 3.3.4 ============== diff --git a/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsQueryServiceIntegrationSpec.groovy b/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsQueryServiceIntegrationSpec.groovy index 53737fba80..74496d3016 100644 --- a/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsQueryServiceIntegrationSpec.groovy +++ b/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsQueryServiceIntegrationSpec.groovy @@ -356,4 +356,23 @@ class CpsQueryServiceIntegrationSpec extends FunctionalSpecBase { 'text-condition' || "/bookstore/categories[@code='1']/books/title[text()='[@hello=world]']" 'contains-condition' || "/bookstore/categories[@code='1']/books[contains(@title, '[@hello=world]')]" } + + def 'Cps Path get and query can handle apostrophe inside #quotes.'() { + given: 'a book with special characters in title' + cpsDataService.saveData(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, "/bookstore/categories[@code='1']", + '{"books": [ {"title":"I\'m escaping"} ] }', OffsetDateTime.now()) + when: 'a query is executed' + def result = objectUnderTest.queryDataNodes(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, cpsPath, OMIT_DESCENDANTS) + then: 'the node is returned' + assert result.size() == 1 + assert result[0].xpath == "/bookstore/categories[@code='1']/books[@title='I''m escaping']" + cleanup: 'the new datanode' + cpsDataService.deleteDataNode(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, "/bookstore/categories[@code='1']/books[@title='I''m escaping']", OffsetDateTime.now()) + where: + quotes || cpsPath + 'single quotes' || "/bookstore/categories[@code='1']/books[@title='I''m escaping']" + 'double quotes' || '/bookstore/categories[@code="1"]/books[@title="I\'m escaping"]' + 'text-condition' || "/bookstore/categories[@code='1']/books/title[text()='I''m escaping']" + 'contains-condition' || "/bookstore/categories[@code='1']/books[contains(@title, 'I''m escaping')]" + } } |