From 8ade4cb3f9a80a8214578c6763519f559774db44 Mon Sep 17 00:00:00 2001 From: Alan Davis Date: Fri, 6 Sep 2013 07:09:52 +0000 Subject: [PATCH] Merged HEAD-BUG-FIX to HEAD (4.2) 54919: MNT-9571: Merged V4.1-BUG-FIX (4.1.7) to HEAD-BUG-FIX (4.2) 54377: MNT-9090 "CMIS: Integer overflow in properties": checks for overflow + tests git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@55006 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../org/alfresco/opencmis/CMISConnector.java | 108 +++++++++++++++--- .../org/alfresco/opencmis/CMISTest.java | 87 ++++++++++++++ 2 files changed, 181 insertions(+), 14 deletions(-) diff --git a/source/java/org/alfresco/opencmis/CMISConnector.java b/source/java/org/alfresco/opencmis/CMISConnector.java index 836008de30..b89e88b968 100644 --- a/source/java/org/alfresco/opencmis/CMISConnector.java +++ b/source/java/org/alfresco/opencmis/CMISConnector.java @@ -93,6 +93,7 @@ import org.alfresco.service.cmr.audit.AuditQueryParameters; import org.alfresco.service.cmr.audit.AuditService; import org.alfresco.service.cmr.audit.AuditService.AuditQueryCallback; import org.alfresco.service.cmr.coci.CheckOutCheckInService; +import org.alfresco.service.cmr.dictionary.DataTypeDefinition; import org.alfresco.service.cmr.dictionary.DictionaryService; import org.alfresco.service.cmr.dictionary.InvalidAspectException; import org.alfresco.service.cmr.lock.LockService; @@ -272,6 +273,11 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen private static final String CMIS_USER = "cmis:user"; + private static final BigInteger maxInt = BigInteger.valueOf(Integer.MAX_VALUE); + private static final BigInteger minInt = BigInteger.valueOf(Integer.MIN_VALUE); + private static final BigInteger maxLong = BigInteger.valueOf(Long.MAX_VALUE); + private static final BigInteger minLong = BigInteger.valueOf(Long.MIN_VALUE); + // lifecycle private ProcessorLifecycle lifecycle = new ProcessorLifecycle(); @@ -2948,8 +2954,8 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen ArrayList values = new ArrayList(); if (property.getChildren() != null) { - try - { +// try +// { for (CmisExtensionElement valueElement : property.getChildren()) { if ("value".equals(valueElement.getName())) @@ -2957,28 +2963,80 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen switch (propertyType) { case BOOLEAN: - values.add(Boolean.parseBoolean(valueElement.getValue())); + try + { + values.add(Boolean.parseBoolean(valueElement.getValue())); + } + catch (Exception e) + { + throw new CmisInvalidArgumentException("Invalid property aspect value: " + propertyId, e); + } break; case DATETIME: - values.add(df.newXMLGregorianCalendar(valueElement.getValue()) - .toGregorianCalendar()); + try + { + values.add(df.newXMLGregorianCalendar(valueElement.getValue()) + .toGregorianCalendar()); + } + catch (Exception e) + { + throw new CmisInvalidArgumentException("Invalid property aspect value: " + propertyId, e); + } break; case INTEGER: - values.add(new BigInteger(valueElement.getValue())); + BigInteger value = null; + try + { + value = new BigInteger(valueElement.getValue()); + } + catch (Exception e) + { + throw new CmisInvalidArgumentException("Invalid property aspect value: " + propertyId, e); + } + + // overflow check + PropertyDefinitionWrapper propDef = cmisDictionaryService.findProperty(propertyId); + if(propDef == null) + { + throw new CmisInvalidArgumentException("Property " + propertyId + " is unknown!"); + } + + QName propertyQName = propDef.getPropertyAccessor().getMappedProperty(); + if (propertyQName == null) + { + throw new CmisConstraintException("Unable to set property " + propertyId + "!"); + } + + org.alfresco.service.cmr.dictionary.PropertyDefinition def = dictionaryService.getProperty(propertyQName); + QName dataDef = def.getDataType().getName(); + + if (dataDef.equals(DataTypeDefinition.INT) && (value.compareTo(maxInt) > 0 || value.compareTo(minInt) < 0)) + { + throw new CmisConstraintException("Value is out of range for property " + propertyId); + } + + if (dataDef.equals(DataTypeDefinition.LONG) && (value.compareTo(maxLong) > 0 || value.compareTo(minLong) < 0 )) + { + throw new CmisConstraintException("Value is out of range for property " + propertyId); + } + + values.add(value); break; case DECIMAL: - values.add(new BigDecimal(valueElement.getValue())); + try + { + values.add(new BigDecimal(valueElement.getValue())); + } + catch (Exception e) + { + throw new CmisInvalidArgumentException("Invalid property aspect value: " + propertyId, e); + } break; default: values.add(valueElement.getValue()); } } } - } - catch (Exception e) - { - throw new CmisInvalidArgumentException("Invalid property aspect value: " + propertyId, e); - } } aspectProperties.put(QName.createQName(propertyId, namespaceService), values); @@ -3069,6 +3127,8 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen // set property for (Map.Entry> property : aspectProperties.entrySet()) { + QName propertyQName = property.getKey(); + if (property.getValue().isEmpty()) { if(HiddenAspect.HIDDEN_PROPERTIES.contains(property.getKey())) @@ -3076,7 +3136,7 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen if(hiddenAspect.isClientControlled(nodeRef) || aspectProperties.containsKey(ContentModel.PROP_CLIENT_CONTROLLED)) { // manipulate hidden aspect property only if client controlled - nodeService.removeProperty(nodeRef, property.getKey()); + nodeService.removeProperty(nodeRef, propertyQName); } } else @@ -3097,8 +3157,9 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen } else { + Serializable value = (Serializable)property.getValue(); nodeService.setProperty(nodeRef, property.getKey(), property.getValue().size() == 1 ? property - .getValue().get(0) : (Serializable) property.getValue()); + .getValue().get(0) : value); } } } @@ -3161,6 +3222,7 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen throw new CmisConstraintException("Unable to set property " + property.getId() + "!"); } + if (property.getId().equals(PropertyIds.NAME)) { if (!(value instanceof String)) @@ -3189,6 +3251,24 @@ public class CMISConnector implements ApplicationContextAware, ApplicationListen } else { + // overflow check + if(propDef.getPropertyDefinition().getPropertyType() == PropertyType.INTEGER && value instanceof BigInteger) + { + org.alfresco.service.cmr.dictionary.PropertyDefinition def = dictionaryService.getProperty(propertyQName); + QName dataDef = def.getDataType().getName(); + BigInteger bigValue = (BigInteger)value; + + if ((bigValue.compareTo(maxInt) > 0 || bigValue.compareTo(minInt) < 0 ) && dataDef.equals(DataTypeDefinition.INT)) + { + throw new CmisConstraintException("Value is out of range for property " + propertyQName.getLocalName()); + } + + if ((bigValue.compareTo(maxLong) > 0 || bigValue.compareTo(minLong) < 0 ) && dataDef.equals(DataTypeDefinition.LONG)) + { + throw new CmisConstraintException("Value is out of range for property " + propertyQName.getLocalName()); + } + } + nodeService.setProperty(nodeRef, propertyQName, value); } } diff --git a/source/test-java/org/alfresco/opencmis/CMISTest.java b/source/test-java/org/alfresco/opencmis/CMISTest.java index 51bd1c0611..f3ff12c2d6 100644 --- a/source/test-java/org/alfresco/opencmis/CMISTest.java +++ b/source/test-java/org/alfresco/opencmis/CMISTest.java @@ -33,14 +33,18 @@ import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.cmr.rule.Rule; import org.alfresco.service.cmr.rule.RuleService; import org.alfresco.service.cmr.rule.RuleType; +import org.alfresco.service.namespace.NamespaceService; +import org.alfresco.service.namespace.QName; import org.alfresco.service.cmr.version.VersionService; import org.alfresco.service.transaction.TransactionService; import org.alfresco.util.ApplicationContextHelper; import org.apache.chemistry.opencmis.commons.PropertyIds; import org.apache.chemistry.opencmis.commons.data.AllowableActions; +import org.apache.chemistry.opencmis.commons.data.CmisExtensionElement; import org.apache.chemistry.opencmis.commons.data.ObjectData; import org.apache.chemistry.opencmis.commons.data.ObjectInFolderData; import org.apache.chemistry.opencmis.commons.data.ObjectInFolderList; +import org.apache.chemistry.opencmis.commons.data.Properties; import org.apache.chemistry.opencmis.commons.data.PropertyData; import org.apache.chemistry.opencmis.commons.data.RepositoryInfo; import org.apache.chemistry.opencmis.commons.enums.Action; @@ -48,6 +52,7 @@ import org.apache.chemistry.opencmis.commons.enums.CmisVersion; import org.apache.chemistry.opencmis.commons.enums.IncludeRelationships; import org.apache.chemistry.opencmis.commons.enums.VersioningState; import org.apache.chemistry.opencmis.commons.exceptions.CmisConstraintException; +import org.apache.chemistry.opencmis.commons.impl.dataobjects.CmisExtensionElementImpl; import org.apache.chemistry.opencmis.commons.impl.dataobjects.ContentStreamImpl; import org.apache.chemistry.opencmis.commons.impl.dataobjects.PropertiesImpl; import org.apache.chemistry.opencmis.commons.impl.dataobjects.PropertyIdImpl; @@ -888,4 +893,86 @@ public class CMISTest } } + @Test + public void testMNT9090() throws Exception + { + AuthenticationUtil.pushAuthentication(); + AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getAdminUserName()); + + CmisService cmisService = factory.getService(context); + + try + { + FileInfo fileInfo = transactionService.getRetryingTransactionHelper().doInTransaction(new RetryingTransactionCallback() + { + @Override + public FileInfo execute() throws Throwable + { + NodeRef companyHomeNodeRef = repositoryHelper.getCompanyHome(); + + String folderName = GUID.generate(); + FileInfo folderInfo = fileFolderService.create(companyHomeNodeRef, folderName, ContentModel.TYPE_FOLDER); + nodeService.setProperty(folderInfo.getNodeRef(), ContentModel.PROP_NAME, folderName); + assertNotNull(folderInfo); + + String docName = GUID.generate(); + FileInfo fileInfo = fileFolderService.create(folderInfo.getNodeRef(), docName, ContentModel.TYPE_CONTENT); + assertNotNull(fileInfo); + nodeService.setProperty(fileInfo.getNodeRef(), ContentModel.PROP_NAME, docName); + + QName ASPECT_AUDIO = QName.createQName(NamespaceService.AUDIO_MODEL_1_0_URI, "audio"); + Map aspectProperties = new HashMap(); + nodeService.addAspect(fileInfo.getNodeRef(), ASPECT_AUDIO, aspectProperties); + + return fileInfo; + } + }); + + // get repository id + List repositories = cmisService.getRepositoryInfos(null); + assertTrue(repositories.size() > 0); + RepositoryInfo repo = repositories.get(0); + String repositoryId = repo.getId(); + + String objectIdStr = fileInfo.getNodeRef().toString(); + Holder objectId = new Holder(objectIdStr); + + // try to overflow the value + Object value = BigInteger.valueOf(Integer.MAX_VALUE + 1l); + + Properties properties = new PropertiesImpl(); + List extensions = new ArrayList(); + + CmisExtensionElement valueElem = new CmisExtensionElementImpl(CMISConnector.ALFRESCO_EXTENSION_NAMESPACE, "value", null, value.toString()); + List valueElems = new ArrayList(); + valueElems.add(valueElem); + + List children = new ArrayList(); + Map attributes = new HashMap(); + attributes.put("propertyDefinitionId", "audio:trackNumber"); + children.add(new CmisExtensionElementImpl(CMISConnector.ALFRESCO_EXTENSION_NAMESPACE, "propertyInteger", attributes, valueElems)); + + List propertyValuesExtension = new ArrayList(); + propertyValuesExtension.add(new CmisExtensionElementImpl(CMISConnector.ALFRESCO_EXTENSION_NAMESPACE, CMISConnector.PROPERTIES, null, children)); + + CmisExtensionElement setAspectsExtension = new CmisExtensionElementImpl(CMISConnector.ALFRESCO_EXTENSION_NAMESPACE, CMISConnector.SET_ASPECTS, null, propertyValuesExtension); + extensions.add(setAspectsExtension); + properties.setExtensions(extensions); + + // should throw a CMISConstraintException + cmisService.updateProperties(repositoryId, objectId, null, properties, null); + fail(); + } + catch(CmisConstraintException e) + { + assertTrue(e.getMessage().startsWith("Value is out of range for property")); + // ok + } + finally + { + cmisService.close(); + + AuthenticationUtil.popAuthentication(); + } + } }