From 521000b2278eaf9fb1e57533e523014da1900098 Mon Sep 17 00:00:00 2001 From: talio Date: Sun, 21 Apr 2019 12:43:15 +0300 Subject: 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 --- .../be/components/impl/PropertyBusinessLogic.java | 36 ++---- .../sdc/be/servlets/BeGenericServlet.java | 47 ++++++-- .../sdc/be/servlets/ComponentPropertyServlet.java | 35 +++--- .../main/resources/config/error-configuration.yaml | 6 + .../be/servlets/ComponentPropertyServletTest.java | 128 +++++++++++++++++++++ .../org/openecomp/sdc/be/dao/api/ActionStatus.java | 2 + 6 files changed, 195 insertions(+), 59 deletions(-) create mode 100644 catalog-be/src/test/java/org/openecomp/sdc/be/servlets/ComponentPropertyServletTest.java 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 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 handleProperty(PropertyDefinition newPropertyDefinition, Map dataTypes) { - - StorageOperationStatus validateAndUpdateProperty = validateAndUpdateProperty(newPropertyDefinition, dataTypes); - if (validateAndUpdateProperty != StorageOperationStatus.OK) { - return Either.right(validateAndUpdateProperty); - } - - return Either.left(newPropertyDefinition); - } - private StorageOperationStatus validateAndUpdateProperty(IComplexDefaultValue propertyDefinition, Map 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, ActionStatus> getPropertyModel(String componentId, - String data) { + protected Either, ActionStatus> getPropertyModel(String componentId, String data) { JSONParser parser = new JSONParser(); JSONObject root; try { - Map properties = new HashMap(); + Map 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 convertJsonToObject = convertJsonToObject(jsonString, PropertyDefinition.class); - if (convertJsonToObject.isRight()) { - return Either.right(convertJsonToObject.right().value()); + Either 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 getPropertyDefinitionFromJson(String componentId, String propertyName, JSONObject value) { + String jsonString = value.toJSONString(); + Either 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, ActionStatus> getPropertiesListForUpdate(String data) { Map 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, 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, 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 diff --git a/catalog-be/src/test/java/org/openecomp/sdc/be/servlets/ComponentPropertyServletTest.java b/catalog-be/src/test/java/org/openecomp/sdc/be/servlets/ComponentPropertyServletTest.java new file mode 100644 index 0000000000..b816702317 --- /dev/null +++ b/catalog-be/src/test/java/org/openecomp/sdc/be/servlets/ComponentPropertyServletTest.java @@ -0,0 +1,128 @@ +package org.openecomp.sdc.be.servlets; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.when; + +import com.google.gson.Gson; +import fj.data.Either; +import javax.servlet.ServletContext; +import javax.servlet.http.HttpSession; +import javax.ws.rs.core.Response; +import org.glassfish.grizzly.http.util.HttpStatus; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Spy; +import org.mockito.junit.MockitoJUnitRunner; +import org.openecomp.sdc.be.components.impl.PropertyBusinessLogic; +import org.openecomp.sdc.be.dao.api.ActionStatus; +import org.openecomp.sdc.be.impl.ComponentsUtils; +import org.openecomp.sdc.be.impl.WebAppContextWrapper; +import org.openecomp.sdc.be.model.PropertyDefinition; +import org.openecomp.sdc.be.resources.data.EntryData; +import org.openecomp.sdc.common.api.Constants; +import org.openecomp.sdc.exception.ResponseFormat; +import org.springframework.web.context.WebApplicationContext; + +@RunWith(MockitoJUnitRunner.class) +public class ComponentPropertyServletTest extends JerseySpringBaseTest { + @Mock + private static HttpSession session; + @Mock + private static ServletContext context; + @Mock + private static WebAppContextWrapper wrapper; + @Mock + private static WebApplicationContext webAppContext; + @Mock + private static PropertyBusinessLogic propertyBl; + @Mock + private static ComponentsUtils componentsUtils; + @InjectMocks + @Spy + private ComponentPropertyServlet componentPropertyServlet; + + private static final String SERVICE_ID = "service1"; + private final static String USER_ID = "jh0003"; + private static final String VALID_PROPERTY_NAME = "valid_name_123"; + private static final String INVALID_PROPERTY_NAME = "invalid_name_$.&"; + private static final String STRING_TYPE = "string"; + + @Before + public void initClass() { + initMockitoStubbings(); + } + + @Test + public void testCreatePropertyOnService_success() { + PropertyDefinition property = new PropertyDefinition(); + property.setName(VALID_PROPERTY_NAME); + property.setType(STRING_TYPE); + + EntryData propertyEntry = new EntryData<>(VALID_PROPERTY_NAME, property); + when(propertyBl.addPropertyToComponent(eq(SERVICE_ID), any(), any(), any())).thenReturn(Either.left(propertyEntry)); + + Response propertyInService = + componentPropertyServlet.createPropertyInService(SERVICE_ID, getValidProperty(), request, USER_ID); + + Assert.assertEquals(HttpStatus.OK_200.getStatusCode(), propertyInService.getStatus()); + } + + @Test + public void testCreatePropertyInvalidName_failure() { + PropertyDefinition property = new PropertyDefinition(); + property.setName(INVALID_PROPERTY_NAME); + property.setType(STRING_TYPE); + + ResponseFormat responseFormat = new ResponseFormat(); + responseFormat.setStatus(HttpStatus.BAD_REQUEST_400.getStatusCode()); + + when(componentsUtils.getResponseFormat(eq(ActionStatus.INVALID_PROPERTY_NAME))).thenReturn(responseFormat); + + + Response propertyInService = + componentPropertyServlet.createPropertyInService(SERVICE_ID, getInvalidProperty(), request, USER_ID); + + Assert.assertEquals(HttpStatus.BAD_REQUEST_400.getStatusCode(), propertyInService.getStatus()); + } + + private static void initMockitoStubbings() { + when(request.getSession()).thenReturn(session); + when(session.getServletContext()).thenReturn(context); + when(context.getAttribute(eq(Constants.WEB_APPLICATION_CONTEXT_WRAPPER_ATTR))).thenReturn(wrapper); + when(wrapper.getWebAppContext(any())).thenReturn(webAppContext); + when(webAppContext.getBean(eq(PropertyBusinessLogic.class))).thenReturn(propertyBl); + when(webAppContext.getBean(eq(ComponentsUtils.class))).thenReturn(componentsUtils); + } + + private String getValidProperty() { + return "{\n" + + " \"valid_name_123\": {\n" + + " \"schema\": {\n" + + " \"property\": {\n" + + " \"type\": \"\"\n" + + " }\n" + " },\n" + + " \"type\": \"string\",\n" + + " \"name\": \"valid_name_123\"\n" + + " }\n" + + "}"; + } + + private String getInvalidProperty() { + return "{\n" + + " \"invalid_name_$.&\": {\n" + + " \"schema\": {\n" + + " \"property\": {\n" + + " \"type\": \"\"\n" + + " }\n" + " },\n" + + " \"type\": \"string\",\n" + + " \"name\": \"invalid_name_$.&\"\n" + + " }\n" + + "}"; + } + +} diff --git a/catalog-dao/src/main/java/org/openecomp/sdc/be/dao/api/ActionStatus.java b/catalog-dao/src/main/java/org/openecomp/sdc/be/dao/api/ActionStatus.java index 2a36b05189..6fd2eb26c8 100644 --- a/catalog-dao/src/main/java/org/openecomp/sdc/be/dao/api/ActionStatus.java +++ b/catalog-dao/src/main/java/org/openecomp/sdc/be/dao/api/ActionStatus.java @@ -149,6 +149,8 @@ public enum ActionStatus { //InterfaceLifeCycleType INTERFACE_LIFECYCLE_TYPES_NOT_FOUND, + INVALID_PROPERTY_NAME, + //Capability related CAPABILITY_NOT_FOUND, CAPABILITY_NAME_MANDATORY, CAPABILITY_TYPE_MANDATORY,CAPABILITY_NAME_ALREADY_IN_USE, MAX_OCCURRENCES_SHOULD_BE_GREATER_THAN_MIN_OCCURRENCES, CAPABILITY_DELETION_NOT_ALLOWED_USED_IN_COMPOSITION, -- cgit 1.2.3-korg