From f6d884cd118fdb6987eb3c369fc9a4c9317acf68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1rton=20Elek?= Date: Sat, 14 Sep 2019 06:18:33 +0200 Subject: [PATCH] HDDS-2110. Arbitrary file can be downloaded with the help of ProfilerServlet Signed-off-by: Anu Engineer --- .../hadoop/hdds/server/ProfileServlet.java | 60 +++++++++++++----- .../hdds/server/TestProfileServlet.java | 63 +++++++++++++++++++ 2 files changed, 109 insertions(+), 14 deletions(-) create mode 100644 hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/server/TestProfileServlet.java diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/ProfileServlet.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/ProfileServlet.java index e09e9b5ee54..42944e11e22 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/ProfileServlet.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/ProfileServlet.java @@ -32,7 +32,9 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; +import java.util.regex.Pattern; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import org.apache.commons.io.IOUtils; import org.slf4j.Logger; @@ -111,6 +113,10 @@ public class ProfileServlet extends HttpServlet { private static final AtomicInteger ID_GEN = new AtomicInteger(0); static final Path OUTPUT_DIR = Paths.get(System.getProperty("java.io.tmpdir"), "prof-output"); + public static final String FILE_PREFIX = "async-prof-pid-"; + + public static final Pattern FILE_NAME_PATTERN = + Pattern.compile(FILE_PREFIX + "[0-9]+-[0-9A-Za-z\\-_]+-[0-9]+\\.[a-z]+"); private Lock profilerLock = new ReentrantLock(); private Integer pid; @@ -165,6 +171,26 @@ public class ProfileServlet extends HttpServlet { } } + @VisibleForTesting + protected static String generateFileName(Integer pid, Output output, + Event event) { + return FILE_PREFIX + pid + "-" + + event.name().toLowerCase() + "-" + ID_GEN.incrementAndGet() + + "." + + output.name().toLowerCase(); + } + + @VisibleForTesting + protected static String validateFileName(String filename) { + if (!FILE_NAME_PATTERN.matcher(filename).matches()) { + throw new IllegalArgumentException( + "Invalid file name parameter " + filename + " doesn't match pattern " + + FILE_NAME_PATTERN); + + } + return filename; + } + @Override protected void doGet(final HttpServletRequest req, final HttpServletResponse resp) throws IOException { @@ -195,7 +221,8 @@ public class ProfileServlet extends HttpServlet { return; } - final int duration = getInteger(req, "duration", DEFAULT_DURATION_SECONDS); + final int duration = + getInteger(req, "duration", DEFAULT_DURATION_SECONDS); final Output output = getOutput(req); final Event event = getEvent(req); final Long interval = getLong(req, "interval"); @@ -213,11 +240,11 @@ public class ProfileServlet extends HttpServlet { int lockTimeoutSecs = 3; if (profilerLock.tryLock(lockTimeoutSecs, TimeUnit.SECONDS)) { try { + //Should be in sync with FILE_NAME_PATTERN File outputFile = - OUTPUT_DIR.resolve("async-prof-pid-" + pid + "-" + - event.name().toLowerCase() + "-" + ID_GEN.incrementAndGet() - + "." + - output.name().toLowerCase()).toFile(); + OUTPUT_DIR.resolve( + ProfileServlet.generateFileName(pid, output, event)) + .toFile(); List cmd = new ArrayList<>(); cmd.add(asyncProfilerHome + PROFILER_SCRIPT); cmd.add("-e"); @@ -270,7 +297,8 @@ public class ProfileServlet extends HttpServlet { String relativeUrl = "/prof?file=" + outputFile.getName(); resp.getWriter().write( "Started [" + event.getInternalName() - + "] profiling. This page will automatically redirect to " + + + "] profiling. This page will automatically redirect to " + + relativeUrl + " after " + duration + " seconds.\n\ncommand:\n" + Joiner.on(" ").join(cmd)); resp.getWriter().write( @@ -320,9 +348,12 @@ public class ProfileServlet extends HttpServlet { final HttpServletResponse resp) throws IOException { + ; + String safeFileName = validateFileName(fileName); File requestedFile = - ProfileServlet.OUTPUT_DIR.resolve(fileName).toAbsolutePath() - .toFile(); + ProfileServlet.OUTPUT_DIR + .resolve(safeFileName) + .toAbsolutePath().toFile(); // async-profiler version 1.4 writes 'Started [cpu] profiling' to output // file when profiler is running which // gets replaced by final output. If final output is not ready yet, the @@ -331,14 +362,14 @@ public class ProfileServlet extends HttpServlet { LOG.info("{} is incomplete. Sending auto-refresh header..", requestedFile); resp.setHeader("Refresh", - "2," + req.getRequestURI() + "?file=" + fileName); + "2," + req.getRequestURI() + "?file=" + safeFileName); resp.getWriter().write( "This page will auto-refresh every 2 second until output file is " + "ready.."); } else { - if (fileName.endsWith(".svg")) { + if (safeFileName.endsWith(".svg")) { resp.setContentType("image/svg+xml"); - } else if (fileName.endsWith(".tree")) { + } else if (safeFileName.endsWith(".tree")) { resp.setContentType("text/html"); } try (InputStream input = new FileInputStream(requestedFile)) { @@ -347,7 +378,8 @@ public class ProfileServlet extends HttpServlet { } } - private Integer getInteger(final HttpServletRequest req, final String param, + private Integer getInteger(final HttpServletRequest req, + final String param, final Integer defaultValue) { final String value = req.getParameter(param); if (value != null) { @@ -439,8 +471,8 @@ public class ProfileServlet extends HttpServlet { L1_DCACHE_LOAD_MISSES("L1-dcache-load-misses"), LLC_LOAD_MISSES("LLC-load-misses"), DTLB_LOAD_MISSES("dTLB-load-misses"), - MEM_BREAKPOINT("mem:breakpoint"), - TRACE_TRACEPOINT("trace:tracepoint"); + MEM_BREAKPOINT("mem-breakpoint"), + TRACE_TRACEPOINT("trace-tracepoint"); private String internalName; diff --git a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/server/TestProfileServlet.java b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/server/TestProfileServlet.java new file mode 100644 index 00000000000..c77fee06f46 --- /dev/null +++ b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/server/TestProfileServlet.java @@ -0,0 +1,63 @@ +/** + * 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.hdds.server; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.OutputStreamWriter; + +import org.apache.hadoop.hdds.server.ProfileServlet.Event; +import org.apache.hadoop.hdds.server.ProfileServlet.Output; +import org.apache.hadoop.metrics2.MetricsSystem; +import org.apache.hadoop.metrics2.annotation.Metric; +import org.apache.hadoop.metrics2.annotation.Metrics; +import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem; +import org.apache.hadoop.metrics2.lib.MutableCounterLong; + +import static java.nio.charset.StandardCharsets.UTF_8; +import org.junit.Assert; +import org.junit.Test; + +/** + * Test prometheus Sink. + */ +public class TestProfileServlet { + + @Test + public void testNameValidation() throws IOException { + ProfileServlet.validateFileName( + ProfileServlet.generateFileName(1, Output.SVG, Event.ALLOC)); + + ProfileServlet.validateFileName( + ProfileServlet.generateFileName(23, Output.COLLAPSED, + Event.L1_DCACHE_LOAD_MISSES)); + } + + @Test(expected = IllegalArgumentException.class) + public void testNameValidationWithNewLine() throws IOException { + ProfileServlet.validateFileName( + "test\n" + ProfileServlet.generateFileName(1, Output.SVG, Event.ALLOC)); + } + + @Test(expected = IllegalArgumentException.class) + public void testNameValidationWithSlash() throws IOException { + ProfileServlet.validateFileName( + "../" + ProfileServlet.generateFileName(1, Output.SVG, Event.ALLOC)); + } + +} \ No newline at end of file