Fix property name regex

property name should contain only letters, numbers and / or underscores

Change-Id: Ic6b8d627a30f5f157bed1617a8af819b66352136
Issue-ID: SDC-2243
Signed-off-by: talio <tali.orenbach@amdocs.com>

4 files changed, 65 insertions, 59 deletions

diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/PropertyBusinessLogic.java b/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/PropertyBusinessLogic.java
index 3c49e21471..275d1726ed 100644
--- a/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/PropertyBusinessLogic.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/be/components/impl/PropertyBusinessLogic.java
@@ -22,6 +22,13 @@ package org.openecomp.sdc.be.components.impl;
 
 import com.google.gson.JsonElement;
 import fj.data.Either;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.function.Supplier;
+import javax.servlet.ServletContext;
 import org.apache.commons.collections.CollectionUtils;
 import org.apache.commons.collections.MapUtils;
 import org.apache.commons.lang3.tuple.ImmutablePair;
@@ -54,10 +61,6 @@ import org.openecomp.sdc.common.log.wrappers.Logger;
 import org.openecomp.sdc.exception.ResponseFormat;
 import org.springframework.web.context.WebApplicationContext;
 
-import javax.servlet.ServletContext;
-import java.util.*;
-import java.util.function.Supplier;
-
 @org.springframework.stereotype.Component("propertyBusinessLogic")
 public class PropertyBusinessLogic extends BaseBusinessLogic {
 
@@ -467,31 +470,6 @@ public class PropertyBusinessLogic extends BaseBusinessLogic {
         return propertyCandidate.isPresent();
     }
 
-    private boolean isPropertyExist(List<PropertyDefinition> properties, String resourceUid, String propertyName, String propertyType) {
-        boolean result = false;
-        if (!CollectionUtils.isEmpty(properties)) {
-            for (PropertyDefinition propertyDefinition : properties) {
-
-                if ( propertyDefinition.getName().equals(propertyName) &&
-                    (propertyDefinition.getParentUniqueId().equals(resourceUid) || !propertyDefinition.getType().equals(propertyType)) ) {
-                    result = true;
-                    break;
-                }
-            }
-        }
-        return result;
-    }
-
-    private Either<PropertyDefinition, StorageOperationStatus> handleProperty(PropertyDefinition newPropertyDefinition, Map<String, DataTypeDefinition> dataTypes) {
-
-        StorageOperationStatus validateAndUpdateProperty = validateAndUpdateProperty(newPropertyDefinition, dataTypes);
-        if (validateAndUpdateProperty != StorageOperationStatus.OK) {
-            return Either.right(validateAndUpdateProperty);
-        }
-
-        return Either.left(newPropertyDefinition);
-    }
-
     private StorageOperationStatus validateAndUpdateProperty(IComplexDefaultValue propertyDefinition, Map<String, DataTypeDefinition> dataTypes) {
 
         log.trace("Going to validate property type and value. {}", propertyDefinition);
diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/BeGenericServlet.java b/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/BeGenericServlet.java
index b397439a99..8e7d406bea 100644
--- a/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/BeGenericServlet.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/BeGenericServlet.java
@@ -33,6 +33,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Objects;
 import java.util.Set;
 import java.util.function.Supplier;
 import javax.servlet.ServletContext;
@@ -96,6 +97,8 @@ public class BeGenericServlet extends BasicServlet {
 
     private static final Logger log = Logger.getLogger(BeGenericServlet.class);
 
+    private static final String PROPERTY_NAME_REGEX = "[\\w,\\d,_]+";
+
     /******************** New error response mechanism
      * @param requestErrorWrapper **************/
 
@@ -315,12 +318,11 @@ public class BeGenericServlet extends BasicServlet {
         }
     }
 
-    protected Either<Map<String, PropertyDefinition>, ActionStatus> getPropertyModel(String componentId,
-                                                                                     String data) {
+    protected Either<Map<String, PropertyDefinition>, ActionStatus> getPropertyModel(String componentId, String data) {
         JSONParser parser = new JSONParser();
         JSONObject root;
         try {
-            Map<String, PropertyDefinition> properties = new HashMap<String, PropertyDefinition>();
+            Map<String, PropertyDefinition> properties = new HashMap<>();
             root = (JSONObject) parser.parse(data);
 
             Set entrySet = root.entrySet();
@@ -328,16 +330,20 @@ public class BeGenericServlet extends BasicServlet {
             while (iterator.hasNext()) {
                 Entry next = (Entry) iterator.next();
                 String propertyName = (String) next.getKey();
+
+                if(!isPropertyNameValid(propertyName)) {
+                    return Either.right(ActionStatus.INVALID_PROPERTY_NAME);
+                }
+
                 JSONObject value = (JSONObject) next.getValue();
-                String jsonString = value.toJSONString();
-                Either<PropertyDefinition, ActionStatus> convertJsonToObject = convertJsonToObject(jsonString, PropertyDefinition.class);
-                if (convertJsonToObject.isRight()) {
-                    return Either.right(convertJsonToObject.right().value());
+                Either<PropertyDefinition, ActionStatus> propertyDefinitionEither =
+                        getPropertyDefinitionFromJson(componentId, propertyName, value);
+
+                if(propertyDefinitionEither.isRight()) {
+                    return Either.right(propertyDefinitionEither.right().value());
                 }
-                PropertyDefinition propertyDefinition = convertJsonToObject.left().value();
-                String uniqueId = UniqueIdBuilder.buildPropertyUniqueId(componentId, (String) propertyName);
-                propertyDefinition.setUniqueId(uniqueId);
-                properties.put(propertyName, propertyDefinition);
+
+                properties.put(propertyName, propertyDefinitionEither.left().value());
             }
 
             return Either.left(properties);
@@ -347,6 +353,25 @@ public class BeGenericServlet extends BasicServlet {
         }
     }
 
+    protected boolean isPropertyNameValid(String propertyName) {
+        return Objects.nonNull(propertyName)
+                       && propertyName.matches(PROPERTY_NAME_REGEX);
+
+    }
+
+    private Either<PropertyDefinition, ActionStatus> getPropertyDefinitionFromJson(String componentId, String propertyName, JSONObject value) {
+        String jsonString = value.toJSONString();
+        Either<PropertyDefinition, ActionStatus> convertJsonToObject = convertJsonToObject(jsonString, PropertyDefinition.class);
+        if (convertJsonToObject.isRight()) {
+            return Either.right(convertJsonToObject.right().value());
+        }
+        PropertyDefinition propertyDefinition = convertJsonToObject.left().value();
+        String uniqueId = UniqueIdBuilder.buildPropertyUniqueId(componentId, propertyName);
+        propertyDefinition.setUniqueId(uniqueId);
+
+        return Either.left(propertyDefinition);
+    }
+
     protected Either<Map<String, PropertyDefinition>, ActionStatus> getPropertiesListForUpdate(String data) {
 
         Map<String, PropertyDefinition> properties = new HashMap<>();
diff --git a/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/ComponentPropertyServlet.java b/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/ComponentPropertyServlet.java
index 38c04a131c..037bd95190 100644
--- a/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/ComponentPropertyServlet.java
+++ b/catalog-be/src/main/java/org/openecomp/sdc/be/servlets/ComponentPropertyServlet.java
@@ -17,30 +17,14 @@
 package org.openecomp.sdc.be.servlets;
 
 import com.jcabi.aspects.Loggable;
-
-import java.util.List;
-import java.util.Map;
-
 import fj.data.Either;
 import io.swagger.annotations.Api;
 import io.swagger.annotations.ApiOperation;
 import io.swagger.annotations.ApiParam;
 import io.swagger.annotations.ApiResponse;
 import io.swagger.annotations.ApiResponses;
-import org.openecomp.sdc.be.components.impl.PropertyBusinessLogic;
-import org.openecomp.sdc.be.config.BeEcompErrorManager;
-import org.openecomp.sdc.be.dao.api.ActionStatus;
-import org.openecomp.sdc.be.datamodel.utils.PropertyValueConstraintValidationUtil;
-import org.openecomp.sdc.be.model.PropertyDefinition;
-import org.openecomp.sdc.be.model.User;
-import org.openecomp.sdc.be.model.cache.ApplicationDataTypeCache;
-import org.openecomp.sdc.be.resources.data.EntryData;
-import org.openecomp.sdc.common.api.Constants;
-import org.openecomp.sdc.exception.ResponseFormat;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.springframework.beans.factory.annotation.Autowired;
-
+import java.util.List;
+import java.util.Map;
 import javax.inject.Singleton;
 import javax.servlet.ServletContext;
 import javax.servlet.http.HttpServletRequest;
@@ -56,6 +40,19 @@ import javax.ws.rs.Produces;
 import javax.ws.rs.core.Context;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
+import org.openecomp.sdc.be.components.impl.PropertyBusinessLogic;
+import org.openecomp.sdc.be.config.BeEcompErrorManager;
+import org.openecomp.sdc.be.dao.api.ActionStatus;
+import org.openecomp.sdc.be.datamodel.utils.PropertyValueConstraintValidationUtil;
+import org.openecomp.sdc.be.model.PropertyDefinition;
+import org.openecomp.sdc.be.model.User;
+import org.openecomp.sdc.be.model.cache.ApplicationDataTypeCache;
+import org.openecomp.sdc.be.resources.data.EntryData;
+import org.openecomp.sdc.common.api.Constants;
+import org.openecomp.sdc.exception.ResponseFormat;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
 
 @Loggable(prepend = true, value = Loggable.DEBUG, trim = false)
 @Path("/v1/catalog")
@@ -216,6 +213,7 @@ public class ComponentPropertyServlet extends BeGenericServlet {
     log.debug("Start handle request of {} modifier id is {} data is {}", url, userId, data);
 
     try{
+      PropertyBusinessLogic propertyBL = getPropertyBL(context);
       Either<Map<String, PropertyDefinition>, ActionStatus> propertyDefinition =
               getPropertyModel(componentId, data);
       if (propertyDefinition.isRight()) {
@@ -235,7 +233,6 @@ public class ComponentPropertyServlet extends BeGenericServlet {
       newPropertyDefinition.setParentUniqueId(componentId);
       String propertyName = newPropertyDefinition.getName();
 
-      PropertyBusinessLogic propertyBL = getPropertyBL(context);
       Either<EntryData<String, PropertyDefinition>, ResponseFormat> addPropertyEither =
               propertyBL.addPropertyToComponent(componentId, propertyName, newPropertyDefinition, userId);
 
diff --git a/catalog-be/src/main/resources/config/error-configuration.yaml b/catalog-be/src/main/resources/config/error-configuration.yaml
index 312d90c307..f9e5407e30 100644
--- a/catalog-be/src/main/resources/config/error-configuration.yaml
+++ b/catalog-be/src/main/resources/config/error-configuration.yaml
@@ -2268,4 +2268,10 @@ errors:
         code: 400,
         message: "Error: Invalid property values provided:\n %1",
         messageId: "SVC4735"
+    }
+#---------SVC4727------------------------------
+    INVALID_PROPERTY_NAME: {
+        code: 400,
+        message: "Error: Property name contains invalid characters. It should have only letters, numbers and underscores.",
+        messageId: "SVC4727"
     }
\ No newline at end of file