HDFS-4162. Some malformed and unquoted HTML strings are returned from datanode web ui. Contributed by Darek Dagit.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1407556 13f79535-47bb-0310-9956-ffa450edef68

hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt

@@ -574,6 +574,9 @@ Release 2.0.3-alpha - Unreleased
 
     HDFS-3810. Implement format() for BKJM (Ivan Kelly via umamahesh)
 
+    HDFS-4162. Some malformed and unquoted HTML strings are returned from 
+    datanode web ui. (Darek Dagit via suresh)
+
 Release 2.0.2-alpha - 2012-09-07 
 
   INCOMPATIBLE CHANGES

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/common/JspHelper.java

@@ -408,15 +408,15 @@ public class JspHelper {
         if (!parts[i].equals("")) {
           tempPath.append(parts[i]);
           out.print("<a href=\"browseDirectory.jsp" + "?dir="
-              + tempPath.toString() + "&namenodeInfoPort=" + namenodeInfoPort
+              + HtmlQuoting.quoteHtmlChars(tempPath.toString()) + "&namenodeInfoPort=" + namenodeInfoPort
               + getDelegationTokenUrlParam(tokenString)
               + getUrlParam(NAMENODE_ADDRESS, nnAddress));
-          out.print("\">" + parts[i] + "</a>" + Path.SEPARATOR);
+          out.print("\">" + HtmlQuoting.quoteHtmlChars(parts[i]) + "</a>" + Path.SEPARATOR);
           tempPath.append(Path.SEPARATOR);
         }
       }
       if(parts.length > 0) {
-        out.print(parts[parts.length-1]);
+        out.print(HtmlQuoting.quoteHtmlChars(parts[parts.length-1]));
       }
     }
     catch (UnsupportedEncodingException ex) {
@@ -431,16 +431,16 @@ public class JspHelper {
                                    String nnAddress) throws IOException {
     out.print("<form action=\"browseDirectory.jsp\" method=\"get\" name=\"goto\">");
     out.print("Goto : ");
-    out.print("<input name=\"dir\" type=\"text\" width=\"50\" id\"dir\" value=\""+ file+"\">");
-    out.print("<input name=\"go\" type=\"submit\" value=\"go\">");
+    out.print("<input name=\"dir\" type=\"text\" width=\"50\" id=\"dir\" value=\""+ HtmlQuoting.quoteHtmlChars(file)+"\"/>");
+    out.print("<input name=\"go\" type=\"submit\" value=\"go\"/>");
     out.print("<input name=\"namenodeInfoPort\" type=\"hidden\" "
-        + "value=\"" + namenodeInfoPort  + "\">");
+        + "value=\"" + namenodeInfoPort  + "\"/>");
     if (UserGroupInformation.isSecurityEnabled()) {
       out.print("<input name=\"" + DELEGATION_PARAMETER_NAME
-          + "\" type=\"hidden\" value=\"" + tokenString + "\">");
+          + "\" type=\"hidden\" value=\"" + tokenString + "\"/>");
     }
     out.print("<input name=\""+ NAMENODE_ADDRESS +"\" type=\"hidden\" "
-        + "value=\"" + nnAddress  + "\">");
+        + "value=\"" + nnAddress  + "\"/>");
     out.print("</form>");
   }
   

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DatanodeJspHelper.java

@@ -43,6 +43,7 @@ import org.apache.hadoop.hdfs.protocol.LocatedBlock;
 import org.apache.hadoop.hdfs.security.token.block.BlockTokenIdentifier;
 import org.apache.hadoop.hdfs.security.token.block.BlockTokenSecretManager;
 import org.apache.hadoop.hdfs.server.common.JspHelper;
+import org.apache.hadoop.http.HtmlQuoting;
 import org.apache.hadoop.http.HttpConfig;
 import org.apache.hadoop.net.NetUtils;
 import org.apache.hadoop.security.UserGroupInformation;
@@ -119,7 +120,7 @@ public class DatanodeJspHelper {
     String target = dir;
     final HdfsFileStatus targetStatus = dfs.getFileInfo(target);
     if (targetStatus == null) { // not exists
-      out.print("<h3>File or directory : " + target + " does not exist</h3>");
+      out.print("<h3>File or directory : " + StringEscapeUtils.escapeHtml(target) + " does not exist</h3>");
       JspHelper.printGotoForm(out, namenodeInfoPort, tokenString, target,
           nnAddr);
     } else {
@@ -203,7 +204,7 @@ public class DatanodeJspHelper {
               + JspHelper.getDelegationTokenUrlParam(tokenString)
               + JspHelper.getUrlParam(JspHelper.NAMENODE_ADDRESS, nnAddr);
             cols[0] = "<a href=\"" + datanodeUrl + "\">"
-              + localFileName + "</a>";
+              + HtmlQuoting.quoteHtmlChars(localFileName) + "</a>";
             cols[5] = lsDateFormat.format(new Date((files[i]
               .getModificationTime())));
             cols[6] = files[i].getPermission().toString();

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/common/TestJspHelper.java

@@ -19,13 +19,20 @@ package org.apache.hadoop.hdfs.server.common;
 
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.doAnswer;
 
 import java.io.IOException;
+import java.io.StringReader;
 import java.net.InetSocketAddress;
 import java.util.ArrayList;
 
 import javax.servlet.ServletContext;
 import javax.servlet.http.HttpServletRequest;
+import javax.servlet.jsp.JspWriter;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+
 
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hdfs.DFSConfigKeys;
@@ -46,10 +53,17 @@ import org.apache.hadoop.security.token.TokenIdentifier;
 import org.apache.hadoop.security.token.delegation.AbstractDelegationTokenSecretManager;
 import org.junit.Assert;
 import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+import org.xml.sax.InputSource;
+import org.xml.sax.SAXException;
+
 
 public class TestJspHelper {
 
   private Configuration conf = new HdfsConfiguration();
+  private String jspWriterOutput = "";
 
   public static class DummySecretManager extends
       AbstractDelegationTokenSecretManager<DelegationTokenIdentifier> {
@@ -368,7 +382,33 @@ public class TestJspHelper {
            ae.getMessage());
     }
   }
-  
+
+  @Test
+  public void testPrintGotoFormWritesValidXML() throws IOException,
+         ParserConfigurationException, SAXException {
+    JspWriter mockJspWriter = mock(JspWriter.class);
+    ArgumentCaptor<String> arg = ArgumentCaptor.forClass(String.class);
+    doAnswer(new Answer<Object>() {
+      @Override
+      public Object answer(InvocationOnMock invok) {
+        Object[] args = invok.getArguments();
+        jspWriterOutput += (String) args[0];
+        return null;
+      }
+    }).when(mockJspWriter).print(arg.capture());
+
+    jspWriterOutput = "";
+
+    JspHelper.printGotoForm(mockJspWriter, 424242, "a token string",
+            "foobar/file", "0.0.0.0");
+
+    DocumentBuilder parser =
+        DocumentBuilderFactory.newInstance().newDocumentBuilder();
+    InputSource is = new InputSource();
+    is.setCharacterStream(new StringReader(jspWriterOutput));
+    parser.parse(is);
+  }
+
   private HttpServletRequest getMockRequest(String remoteUser, String user, String doAs) {
     HttpServletRequest request = mock(HttpServletRequest.class);
     when(request.getParameter(UserParam.NAME)).thenReturn(user);