Closes OOZIE-100 escape characters for xml when create dag evaluator

core/src/main/java/org/apache/oozie/DagELFunctions.java

@@ -23,6 +23,7 @@
 import org.apache.oozie.util.PropertiesUtils;
 import org.apache.oozie.util.XConfiguration;
 import org.apache.oozie.util.ParamChecker;
+import org.apache.oozie.util.XmlUtils;
 
 import java.io.IOException;
 import java.io.StringReader;
@@ -52,7 +53,10 @@ public static void configureEvaluator(ELEvaluator evaluator, WorkflowJobBean wor
         evaluator.setVariable(ACTION, action);
         for (Map.Entry<String, String> entry : workflow.getWorkflowInstance().getConf()) {
             if (ParamChecker.isValidIdentifier(entry.getKey())) {
-                evaluator.setVariable(entry.getKey().trim(), entry.getValue().trim());
+                String value = entry.getValue().trim();
+                // escape the characters for xml
+                value = XmlUtils.escapeCharsForXML(value);
+                evaluator.setVariable(entry.getKey().trim(), value);
             }
         }
         try {

core/src/main/java/org/apache/oozie/coord/CoordELEvaluator.java

@@ -263,7 +263,7 @@ private static SyncCoordDataset getDSObject(Element eData) throws Exception {
      */
     private static void setConfigToEval(ELEvaluator eval, Configuration conf) {
         for (Map.Entry<String, String> entry : conf) {
-            eval.setVariable(entry.getKey(), entry.getValue());
+            eval.setVariable(entry.getKey(), entry.getValue().trim());
         }
     }
 

core/src/main/java/org/apache/oozie/util/XmlUtils.java

@@ -19,6 +19,8 @@
 import java.io.InputStream;
 import java.io.StringReader;
 import java.io.StringWriter;
+import java.text.CharacterIterator;
+import java.text.StringCharacterIterator;
 import java.util.Enumeration;
 import java.util.Iterator;
 import java.util.List;
@@ -353,4 +355,46 @@ public static String writePropToString(Properties props) throws IOException {
         }
     }
 
+    /**
+     * Escape characters for text appearing as XML data, between tags.
+     * <P/>
+     * The following characters are replaced with corresponding character entities :
+     * '<' to '&lt';
+     * '>' to '&gt';
+     * '&' to '&amp;'
+     * '"' to '&quot;'
+     * "'" to "&#039;"
+     * <P/>
+     * Note that JSTL's {@code <c:out>} escapes the exact same set of characters as this method.
+     */
+    public static String escapeCharsForXML(String aText) {
+        final StringBuilder result = new StringBuilder();
+        final StringCharacterIterator iterator = new StringCharacterIterator(aText);
+        char character = iterator.current();
+        while (character != CharacterIterator.DONE) {
+            if (character == '<') {
+                result.append("&lt;");
+            }
+            else if (character == '>') {
+                result.append("&gt;");
+            }
+            else if (character == '\"') {
+                result.append("&quot;");
+            }
+            else if (character == '\'') {
+                result.append("&#039;");
+            }
+            else if (character == '&') {
+                result.append("&amp;");
+            }
+            else {
+                // the char is not a special one
+                // add it to the result as is
+                result.append(character);
+            }
+            character = iterator.next();
+        }
+        return result.toString();
+    }
+
 }

core/src/test/java/org/apache/oozie/command/coord/TestCoordActionStartXCommand.java

@@ -15,22 +15,57 @@
 package org.apache.oozie.command.coord;
 
 import java.io.File;
+import java.io.FileInputStream;
 import java.io.FileWriter;
 import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
+import java.io.Reader;
+import java.io.Writer;
 import java.util.Date;
+import java.util.Properties;
 
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
 import org.apache.oozie.CoordinatorActionBean;
+import org.apache.oozie.CoordinatorJobBean;
 import org.apache.oozie.WorkflowJobBean;
+import org.apache.oozie.action.hadoop.MapperReducerForTest;
 import org.apache.oozie.client.CoordinatorAction;
+import org.apache.oozie.client.CoordinatorJob;
+import org.apache.oozie.client.OozieClient;
 import org.apache.oozie.client.CoordinatorAction.Status;
 import org.apache.oozie.command.CommandException;
 import org.apache.oozie.executor.jpa.CoordActionGetJPAExecutor;
 import org.apache.oozie.executor.jpa.CoordActionInsertJPAExecutor;
 import org.apache.oozie.executor.jpa.JPAExecutorException;
 import org.apache.oozie.executor.jpa.WorkflowJobGetJPAExecutor;
+import org.apache.oozie.service.JPAService;
+import org.apache.oozie.service.Services;
+import org.apache.oozie.test.XDataTestCase;
+import org.apache.oozie.util.DateUtils;
+import org.apache.oozie.util.IOUtils;
+import org.apache.oozie.util.XConfiguration;
 
-public class TestCoordActionStartXCommand extends CoordXTestCase {
+public class TestCoordActionStartXCommand extends XDataTestCase {
+    private Services services;
+
+    @Override
+    protected void setUp() throws Exception {
+        super.setUp();
+        services = new Services();
+        services.init();
+        cleanUpDBTables();
+    }
+
+    @Override
+    protected void tearDown() throws Exception {
+        services.destroy();
+        super.tearDown();
+    }
 
     public void testActionStartCommand() throws IOException, JPAExecutorException, CommandException {
         String actionId = new Date().getTime() + "-COORD-ActionStartCommand-C@1";
@@ -39,15 +74,99 @@ public void testActionStartCommand() throws IOException, JPAExecutorException, C
         checkCoordAction(actionId);
     }
 
+    /**
+     * Test : configuration contains url string which should be escaped before put into the evaluator.
+     * If not escape, the error 'SAXParseException' will be thrown and workflow job will not be submitted.
+     *
+     * @throws Exception
+     */
+    public void testActionStartWithEscapeStrings() throws Exception {
+        Date start = DateUtils.parseDateUTC("2009-12-15T01:00Z");
+        Date end = DateUtils.parseDateUTC("2009-12-16T01:00Z");
+        CoordinatorJobBean coordJob = addRecordToCoordJobTable(CoordinatorJob.Status.RUNNING, start, end, false,
+                false, 1);
+
+        CoordinatorActionBean action = addRecordToCoordActionTable(coordJob.getId(), 1,
+                CoordinatorAction.Status.SUBMITTED, "coord-action-start-escape-strings.xml", 0);
+
+        String actionId = action.getId();
+        new CoordActionStartXCommand(actionId, getTestUser(), "undef").call();
+
+        final JPAService jpaService = Services.get().get(JPAService.class);
+        action = jpaService.execute(new CoordActionGetJPAExecutor(actionId));
+
+        if (action.getStatus() == CoordinatorAction.Status.SUBMITTED) {
+            fail("CoordActionStartCommand didn't work because the status for action id" + actionId + " is :"
+                    + action.getStatus() + " expected to be NOT SUBMITTED (i.e. RUNNING)");
+        }
+
+        final String wfId = action.getExternalId();
+        waitFor(20 * 1000, new Predicate() {
+            public boolean evaluate() throws Exception {
+                WorkflowJobBean wfJob = jpaService.execute(new WorkflowJobGetJPAExecutor(wfId));
+                return wfJob.getExternalId() != null;
+            }
+        });
+        WorkflowJobBean wfJob = jpaService.execute(new WorkflowJobGetJPAExecutor(wfId));
+        assertNotNull(wfJob.getExternalId());
+    }
+
+    @Override
+    protected Configuration getCoordConf(Path coordAppPath) throws IOException {
+        Path wfAppPath = new Path(getFsTestCaseDir(), "app");
+        FileSystem fs = getFileSystem();
+        fs.mkdirs(new Path(wfAppPath, "lib"));
+        File jarFile = IOUtils.createJar(new File(getTestCaseDir()), "test.jar", MapperReducerForTest.class);
+        InputStream is = new FileInputStream(jarFile);
+        OutputStream os = fs.create(new Path(wfAppPath, "lib/test.jar"));
+        IOUtils.copyStream(is, os);
+        Path input = new Path(wfAppPath, "input");
+        fs.mkdirs(input);
+        Writer writer = new OutputStreamWriter(fs.create(new Path(input, "test.txt")));
+        writer.write("hello");
+        writer.close();
+
+        final String APP1 = "<workflow-app xmlns='uri:oozie:workflow:0.1' name='app'>" +
+                "<start to='end'/>" +
+                "<end name='end'/>" +
+                "</workflow-app>";
+        String subWorkflowAppPath = new Path(wfAppPath, "subwf").toString();
+        fs.mkdirs(new Path(wfAppPath, "subwf"));
+        Writer writer2 = new OutputStreamWriter(fs.create(new Path(subWorkflowAppPath, "workflow.xml")));
+        writer2.write(APP1);
+        writer2.close();
+
+        Reader reader = IOUtils.getResourceAsReader("wf-url-template.xml", -1);
+        Writer writer1 = new OutputStreamWriter(fs.create(new Path(wfAppPath + "/workflow.xml")));
+        IOUtils.copyCharStream(reader, writer1);
+
+        Properties jobConf = new Properties();
+        jobConf.setProperty(OozieClient.COORDINATOR_APP_PATH, coordAppPath.toString());
+        jobConf.setProperty(OozieClient.USER_NAME, getTestUser());
+        jobConf.setProperty(OozieClient.GROUP_NAME, getTestGroup());
+        jobConf.setProperty("myJobTracker", getJobTrackerUri());
+        jobConf.setProperty("myNameNode", getNameNodeUri());
+        jobConf.setProperty("wfAppPath", wfAppPath.toString()+ File.separator + "workflow.xml");
+        jobConf.setProperty("mrclass", MapperReducerForTest.class.getName());
+        jobConf.setProperty("delPath", wfAppPath.toString() + "/output");
+        jobConf.setProperty("subWfApp", wfAppPath.toString() + "/subwf/workflow.xml");
+        injectKerberosInfo(jobConf);
+
+        return new XConfiguration(jobConf);
+    }
+
     private void addRecordToActionTable(String actionId, int actionNum) throws IOException, JPAExecutorException {
+        final JPAService jpaService = Services.get().get(JPAService.class);
         CoordinatorActionBean action = new CoordinatorActionBean();
         action.setJobId(actionId);
         action.setId(actionId);
         action.setActionNumber(actionNum);
         action.setNominalTime(new Date());
         action.setStatus(Status.SUBMITTED);
         String appPath = "/tmp/coord/no-op/";
-        String actionXml = "<coordinator-app xmlns='uri:oozie:coordinator:0.2' xmlns:sla='uri:oozie:sla:0.1' name='NAME' frequency=\"1\" start='2009-02-01T01:00Z' end='2009-02-03T23:59Z' timezone='UTC' freq_timeunit='DAY' end_of_duration='NONE'  instance-number=\"1\" action-nominal-time=\"2009-02-01T01:00Z\">";
+        String actionXml = "<coordinator-app xmlns='uri:oozie:coordinator:0.2' xmlns:sla='uri:oozie:sla:0.1' name='NAME' " +
+        		"frequency=\"1\" start='2009-02-01T01:00Z' end='2009-02-03T23:59Z' timezone='UTC' freq_timeunit='DAY' " +
+        		"end_of_duration='NONE'  instance-number=\"1\" action-nominal-time=\"2009-02-01T01:00Z\">";
         actionXml += "<controls>";
         actionXml += "<timeout>10</timeout>";
         actionXml += "<concurrency>2</concurrency>";
@@ -125,11 +244,11 @@ private void addRecordToActionTable(String actionId, int actionNum) throws IOExc
                 + "</sla:info>";
         content += "<end name='end' />" + slaXml2 + "</workflow-app>";
         writeToFile(content, appPath);
-        // System.out.println("COMMITED TRX");
     }
 
     private void checkCoordAction(String actionId) {
         try {
+            final JPAService jpaService = Services.get().get(JPAService.class);
             CoordinatorActionBean action = jpaService.execute(new CoordActionGetJPAExecutor(actionId));
             if (action.getStatus() == CoordinatorAction.Status.SUBMITTED) {
                 fail("CoordActionStartCommand didn't work because the status for action id" + actionId + " is :"

core/src/test/resources/coord-action-start-escape-strings.xml

@@ -0,0 +1,64 @@
+<!--
+  Copyright (c) 2010 Yahoo! Inc. All rights reserved.
+  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. See accompanying LICENSE file.
+-->
+<coordinator-app xmlns="uri:oozie:coordinator:0.2" name="COORD-TEST" frequency="1" timezone="UTC" freq_timeunit="DAY" end_of_duration="NONE" instance-number="1" action-nominal-time="2009-12-15T01:00Z" action-actual-time="2010-10-01T00:00Z">
+  <controls>
+    <timeout>10</timeout>
+    <concurrency>1</concurrency>
+    <execution>FIFO</execution>
+  </controls>
+  <input-events>
+    <data-in name="din" dataset="din">
+      <uris>#inputDir</uris>
+      <dataset name="din" frequency="1" initial-instance="2009-12-01T01:00Z" timezone="UTC" freq_timeunit="DAY" end_of_duration="NONE">
+        <uri-template>#inputTemplate</uri-template>
+      </dataset>
+    </data-in>
+  </input-events>
+  <output-events>
+    <data-out name="dout" dataset="dout">
+      <uris>#outputDir</uris>
+      <dataset name="dout" frequency="1380" initial-instance="2009-12-01T01:00Z" timezone="UTC" freq_timeunit="MINUTE" end_of_duration="NONE">
+        <uri-template>#outputTemplate</uri-template>
+      </dataset>
+    </data-out>
+  </output-events>
+  <action>
+    <workflow>
+      <app-path>${wfAppPath}</app-path>
+      <configuration>
+        <property>
+          <name>jobTracker</name>
+          <value>${myJobTracker}</value>
+        </property>
+        <property>
+          <name>nameNode</name>
+          <value>${myNameNode}</value>
+        </property>
+        <property>
+          <name>input</name>
+          <value>#inputDir</value>
+        </property>
+        <property>
+          <name>output</name>
+          <value>#outputDir</value>
+        </property>
+        <property>
+		  <name>urlTemplate</name>
+		  <value>http://yahoo.com/test?custid1=yahoo&amp;custid2=us&amp;offset=0&amp;hits=10</value>
+		</property>
+      </configuration>
+    </workflow>
+  </action>
+</coordinator-app>

core/src/test/resources/wf-url-template.xml

@@ -0,0 +1,76 @@
+<!--
+  Copyright (c) 2010 Yahoo! Inc. All rights reserved.
+  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. See accompanying LICENSE file.
+-->
+<workflow-app xmlns="uri:oozie:workflow:0.1" name="recovery-el-wf">
+    <start to="hadoop"/>
+    <action name="hadoop">
+        <map-reduce>
+            <job-tracker>${jobTracker}</job-tracker>
+            <name-node>${nameNode}</name-node>
+            <prepare>
+                <delete path="${delPath}"/>
+            </prepare>
+            <configuration>
+                <property>
+                    <name>mapred.map.tasks</name>
+                    <value>1</value>
+                </property>
+                <property>
+                    <name>mapred.reduce.tasks</name>
+                    <value>0</value>
+                </property>
+                <property>
+                    <name>mapred.mapper.class</name>
+                    <value>${mrclass}</value>
+                </property>
+                <property>
+                    <name>mapred.reducer.class</name>
+                    <value>${mrclass}</value>
+                </property>
+                <property>
+                    <name>mapred.input.dir</name>
+                    <value>${input}</value>
+                </property>
+                <property>
+                    <name>mapred.output.dir</name>
+                    <value>${output}</value>
+                </property>
+                <property>
+                    <name>url.template</name>
+                    <value>${urlTemplate}</value>
+                </property>
+
+            </configuration>
+        </map-reduce>
+        <ok to="subwf"/>
+        <error to="k"/>
+    </action>
+    <action name="subwf">
+        <sub-workflow>
+            <app-path>${subWfApp}</app-path>
+            <configuration>
+                <property>
+                    <name>a</name>
+                    <value>A</value>
+                </property>
+            </configuration>
+        </sub-workflow>
+        <ok to="end"/>
+        <error to="k"/>
+    </action>
+    <kill name="k">
+        <message>kill</message>
+    </kill>
+    <end name="end"/>
+</workflow-app>