From 74a47154f3bce495d9f58a300a860d750ae309f1 Mon Sep 17 00:00:00 2001 From: danielhanrahan Date: Wed, 28 Jun 2023 12:55:20 +0100 Subject: Apostrophe handling in CpsPathParser Apostrophe is not currently handled correctly, and having apostrophe in the xpath will lead to various errors. For example, normalizing this xpath works: /path[@name="I'm quoted"] -> /path[@name='I\'m quoted'] However the resulting xpath will throw a PathParsingException if parsed! (Thus path normalization is not idempotent.) - Use '' for escaping apostrophe in single quoted leaf value, to comply with XPath standard (and use "" for escaping in "). - Use Liquibase to make existing data comply with new rules. - Leaf values in data leaves are now unescaped, e.g. "I'm quoted" - Quoting is now consistent for leaf/text/contains conditions. Issue-ID: CPS-1769 Signed-off-by: danielhanrahan Change-Id: Iafc287f738254d7f99706c6bc548091c0ecd5aa0 --- .../cps/spi/repository/FragmentQueryBuilder.java | 2 +- .../onap/cps/spi/repository/TempTableCreator.java | 7 ++---- .../java/org/onap/cps/spi/utils/EscapeUtils.java | 8 ++++-- .../main/resources/changelog/changelog-master.yaml | 2 ++ .../changes/21-escape-quotes-in-xpath-forward.sql | 19 ++++++++++++++ .../changes/21-escape-quotes-in-xpath-rollback.sql | 19 ++++++++++++++ .../db/changes/21-escape-quotes-in-xpath.yaml | 29 ++++++++++++++++++++++ .../org/onap/cps/spi/utils/EscapeUtilsSpec.groovy | 7 +++++- 8 files changed, 84 insertions(+), 9 deletions(-) create mode 100644 cps-ri/src/main/resources/changelog/db/changes/21-escape-quotes-in-xpath-forward.sql create mode 100644 cps-ri/src/main/resources/changelog/db/changes/21-escape-quotes-in-xpath-rollback.sql create mode 100644 cps-ri/src/main/resources/changelog/db/changes/21-escape-quotes-in-xpath.yaml (limited to 'cps-ri/src') 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 be06ebac0..e371035ba 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 139a8b306..4c7971ead 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 sqlInserts = new HashSet<>(sqlData.size()); for (final Collection rowValues : sqlData) { final Collection 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 3092b7905..2b61d3950 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 4e6986e71..f76c5ba3b 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 000000000..9bf7f9a74 --- /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 000000000..0fd1633a5 --- /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 000000000..7b5b1dbd0 --- /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 7de9b97ba..52330e625 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!" + } + } -- cgit 1.2.3-korg