From f93a6c0ab006258628fc2da69dcfe65010baf848 Mon Sep 17 00:00:00 2001 From: "adheli.tavares" Date: Wed, 6 Apr 2022 09:53:27 +0100 Subject: Error handling when a decoder fails to parse policy - catch exception only if it was a parse error (still raises exception when no decoder found) - reducing of complexity on acm decoding for sonar - added some debug logging and exception messages for better tracking of issues Issue-ID: POLICY-4006 Change-Id: Ie09aaf541fc06244b84477ecbfe70fc837438a86 Signed-off-by: adheli.tavares --- .../AutomationCompositionDecoderFileInCsar.java | 77 ++++++++++++---------- .../file/PolicyDecoderFileInCsarToPolicy.java | 30 +++------ .../distribution/reception/util/ReceptionUtil.java | 12 ++-- .../reception/util/ReceptionUtilTest.java | 40 +++++++++++ .../handling/AbstractReceptionHandler.java | 19 ++++-- .../handling/AbstractReceptionHandlerTest.java | 15 ++--- 6 files changed, 122 insertions(+), 71 deletions(-) create mode 100644 plugins/reception-plugins/src/test/java/org/onap/policy/distribution/reception/util/ReceptionUtilTest.java diff --git a/plugins/reception-plugins/src/main/java/org/onap/policy/distribution/reception/decoding/policy/file/AutomationCompositionDecoderFileInCsar.java b/plugins/reception-plugins/src/main/java/org/onap/policy/distribution/reception/decoding/policy/file/AutomationCompositionDecoderFileInCsar.java index 7bfcb0f9..fb7a8d99 100644 --- a/plugins/reception-plugins/src/main/java/org/onap/policy/distribution/reception/decoding/policy/file/AutomationCompositionDecoderFileInCsar.java +++ b/plugins/reception-plugins/src/main/java/org/onap/policy/distribution/reception/decoding/policy/file/AutomationCompositionDecoderFileInCsar.java @@ -1,7 +1,6 @@ /*- * ============LICENSE_START======================================================= - * Copyright (C) 2022 Nordix Foundation. - * Modifications Copyright (C) 2022 Nordix Foundation. + * Copyright (C) 2021-2022 Nordix Foundation. * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -24,7 +23,9 @@ package org.onap.policy.distribution.reception.decoding.policy.file; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; -import java.util.Enumeration; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import org.onap.policy.common.parameters.ParameterService; @@ -70,42 +71,24 @@ public class AutomationCompositionDecoderFileInCsar implements PolicyDecoder decode(final Csar csar) throws PolicyDecodingException { final Collection automationCompositionList = new ArrayList<>(); - ToscaServiceTemplate nodeTypes = null; - ToscaServiceTemplate dataTypes = null; try (var zipFile = new ZipFile(csar.getCsarFilePath())) { - final Enumeration entries = zipFile.entries(); - while (entries.hasMoreElements()) { - // - // Sonar will flag this as a Security Hotspot - // "Expanding archive files is security-sensitive" - // isZipEntryValid ensures the file being read exists in the archive - // - final ZipEntry entry = entries.nextElement(); // NOSONAR - final String entryName = entry.getName(); - - // Store node_types - if (entryName.contains(NODE_TYPES)) { - nodeTypes = ReceptionUtil.decodeFile(zipFile, entry); - } + final List entries = zipFile.stream() + .filter(entry -> entry.getName().contains(decoderParameters.getAutomationCompositionType())) + .collect(Collectors.toList()); - // Store data_types - if (entryName.contains(DATA_TYPES)) { - dataTypes = ReceptionUtil.decodeFile(zipFile, entry); - } + for (ZipEntry entry : entries) { + ReceptionUtil.validateZipEntry(entry.getName(), csar.getCsarFilePath(), entry.getSize()); + final ToscaServiceTemplate automationComposition = ReceptionUtil.decodeFile(zipFile, entry); + + if (null != automationComposition.getToscaTopologyTemplate()) { + validateTypes(zipFile, NODE_TYPES) + .ifPresent(node -> automationComposition.setNodeTypes(node.getNodeTypes())); + + validateTypes(zipFile, DATA_TYPES) + .ifPresent(data -> automationComposition.setDataTypes(data.getDataTypes())); - if (entryName.contains(decoderParameters.getAutomationCompositionType())) { - ReceptionUtil.validateZipEntry(entryName, csar.getCsarFilePath(), entry.getSize()); - final ToscaServiceTemplate automationComposition = ReceptionUtil.decodeFile(zipFile, entry); - if (null != automationComposition.getToscaTopologyTemplate()) { - if (null != nodeTypes) { - automationComposition.setNodeTypes(nodeTypes.getNodeTypes()); - } - if (null != dataTypes) { - automationComposition.setDataTypes(dataTypes.getDataTypes()); - } - automationCompositionList.add(automationComposition); - } + automationCompositionList.add(automationComposition); } } } catch (final IOException | CoderException exp) { @@ -114,4 +97,28 @@ public class AutomationCompositionDecoderFileInCsar implements PolicyDecoder validateTypes(final ZipFile zipFile, String type) + throws CoderException { + + try { + ToscaServiceTemplate template = null; + final Optional file = zipFile.stream() + .filter(entry -> entry.getName().contains(type)).findFirst(); + + if (file.isPresent()) { + template = ReceptionUtil.decodeFile(zipFile, file.get()); + } + return Optional.ofNullable(template); + } catch (final IOException | CoderException exp) { + throw new CoderException("Couldn't decode " + type + " type", exp); + } + } } diff --git a/plugins/reception-plugins/src/main/java/org/onap/policy/distribution/reception/decoding/policy/file/PolicyDecoderFileInCsarToPolicy.java b/plugins/reception-plugins/src/main/java/org/onap/policy/distribution/reception/decoding/policy/file/PolicyDecoderFileInCsarToPolicy.java index c4ba21fe..8d0a554a 100644 --- a/plugins/reception-plugins/src/main/java/org/onap/policy/distribution/reception/decoding/policy/file/PolicyDecoderFileInCsarToPolicy.java +++ b/plugins/reception-plugins/src/main/java/org/onap/policy/distribution/reception/decoding/policy/file/PolicyDecoderFileInCsarToPolicy.java @@ -1,7 +1,7 @@ /*- * ============LICENSE_START======================================================= * Copyright (C) 2018 Ericsson. All rights reserved. - * Copyright (C) 2022 Nordix Foundation. + * Modifications Copyright (C) 2019, 2021-2022 Nordix Foundation. * Modifications Copyright (C) 2020-2021 AT&T Intellectual Property. All rights reserved. * Modifications Copyright (C) 2021 Bell Canada. All rights reserved. * ================================================================================ @@ -26,7 +26,8 @@ package org.onap.policy.distribution.reception.decoding.policy.file; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; -import java.util.Enumeration; +import java.util.List; +import java.util.stream.Collectors; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import org.onap.policy.common.parameters.ParameterService; @@ -72,25 +73,14 @@ public class PolicyDecoderFileInCsarToPolicy implements PolicyDecoder policyList = new ArrayList<>(); try (var zipFile = new ZipFile(csar.getCsarFilePath())) { - final Enumeration entries = zipFile.entries(); - while (entries.hasMoreElements()) { - // - // Sonar will flag this as a Security Hotspot - // "Expanding archive files is security-sensitive" - // isZipEntryValid ensures the file being read exists in the archive - // - final ZipEntry entry = entries.nextElement(); // NOSONAR - final String entryName = entry.getName(); + final List entries = zipFile.stream() + .filter(entry -> entry.getName().contains(decoderParameters.getPolicyTypeFileName()) + || entry.getName().contains(decoderParameters.getPolicyFileName())).collect(Collectors.toList()); - // - // We only care about policy types and policies - // - if (entryName.contains(decoderParameters.getPolicyTypeFileName()) - || entryName.contains(decoderParameters.getPolicyFileName())) { - ReceptionUtil.validateZipEntry(entryName, csar.getCsarFilePath(), entry.getSize()); - final ToscaServiceTemplate policy = ReceptionUtil.decodeFile(zipFile, entry); - policyList.add(policy); - } + for (ZipEntry entry : entries) { + ReceptionUtil.validateZipEntry(entry.getName(), csar.getCsarFilePath(), entry.getSize()); + final ToscaServiceTemplate policy = ReceptionUtil.decodeFile(zipFile, entry); + policyList.add(policy); } } catch (final IOException | CoderException exp) { throw new PolicyDecodingException("Failed decoding the policy", exp); diff --git a/plugins/reception-plugins/src/main/java/org/onap/policy/distribution/reception/util/ReceptionUtil.java b/plugins/reception-plugins/src/main/java/org/onap/policy/distribution/reception/util/ReceptionUtil.java index 9c0bab43..c26286da 100644 --- a/plugins/reception-plugins/src/main/java/org/onap/policy/distribution/reception/util/ReceptionUtil.java +++ b/plugins/reception-plugins/src/main/java/org/onap/policy/distribution/reception/util/ReceptionUtil.java @@ -1,7 +1,6 @@ /*- * ============LICENSE_START======================================================= * Copyright (C) 2022 Nordix Foundation. - * Modifications Copyright (C) 2022 Nordix Foundation. * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +21,7 @@ package org.onap.policy.distribution.reception.util; import java.io.IOException; +import java.io.InvalidClassException; import java.nio.file.Path; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; @@ -38,8 +38,12 @@ import org.onap.policy.models.tosca.authorative.concepts.ToscaServiceTemplate; */ public class ReceptionUtil { - private static StandardCoder coder = new StandardCoder(); - private static StandardYamlCoder yamlCoder = new StandardYamlCoder(); + private ReceptionUtil() throws InvalidClassException { + throw new InvalidClassException("Can't instantiate a helper class!"); + } + + private static final StandardCoder coder = new StandardCoder(); + private static final StandardYamlCoder yamlCoder = new StandardYamlCoder(); private static final long MAX_FILE_SIZE = 512L * 1024; /** @@ -66,7 +70,7 @@ public class ReceptionUtil { // // Throw an exception if path is outside the csar // - if (! path.startsWith(csarPath)) { + if (!path.startsWith(csarPath)) { throw new PolicyDecodingException("Potential path injection for zip entry " + entryName); } } diff --git a/plugins/reception-plugins/src/test/java/org/onap/policy/distribution/reception/util/ReceptionUtilTest.java b/plugins/reception-plugins/src/test/java/org/onap/policy/distribution/reception/util/ReceptionUtilTest.java new file mode 100644 index 00000000..6747618e --- /dev/null +++ b/plugins/reception-plugins/src/test/java/org/onap/policy/distribution/reception/util/ReceptionUtilTest.java @@ -0,0 +1,40 @@ +/*- + * ============LICENSE_START======================================================= + * Copyright (C) 2022 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========================================================= + */ + +package org.onap.policy.distribution.reception.util; + +import org.assertj.core.api.Assertions; +import org.junit.Test; + +/** + * Class for testing {@link ReceptionUtil}. + */ + +public class ReceptionUtilTest { + + @Test + public void testValidateZipEntry_InvalidSize() { + long invalidFileSize = 512L * 2048; + + Assertions.assertThatThrownBy(() -> + ReceptionUtil.validateZipEntry("entryName", "csarPath", invalidFileSize)) + .hasMessage("Zip entry for entryName is too large " + invalidFileSize); + } +} diff --git a/reception/src/main/java/org/onap/policy/distribution/reception/handling/AbstractReceptionHandler.java b/reception/src/main/java/org/onap/policy/distribution/reception/handling/AbstractReceptionHandler.java index 8ad293c3..e898b335 100644 --- a/reception/src/main/java/org/onap/policy/distribution/reception/handling/AbstractReceptionHandler.java +++ b/reception/src/main/java/org/onap/policy/distribution/reception/handling/AbstractReceptionHandler.java @@ -1,7 +1,7 @@ /*- * ============LICENSE_START======================================================= * Copyright (C) 2018 Ericsson. All rights reserved. - * Copyright (C) 2019 Nordix Foundation. + * Modifications Copyright (C) 2019, 2022 Nordix Foundation. * Modifications Copyright (C) 2020-2021 AT&T Intellectual Property. All rights reserved. * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); @@ -72,17 +72,28 @@ public abstract class AbstractReceptionHandler implements ReceptionHandler { * handler. * * @param policyInput the input that has been received - * @throws PolicyDecodingException if an error occurs in decoding a policy from the received input + * @throws PolicyDecodingException if an error occurs when no decoders are available */ protected void inputReceived(final PolicyInput policyInput) throws PolicyDecodingException { final Collection policies = new ArrayList<>(); - for (final PolicyDecoder policyDecoder : getRelevantPolicyDecoders(policyInput)) { - policies.addAll(policyDecoder.decode(policyInput)); + + try { + for (final PolicyDecoder policyDecoder : getRelevantPolicyDecoders(policyInput)) { + LOGGER.debug("Policy decoder: {}", policyDecoder.getClass()); + policies.addAll(policyDecoder.decode(policyInput)); + } + } catch (PolicyDecodingException decodingException) { + if (decodingException.getMessage().contains("No decoder")) { + throw decodingException; + } else { + LOGGER.error("Couldn't decode the policy", decodingException); + } } for (final PolicyForwarder policyForwarder : pluginHandler.getPolicyForwarders()) { try { + LOGGER.debug("Trying to forward policy to {}", policyForwarder.getClass()); policyForwarder.forward(policies); } catch (final PolicyForwardingException policyForwardingException) { LOGGER.error("Error when forwarding policies to {}", policyForwarder, policyForwardingException); diff --git a/reception/src/test/java/org/onap/policy/distribution/reception/handling/AbstractReceptionHandlerTest.java b/reception/src/test/java/org/onap/policy/distribution/reception/handling/AbstractReceptionHandlerTest.java index 2f2d42c3..1dc4bfe0 100644 --- a/reception/src/test/java/org/onap/policy/distribution/reception/handling/AbstractReceptionHandlerTest.java +++ b/reception/src/test/java/org/onap/policy/distribution/reception/handling/AbstractReceptionHandlerTest.java @@ -1,6 +1,7 @@ /*- * ============LICENSE_START======================================================= * Copyright (C) 2018 Ericsson. All rights reserved. + * Modifications Copyright (C) 2022 Nordix Foundation. * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -118,10 +119,10 @@ public class AbstractReceptionHandlerTest { handler.inputReceived(new DummyPolicyInput()); } - class DummyPolicyInput implements PolicyInput { + static class DummyPolicyInput implements PolicyInput { } - class DummyPolicy1 extends ToscaEntity { + static class DummyPolicy1 extends ToscaEntity { @Override public String getName() { @@ -129,7 +130,7 @@ public class AbstractReceptionHandlerTest { } } - class DummyPolicy2 extends ToscaEntity { + static class DummyPolicy2 extends ToscaEntity { @Override public String getName() { @@ -161,7 +162,7 @@ public class AbstractReceptionHandlerTest { } private Map getPolicyDecoders() { - final Map policyDecoders = new HashMap(); + final Map policyDecoders = new HashMap<>(); final PolicyDecoderParameters pDParameters = new PolicyDecoderParameters(DECODER_TYPE, DECODER_CLASS_NAME, DECODER_CONFIGURATION_PARAMETERS); policyDecoders.put(DECODER_KEY, pDParameters); @@ -170,7 +171,7 @@ public class AbstractReceptionHandlerTest { private Map getPolicyForwarders() { final Map policyForwarders = - new HashMap(); + new HashMap<>(); final PolicyForwarderParameters pFParameters = new PolicyForwarderParameters(FORWARDER_TYPE, FORWARDER_CLASS_NAME, FORWARDER_CONFIGURATION_PARAMETERS); policyForwarders.put(FORWARDER_KEY, pFParameters); @@ -180,9 +181,7 @@ public class AbstractReceptionHandlerTest { private PluginHandlerParameters getPluginHandlerParameters() { final Map policyDecoders = getPolicyDecoders(); final Map policyForwarders = getPolicyForwarders(); - final PluginHandlerParameters pluginHandlerParameters = - new PluginHandlerParameters(policyDecoders, policyForwarders); - return pluginHandlerParameters; + return new PluginHandlerParameters(policyDecoders, policyForwarders); } } -- cgit 1.2.3-korg