HDFS-1575. Viewing block from web UI is broken. Contributed by Aaron T. Myers.
git-svn-id: https://svn.apache.org/repos/asf/hadoop/hdfs/trunk@1124575 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/CHANGES.txt b/CHANGES.txt
index 8d7231a..04aadb6 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1021,6 +1021,8 @@
HDFS-1925. SafeModeInfo should use the correct constant instead of a
hard-coded value for its default. (Joey Echeverria via todd)
+ HDFS-1575. Viewing block from web UI is broken. (Aaron T. Myers via todd)
+
Release 0.21.1 - Unreleased
HDFS-1466. TestFcHdfsSymlink relies on /tmp/test not existing. (eli)
diff --git a/src/java/org/apache/hadoop/hdfs/server/common/JspHelper.java b/src/java/org/apache/hadoop/hdfs/server/common/JspHelper.java
index a41789f..704b4d7 100644
--- a/src/java/org/apache/hadoop/hdfs/server/common/JspHelper.java
+++ b/src/java/org/apache/hadoop/hdfs/server/common/JspHelper.java
@@ -586,12 +586,39 @@
}
/**
- * Returns the url parameter for the given bpid string.
+ * Returns the url parameter for the given string, prefixed with
+ * paramSeparator.
+ *
+ * @param name parameter name
+ * @param val parameter value
+ * @param paramSeparator URL parameter prefix, i.e. either '?' or '&'
+ * @return url parameter
+ */
+ public static String getUrlParam(String name, String val, String paramSeparator) {
+ return val == null ? "" : paramSeparator + name + "=" + val;
+ }
+
+ /**
+ * Returns the url parameter for the given string, prefixed with '?' if
+ * firstParam is true, prefixed with '&' if firstParam is false.
+ *
+ * @param name parameter name
+ * @param val parameter value
+ * @param firstParam true if this is the first parameter in the list, false otherwise
+ * @return url parameter
+ */
+ public static String getUrlParam(String name, String val, boolean firstParam) {
+ return getUrlParam(name, val, firstParam ? "?" : "&");
+ }
+
+ /**
+ * Returns the url parameter for the given string, prefixed with '&'.
+ *
* @param name parameter name
* @param val parameter value
* @return url parameter
*/
public static String getUrlParam(String name, String val) {
- return val == null ? "" : "&" + name + "=" + val;
+ return getUrlParam(name, val, false);
}
}
diff --git a/src/java/org/apache/hadoop/hdfs/server/datanode/DatanodeJspHelper.java b/src/java/org/apache/hadoop/hdfs/server/datanode/DatanodeJspHelper.java
index af1f907..15ac0f7 100644
--- a/src/java/org/apache/hadoop/hdfs/server/datanode/DatanodeJspHelper.java
+++ b/src/java/org/apache/hadoop/hdfs/server/datanode/DatanodeJspHelper.java
@@ -31,6 +31,7 @@
import javax.servlet.http.HttpServletResponse;
import javax.servlet.jsp.JspWriter;
+import org.apache.commons.lang.StringEscapeUtils;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hdfs.DFSClient;
@@ -47,6 +48,7 @@
import org.apache.hadoop.security.UserGroupInformation;
import org.apache.hadoop.security.token.Token;
import org.apache.hadoop.util.StringUtils;
+import org.mortbay.util.URIUtil;
@InterfaceAudience.Private
public class DatanodeJspHelper {
@@ -82,7 +84,8 @@
Configuration conf
) throws IOException,
InterruptedException {
- final String dir = JspHelper.validatePath(req.getParameter("dir"));
+ final String dir = StringEscapeUtils.unescapeHtml(
+ JspHelper.validatePath(req.getParameter("dir")));
if (dir == null) {
out.print("Invalid input");
return;
@@ -257,9 +260,11 @@
else
startOffset = Long.parseLong(startOffsetStr);
- final String filename=JspHelper.validatePath(
- req.getPathInfo() == null ?
- "/" : req.getPathInfo());
+ String path = StringEscapeUtils.unescapeHtml(req.getParameter("filename"));
+ if (path == null) {
+ path = req.getPathInfo() == null ? "/" : req.getPathInfo();
+ }
+ final String filename = JspHelper.validatePath(path);
if (filename == null) {
out.print("Invalid input");
return;
@@ -279,10 +284,9 @@
// Add the various links for looking at the file contents
// URL for downloading the full file
String downloadUrl = "http://" + req.getServerName() + ":"
- + req.getServerPort() + "/streamFile?" + "filename="
- + URLEncoder.encode(filename, "UTF-8")
- + JspHelper.getDelegationTokenUrlParam(tokenString)
- + JspHelper.getUrlParam(JspHelper.NAMENODE_ADDRESS, nnAddr);
+ + req.getServerPort() + "/streamFile" + URIUtil.encodePath(filename)
+ + JspHelper.getUrlParam(JspHelper.NAMENODE_ADDRESS, nnAddr, true)
+ + JspHelper.getDelegationTokenUrlParam(tokenString);
out.print("<a name=\"viewOptions\"></a>");
out.print("<a href=\"" + downloadUrl + "\">Download this file</a><br>");
@@ -396,7 +400,7 @@
namenodeInfoPort = Integer.parseInt(namenodeInfoPortStr);
final String filename = JspHelper
- .validatePath(req.getParameter("filename"));
+ .validatePath(StringEscapeUtils.unescapeHtml(req.getParameter("filename")));
if (filename == null) {
out.print("Invalid input (filename absent)");
return;
@@ -413,22 +417,25 @@
String bpid = null;
Token<BlockTokenIdentifier> blockToken = BlockTokenSecretManager.DUMMY_TOKEN;
- if (conf.getBoolean(
- DFSConfigKeys.DFS_BLOCK_ACCESS_TOKEN_ENABLE_KEY,
- DFSConfigKeys.DFS_BLOCK_ACCESS_TOKEN_ENABLE_DEFAULT)) {
- List<LocatedBlock> blks = dfs.getNamenode().getBlockLocations(filename, 0,
- Long.MAX_VALUE).getLocatedBlocks();
- if (blks == null || blks.size() == 0) {
- out.print("Can't locate file blocks");
- dfs.close();
- return;
- }
- for (int i = 0; i < blks.size(); i++) {
- if (blks.get(i).getBlock().getBlockId() == blockId) {
- bpid = blks.get(i).getBlock().getBlockPoolId();
+ List<LocatedBlock> blks = dfs.getNamenode().getBlockLocations(filename, 0,
+ Long.MAX_VALUE).getLocatedBlocks();
+ if (blks == null || blks.size() == 0) {
+ out.print("Can't locate file blocks");
+ dfs.close();
+ return;
+ }
+
+ boolean needBlockToken = conf.getBoolean(
+ DFSConfigKeys.DFS_BLOCK_ACCESS_TOKEN_ENABLE_KEY,
+ DFSConfigKeys.DFS_BLOCK_ACCESS_TOKEN_ENABLE_DEFAULT);
+
+ for (int i = 0; i < blks.size(); i++) {
+ if (blks.get(i).getBlock().getBlockId() == blockId) {
+ bpid = blks.get(i).getBlock().getBlockPoolId();
+ if (needBlockToken) {
blockToken = blks.get(i).getBlockToken();
- break;
}
+ break;
}
}
@@ -614,7 +621,7 @@
}
final String filename = JspHelper
- .validatePath(req.getParameter("filename"));
+ .validatePath(req.getParameter(StringEscapeUtils.unescapeHtml("filename")));
if (filename == null) {
out.print("Invalid input (file name absent)");
return;
diff --git a/src/test/hdfs/org/apache/hadoop/hdfs/server/datanode/TestDatanodeJsp.java b/src/test/hdfs/org/apache/hadoop/hdfs/server/datanode/TestDatanodeJsp.java
new file mode 100644
index 0000000..8926d90
--- /dev/null
+++ b/src/test/hdfs/org/apache/hadoop/hdfs/server/datanode/TestDatanodeJsp.java
@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.apache.hadoop.hdfs.server.datanode;
+
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.net.URL;
+import java.net.URLEncoder;
+
+import org.apache.commons.httpclient.util.URIUtil;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hdfs.DFSTestUtil;
+import org.apache.hadoop.hdfs.HdfsConfiguration;
+import org.apache.hadoop.hdfs.MiniDFSCluster;
+import org.apache.hadoop.hdfs.server.common.JspHelper;
+import org.junit.Test;
+
+public class TestDatanodeJsp {
+
+ private static final String FILE_DATA = "foo bar baz biz buz";
+
+ private static void testViewingFile(MiniDFSCluster cluster, String filePath) throws IOException {
+ FileSystem fs = cluster.getFileSystem();
+
+ Path testPath = new Path(filePath);
+ DFSTestUtil.writeFile(fs, testPath, FILE_DATA);
+
+ InetSocketAddress nnIpcAddress = cluster.getNameNode().getNameNodeAddress();
+ InetSocketAddress nnHttpAddress = cluster.getNameNode().getHttpAddress();
+ int dnInfoPort = cluster.getDataNodes().get(0).getInfoPort();
+
+ URL url = new URL("http://localhost:" + dnInfoPort + "/browseDirectory.jsp" +
+ JspHelper.getUrlParam("dir", URLEncoder.encode(testPath.toString(), "UTF-8"), true) +
+ JspHelper.getUrlParam("namenodeInfoPort", nnHttpAddress.getPort() +
+ JspHelper.getUrlParam("nnaddr", "localhost:" + nnIpcAddress.getPort())));
+
+ String viewFilePage = DFSTestUtil.urlGet(url);
+
+ assertTrue("page should show preview of file contents", viewFilePage.contains(FILE_DATA));
+ assertTrue("page should show link to download file", viewFilePage
+ .contains("/streamFile" + URIUtil.encodePath(testPath.toString()) +
+ "?nnaddr=localhost:" + nnIpcAddress.getPort()));
+ }
+
+ @Test
+ public void testViewFileJsp() throws IOException {
+ MiniDFSCluster cluster = null;
+ try {
+ Configuration conf = new HdfsConfiguration();
+ cluster = new MiniDFSCluster.Builder(conf).build();
+ cluster.waitActive();
+
+ testViewingFile(cluster, "/test-file");
+ testViewingFile(cluster, "/tmp/test-file");
+ testViewingFile(cluster, "/tmp/test-file%with goofy&characters");
+
+ } finally {
+ if (cluster != null) {
+ cluster.shutdown();
+ }
+ }
+ }
+
+}
