From 37807b38a380ec32950aec934b4b8328fdf1cdbf Mon Sep 17 00:00:00 2001 From: Sean Mackrory Date: Tue, 31 May 2016 10:28:27 -0600 Subject: [PATCH] HBASE-15946 Eliminate possible security concerns in RS web UI's store file metrics (Sean Mackrory) --- .../hbase/io/hfile/HFilePrettyPrinter.java | 111 +++++++++++------- .../hbase-webapps/regionserver/storeFile.jsp | 35 +++--- 2 files changed, 86 insertions(+), 60 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java index a4dce65b0d6..d43ebd6c902 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFilePrettyPrinter.java @@ -1,4 +1,3 @@ - /* * * Licensed to the Apache Software Foundation (ASF) under one @@ -97,6 +96,9 @@ public class HFilePrettyPrinter extends Configured implements Tool { private boolean checkFamily; private boolean isSeekToRow = false; + private PrintStream out = System.out; + private PrintStream err = System.err; + /** * The row which the user wants to specify and print all the KeyValues for. */ @@ -140,6 +142,11 @@ public class HFilePrettyPrinter extends Configured implements Tool { options.addOptionGroup(files); } + public void setPrintStreams(PrintStream out, PrintStream err) { + this.out = out; + this.err = err; + } + public boolean parseOptions(String args[]) throws ParseException, IOException { if (args.length == 0) { @@ -170,7 +177,7 @@ public class HFilePrettyPrinter extends Configured implements Tool { row = Bytes.toBytesBinary(key); isSeekToRow = true; } else { - System.err.println("Invalid row is specified."); + err.println("Invalid row is specified."); System.exit(-1); } } @@ -184,17 +191,17 @@ public class HFilePrettyPrinter extends Configured implements Tool { String enc = HRegionInfo.encodeRegionName(rn); Path regionDir = new Path(tableDir, enc); if (verbose) - System.out.println("region dir -> " + regionDir); + out.println("region dir -> " + regionDir); List regionFiles = HFile.getStoreFiles(FileSystem.get(getConf()), regionDir); if (verbose) - System.out.println("Number of region files found -> " + out.println("Number of region files found -> " + regionFiles.size()); if (verbose) { int i = 1; for (Path p : regionFiles) { if (verbose) - System.out.println("Found file[" + i++ + "] -> " + p); + out.println("Found file[" + i++ + "] -> " + p); } } files.addAll(regionFiles); @@ -227,27 +234,46 @@ public class HFilePrettyPrinter extends Configured implements Tool { // iterate over all files found for (Path fileName : files) { try { - processFile(fileName); + int exitCode = processFile(fileName); + if (exitCode != 0) { + return exitCode; + } } catch (IOException ex) { LOG.error("Error reading " + fileName, ex); - System.exit(-2); + return -2; } } if (verbose || printKey) { - System.out.println("Scanned kv count -> " + count); + out.println("Scanned kv count -> " + count); } return 0; } - private void processFile(Path file) throws IOException { + public int processFile(Path file) throws IOException { if (verbose) - System.out.println("Scanning -> " + file); + out.println("Scanning -> " + file); + + Path rootPath = FSUtils.getRootDir(getConf()); + String rootString = rootPath + rootPath.SEPARATOR; + if (!file.toString().startsWith(rootString)) { + // First we see if fully-qualified URI matches the root dir. It might + // also be an absolute path in the same filesystem, so we prepend the FS + // of the root dir and see if that fully-qualified URI matches. + FileSystem rootFS = rootPath.getFileSystem(getConf()); + String qualifiedFile = rootFS.getUri().toString() + file.toString(); + if (!qualifiedFile.startsWith(rootString)) { + err.println("ERROR, file (" + file + + ") is not in HBase's root directory (" + rootString + ")"); + return -2; + } + } + FileSystem fs = file.getFileSystem(getConf()); if (!fs.exists(file)) { - System.err.println("ERROR, file doesnt exist: " + file); - System.exit(-2); + err.println("ERROR, file doesnt exist: " + file); + return -2; } HFile.Reader reader = HFile.createReader(fs, file, new CacheConfig(getConf()), getConf()); @@ -278,12 +304,12 @@ public class HFilePrettyPrinter extends Configured implements Tool { } if (printBlockIndex) { - System.out.println("Block Index:"); - System.out.println(reader.getDataBlockIndexReader()); + out.println("Block Index:"); + out.println(reader.getDataBlockIndexReader()); } if (printBlockHeaders) { - System.out.println("Block Headers:"); + out.println("Block Headers:"); /* * TODO: this same/similar block iteration logic is used in HFileBlock#blockRange and * TestLazyDataBlockDecompression. Refactor? @@ -299,16 +325,17 @@ public class HFilePrettyPrinter extends Configured implements Tool { block = reader.readBlock(offset, -1, /* cacheBlock */ false, /* pread */ false, /* isCompaction */ false, /* updateCacheMetrics */ false, null, null); offset += block.getOnDiskSizeWithHeader(); - System.out.println(block); + out.println(block); } } if (printStats) { fileStats.finish(); - System.out.println("Stats:\n" + fileStats); + out.println("Stats:\n" + fileStats); } reader.close(); + return 0; } private void scanKeysValues(Path file, KeyValueStatsCollector fileStats, @@ -331,25 +358,25 @@ public class HFilePrettyPrinter extends Configured implements Tool { } // dump key value if (printKey) { - System.out.print("K: " + cell); + out.print("K: " + cell); if (printValue) { - System.out.print(" V: " + out.print(" V: " + Bytes.toStringBinary(cell.getValueArray(), cell.getValueOffset(), cell.getValueLength())); int i = 0; List tags = Tag.asList(cell.getTagsArray(), cell.getTagsOffset(), cell.getTagsLength()); for (Tag tag : tags) { - System.out.print(String.format(" T[%d]: %s", i++, + out.print(String.format(" T[%d]: %s", i++, Bytes.toStringBinary(tag.getBuffer(), tag.getTagOffset(), tag.getTagLength()))); } } - System.out.println(); + out.println(); } // check if rows are in order if (checkRow && pCell != null) { if (CellComparator.compareRows(pCell, cell) > 0) { - System.err.println("WARNING, previous row is greater then" + err.println("WARNING, previous row is greater then" + " current row\n\tfilename -> " + file + "\n\tprevious -> " + CellUtil.getCellKeyAsString(pCell) + "\n\tcurrent -> " + CellUtil.getCellKeyAsString(cell)); @@ -360,12 +387,12 @@ public class HFilePrettyPrinter extends Configured implements Tool { String fam = Bytes.toString(cell.getFamilyArray(), cell.getFamilyOffset(), cell.getFamilyLength()); if (!file.toString().contains(fam)) { - System.err.println("WARNING, filename does not match kv family," + err.println("WARNING, filename does not match kv family," + "\n\tfilename -> " + file + "\n\tkeyvalue -> " + CellUtil.getCellKeyAsString(cell)); } if (pCell != null && CellComparator.compareFamilies(pCell, cell) != 0) { - System.err.println("WARNING, previous kv has different family" + err.println("WARNING, previous kv has different family" + " compared to current key\n\tfilename -> " + file + "\n\tprevious -> " + CellUtil.getCellKeyAsString(pCell) + "\n\tcurrent -> " + CellUtil.getCellKeyAsString(cell)); @@ -387,33 +414,35 @@ public class HFilePrettyPrinter extends Configured implements Tool { private void printMeta(HFile.Reader reader, Map fileInfo) throws IOException { - System.out.println("Block index size as per heapsize: " + out.println("Block index size as per heapsize: " + reader.indexSize()); - System.out.println(asSeparateLines(reader.toString())); - System.out.println("Trailer:\n " + out.println(asSeparateLines(reader.toString())); + out.println("Trailer:\n " + asSeparateLines(reader.getTrailer().toString())); - System.out.println("Fileinfo:"); + out.println("Fileinfo:"); for (Map.Entry e : fileInfo.entrySet()) { - System.out.print(FOUR_SPACES + Bytes.toString(e.getKey()) + " = "); + out.print(FOUR_SPACES + Bytes.toString(e.getKey()) + " = "); if (Bytes.compareTo(e.getKey(), Bytes.toBytes("MAX_SEQ_ID_KEY")) == 0) { long seqid = Bytes.toLong(e.getValue()); - System.out.println(seqid); + out.println(seqid); } else if (Bytes.compareTo(e.getKey(), Bytes.toBytes("TIMERANGE")) == 0) { + TimeRangeTracker timeRangeTracker = new TimeRangeTracker(); Writables.copyWritable(e.getValue(), timeRangeTracker); - System.out.println(timeRangeTracker.getMin() + "...." + timeRangeTracker.getMax()); + out.println(timeRangeTracker.getMin() + "...." + timeRangeTracker.getMax()); } else if (Bytes.compareTo(e.getKey(), FileInfo.AVG_KEY_LEN) == 0 || Bytes.compareTo(e.getKey(), FileInfo.AVG_VALUE_LEN) == 0) { - System.out.println(Bytes.toInt(e.getValue())); + out.println(Bytes.toInt(e.getValue())); } else { - System.out.println(Bytes.toStringBinary(e.getValue())); + out.println(Bytes.toStringBinary(e.getValue())); } } try { - System.out.println("Mid-key: " + Bytes.toStringBinary(reader.midkey())); + + out.println("Mid-key: " + Bytes.toStringBinary(reader.midkey())); } catch (Exception e) { - System.out.println ("Unable to retrieve the midkey"); + out.println ("Unable to retrieve the midkey"); } // Printing general bloom information @@ -422,12 +451,12 @@ public class HFilePrettyPrinter extends Configured implements Tool { if (bloomMeta != null) bloomFilter = BloomFilterFactory.createFromMeta(bloomMeta, reader); - System.out.println("Bloom filter:"); + out.println("Bloom filter:"); if (bloomFilter != null) { - System.out.println(FOUR_SPACES + bloomFilter.toString().replaceAll( + out.println(FOUR_SPACES + bloomFilter.toString().replaceAll( ByteBloomFilter.STATS_RECORD_SEP, "\n" + FOUR_SPACES)); } else { - System.out.println(FOUR_SPACES + "Not present"); + out.println(FOUR_SPACES + "Not present"); } // Printing delete bloom information @@ -436,13 +465,13 @@ public class HFilePrettyPrinter extends Configured implements Tool { if (bloomMeta != null) bloomFilter = BloomFilterFactory.createFromMeta(bloomMeta, reader); - System.out.println("Delete Family Bloom filter:"); + out.println("Delete Family Bloom filter:"); if (bloomFilter != null) { - System.out.println(FOUR_SPACES + out.println(FOUR_SPACES + bloomFilter.toString().replaceAll(ByteBloomFilter.STATS_RECORD_SEP, "\n" + FOUR_SPACES)); } else { - System.out.println(FOUR_SPACES + "Not present"); + out.println(FOUR_SPACES + "Not present"); } } diff --git a/hbase-server/src/main/resources/hbase-webapps/regionserver/storeFile.jsp b/hbase-server/src/main/resources/hbase-webapps/regionserver/storeFile.jsp index cbbb61fa9d1..fe8cfe0421f 100644 --- a/hbase-server/src/main/resources/hbase-webapps/regionserver/storeFile.jsp +++ b/hbase-server/src/main/resources/hbase-webapps/regionserver/storeFile.jsp @@ -18,20 +18,15 @@ */ --%> <%@ page contentType="text/html;charset=UTF-8" - import="java.util.Collection" - import="java.util.Date" - import="java.util.List" import="java.io.ByteArrayOutputStream" import="java.io.PrintStream" - import="java.io.BufferedReader" - import="java.io.InputStreamReader" import="org.apache.hadoop.conf.Configuration" + import="org.apache.hadoop.fs.Path" import="org.apache.hadoop.hbase.HBaseConfiguration" import="org.apache.hadoop.hbase.io.hfile.HFilePrettyPrinter" import="org.apache.hadoop.hbase.regionserver.HRegionServer" - import="org.apache.hadoop.hbase.regionserver.Region" - import="org.apache.hadoop.hbase.regionserver.Store" - import="org.apache.hadoop.hbase.regionserver.StoreFile"%> + import="org.apache.hadoop.hbase.regionserver.StoreFile" + %> <% String storeFile = request.getParameter("name"); HRegionServer rs = (HRegionServer) getServletContext().getAttribute(HRegionServer.REGIONSERVER); @@ -91,17 +86,19 @@ 
 <%
    try {
-     ProcessBuilder pb=new ProcessBuilder("hbase", "hfile", "-s", "-f", storeFile);
-     pb.redirectErrorStream(true);
-     Process pr = pb.start();
-     BufferedReader in = new BufferedReader(new InputStreamReader(pr.getInputStream()));
-     String line;
-     while ((line = in.readLine()) != null) {%>
-       <%= line %>
-     <%}
-     pr.waitFor();
-     in.close();
-   }
+     ByteArrayOutputStream byteStream = new ByteArrayOutputStream();
+     PrintStream printerOutput = new PrintStream(byteStream);
+     HFilePrettyPrinter printer = new HFilePrettyPrinter();
+     printer.setPrintStreams(printerOutput, printerOutput);
+     printer.setConf(conf);
+     String[] options = {"-s"};
+     printer.parseOptions(options);
+     printer.processFile(new Path(storeFile));
+     String text = byteStream.toString();%>
+     <%=
+       text
+     %>
+   <%}
    catch (Exception e) {%>
      <%= e %>
    <%}