Make sure that empty CDATA is properly parsed

There is a bug in the default Java XML Stream Reader in which an empty CDATA throws an IndexOutOfBoundsException when the schema is being validated.
Therefore, we are creating our own delegating FlowableXMLStreamReader and handle the edge case for an empty CDATA.

Author: filiphr

modules/flowable-bpmn-converter/src/main/java/org/flowable/bpmn/converter/BpmnXMLConverter.java

@@ -282,7 +282,7 @@ public BpmnModel convertToBpmnModel(InputStreamProvider inputStreamProvider, boo
                 if (!enableSafeBpmnXml) {
                     validateModel(inputStreamProvider);
                 } else {
-                    validateModel(xif.createXMLStreamReader(in));
+                    validateModel(new FlowableXMLStreamReader(xif.createXMLStreamReader(in)));
                 }
             } catch (UnsupportedEncodingException e) {
                 throw new XMLException("The bpmn 2.0 xml is not properly encoded", e);

modules/flowable-bpmn-converter/src/main/java/org/flowable/bpmn/converter/FlowableXMLStreamReader.java

@@ -0,0 +1,41 @@
+/* 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.
+ */
+package org.flowable.bpmn.converter;
+
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
+import javax.xml.stream.util.StreamReaderDelegate;
+
+/**
+ * @author Filip Hrisafov
+ */
+public class FlowableXMLStreamReader extends StreamReaderDelegate {
+
+    public FlowableXMLStreamReader(XMLStreamReader reader) {
+        super(reader);
+    }
+
+    @Override
+    public int getTextCharacters(int sourceStart, char[] target, int targetStart, int length) throws XMLStreamException {
+        try {
+            return super.getTextCharacters(sourceStart, target, targetStart, length);
+        } catch (IndexOutOfBoundsException e) {
+            if (length == 0) {
+                // The default java stream reader has a bug where an empty CDATA will throw an IndexOutOfBoundsException
+                // When the length to copy is 0, then we should not copy anything and just return 0
+                return 0;
+            }
+            throw e;
+        }
+    }
+}

modules/flowable-cmmn-converter/src/main/java/org/flowable/cmmn/converter/CmmnXmlConverter.java

@@ -175,7 +175,7 @@ public CmmnModel convertToCmmnModel(InputStreamProvider inputStreamProvider, boo
                 if (!enableSafeBpmnXml) {
                     validateModel(inputStreamProvider);
                 } else {
-                    validateModel(xif.createXMLStreamReader(in));
+                    validateModel(new FlowableXMLStreamReader(xif.createXMLStreamReader(in)));
                 }
             } catch (UnsupportedEncodingException e) {
                 throw new CmmnXMLException("The CMMN 1.1 xml is not properly encoded", e);

modules/flowable-cmmn-converter/src/main/java/org/flowable/cmmn/converter/FlowableXMLStreamReader.java

@@ -0,0 +1,41 @@
+/* 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.
+ */
+package org.flowable.cmmn.converter;
+
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
+import javax.xml.stream.util.StreamReaderDelegate;
+
+/**
+ * @author Filip Hrisafov
+ */
+public class FlowableXMLStreamReader extends StreamReaderDelegate {
+
+    public FlowableXMLStreamReader(XMLStreamReader reader) {
+        super(reader);
+    }
+
+    @Override
+    public int getTextCharacters(int sourceStart, char[] target, int targetStart, int length) throws XMLStreamException {
+        try {
+            return super.getTextCharacters(sourceStart, target, targetStart, length);
+        } catch (IndexOutOfBoundsException e) {
+            if (length == 0) {
+                // The default java stream reader has a bug where an empty CDATA will throw an IndexOutOfBoundsException
+                // When the length to copy is 0, then we should not copy anything and just return 0
+                return 0;
+            }
+            throw e;
+        }
+    }
+}

modules/flowable-cmmn-engine/src/main/java/org/flowable/cmmn/engine/impl/deployer/CmmnDeployer.java

@@ -431,7 +431,8 @@ public String xmlEncoding() {
 
         @Override
         public boolean validateXml() {
-            return !cmmnEngineConfiguration.isDisableCmmnXmlValidation();
+            // On redeploy, we assume it is validated at the first deploy
+            return newDeployment && !cmmnEngineConfiguration.isDisableCmmnXmlValidation();
         }
 
         @Override

modules/flowable-cmmn-engine/src/test/java/org/flowable/cmmn/test/repository/DeploymentTest.java

@@ -244,4 +244,17 @@ public void deployingCaseModelWithWarningsShouldNotFail() {
         }
 
     }
+
+    @Test
+    public void deployingCaseModelWithEmptyCDATAShouldNotFail() {
+        org.flowable.cmmn.api.repository.CmmnDeployment cmmnDeployment = cmmnRepositoryService.createDeployment()
+                .addClasspathResource("org/flowable/cmmn/test/repository/DeploymentTest.testCaseDefinitionWithEmptyCDATA.cmmn")
+                .deploy();
+        autoCleanupDeploymentIds.add(cmmnDeployment.getId());
+        CaseDefinition caseDefinition = cmmnRepositoryService.createCaseDefinitionQuery()
+                .caseDefinitionKey("myCase")
+                .singleResult();
+        assertThat(caseDefinition).isNotNull();
+
+    }
 }

modules/flowable-cmmn-engine/src/test/resources/flowable.cmmn.cfg.xml

@@ -39,6 +39,7 @@
 
         <property name="enableEntityLinks" value="true" />
         <property name="enableCaseDefinitionHistoryLevel" value="true" />
+        <property name="enableSafeCmmnXml" value="true" />
 
         <property name="enableHistoricTaskLogging" value="true" />
 

modules/flowable-cmmn-engine/src/test/resources/org/flowable/cmmn/test/repository/DeploymentTest.testCaseDefinitionWithEmptyCDATA.cmmn

@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<definitions
+        xmlns="http://www.omg.org/spec/CMMN/20151109/MODEL"
+        xmlns:flowable="http://flowable.org/cmmn"
+        targetNamespace="http://flowable.org/cmmn">
+
+    <case id="myCase" name="My Invalid Case Model">
+        <casePlanModel id="myPlanModel" name="My CasePlanModel">
+            <planItem id="planItem1" name="Task 1" definitionRef="CasePageTask_1"/>
+            <task id="CasePageTask_1" name="Case page" flowable:type="casePage">
+                <extensionElements>
+                    <flowable:translation key="label" locale="it_it"><![CDATA[]]></flowable:translation>
+                </extensionElements>
+            </task>
+        </casePlanModel>
+    </case>
+
+</definitions>

modules/flowable-engine/src/test/java/org/flowable/engine/test/bpmn/deployment/BpmnDeploymentTest.java

@@ -337,4 +337,17 @@ public void testV5Deployment() {
                 .as("Expected deployment exception because v5 compatibility handler is not enabled");
     }
 
+    @Test
+    public void deployingModelWithEmptyCDATAShouldNotFail() {
+        org.flowable.engine.repository.Deployment deployment = repositoryService.createDeployment()
+                .addClasspathResource("org/flowable/engine/test/bpmn/deployment/BpmnDeploymentTest.testBpmnWithEmptyCDATA.bpmn20.xml")
+                .deploy();
+        deploymentIdsForAutoCleanup.add(deployment.getId());
+
+        ProcessDefinition definition = repositoryService.createProcessDefinitionQuery()
+                .processDefinitionKey("myProcess")
+                .singleResult();
+        assertThat(definition).isNotNull();
+    }
+
 }

modules/flowable-engine/src/test/resources/flowable.cfg.xml

@@ -70,6 +70,7 @@
     <property name="asyncHistoryEnabled" value="false" />
     <property name="enableEntityLinks" value="true" />
     <property name="enableProcessDefinitionHistoryLevel" value="true" />
+    <property name="enableSafeBpmnXml" value="true"/>
 
     <property name="enableProcessDefinitionInfoCache" value="true" />
 

modules/flowable-engine/src/test/resources/org/flowable/engine/test/bpmn/deployment/BpmnDeploymentTest.testBpmnWithEmptyCDATA.bpmn20.xml

@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<definitions id="definitions"
+  xmlns="http://www.omg.org/spec/BPMN/20100524/MODEL" 
+  xmlns:flowable="http://flowable.org/bpmn"
+  targetNamespace="Examples">
+
+  <process id="myProcess">
+
+    <startEvent id="start" />
+
+    <sequenceFlow id="flow1" sourceRef="start" targetRef="end" >
+      <extensionElements>
+        <flowable:translation key="label" locale="it_it"><![CDATA[]]></flowable:translation>
+      </extensionElements>
+    </sequenceFlow>
+
+    <endEvent id="end" />
+
+  </process>
+
+</definitions>