Skip to content

TEZ-4743: Refactor ProfileServlet and ProfileOutputServlet to use standard response.sendError API #527

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Aggarwal-Raghav wants to merge 1 commit into
base: master
Choose a base branch
from
+21 −45
Open

TEZ-4743: Refactor ProfileServlet and ProfileOutputServlet to use standard response.sendError API #527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 7 additions & 17 deletions tez-common/src/main/java/org/apache/tez/common/web/ProfileOutputServlet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,10 @@

import java.io.File;
import java.io.IOException;
import java.io.PrintWriter;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;

import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

Expand All @@ -40,46 +38,38 @@
public class ProfileOutputServlet extends DefaultServlet {
public static final String FILE_QUERY_PARAM = "file";

public void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException {
if (!HttpServer2.isInstrumentationAccessAllowed(this.getServletContext(), request, response)) {
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
writeMessage(response, ProfileServlet.ACCESS_DENIED_MESSAGE);
response.sendError(HttpServletResponse.SC_UNAUTHORIZED, ProfileServlet.ACCESS_DENIED_MESSAGE);
return;
}
String queriedFile = request.getParameter(FILE_QUERY_PARAM);
if (queriedFile == null) {
writeMessage(response, "Run the profiler to be able to receive its output");
response.sendError(HttpServletResponse.SC_BAD_REQUEST, "Run the profiler to be able to receive its output");
return;
}
Path outputDir = Paths.get(ProfileServlet.OUTPUT_DIR).toAbsolutePath().normalize();
Path requestedPath = outputDir.resolve(queriedFile).normalize();

if (!requestedPath.startsWith(outputDir)) {
response.setStatus(HttpServletResponse.SC_FORBIDDEN);
writeMessage(response, "Access denied: Invalid Path");
response.sendError(HttpServletResponse.SC_FORBIDDEN, "Access denied: Invalid Path");
return;
}
File outputFile = requestedPath.toFile();

if (!outputFile.exists()) {
writeMessage(response, "Requested file does not exist: " + queriedFile);
response.sendError(HttpServletResponse.SC_NOT_FOUND, "Requested file does not exist: " + queriedFile);
return;
}
if (outputFile.length() < 100) {
response.setIntHeader("Refresh", 2);
writeMessage(response, "This page auto-refreshes every 2 seconds until output file is ready...");
response.setContentType(MimeType.TEXT);
response.getWriter().println("This page auto-refreshes every 2 seconds until output file is ready...");
return;
}
response.setContentType(MimeType.HTML);
response.getOutputStream().write(Files.readAllBytes(Paths.get(outputFile.getPath())));
response.getOutputStream().flush();
response.getOutputStream().close();
}

private void writeMessage(HttpServletResponse response, String message) throws IOException {
response.setContentType(MimeType.TEXT);
PrintWriter out = response.getWriter();
out.println(message);
out.close();
}
}
42 changes: 14 additions & 28 deletions tez-common/src/main/java/org/apache/tez/common/web/ProfileServlet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@

import java.io.File;
import java.io.IOException;
import java.io.PrintStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
Expand All @@ -31,7 +30,6 @@
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;

import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
Expand Down Expand Up @@ -161,34 +159,24 @@ public ProfileServlet() {
LOG.info("Servlet process PID: {} asyncProfilerHome: {}", pid, asyncProfilerHome);
}

public void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
response.setContentType("text/plain; charset=UTF-8");
PrintStream out = new PrintStream(response.getOutputStream(), false, "UTF-8");
public void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException {
if (!HttpServer2.isInstrumentationAccessAllowed(this.getServletContext(), request, response)) {
response.setStatus(HttpServletResponse.SC_UNAUTHORIZED);
setResponseHeader(response);
out.println(ACCESS_DENIED_MESSAGE);
out.close();
response.sendError(HttpServletResponse.SC_UNAUTHORIZED, ACCESS_DENIED_MESSAGE);
return;
}

// make sure async profiler home is set
if (asyncProfilerHome == null || asyncProfilerHome.trim().isEmpty()) {
response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
setResponseHeader(response);
out.println("ASYNC_PROFILER_HOME env is not set");
out.close();
response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, "ASYNC_PROFILER_HOME env is not set");
return;
}

// if pid is explicitly specified, use it else default to current process
pid = getInteger(request, "pid", pid);
// if pid is not specified in query param and if current process pid cannot be determined
if (pid == null) {
response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
setResponseHeader(response);
out.println("'pid' query parameter unspecified or unable to determine PID of current process.");
out.close();
response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
"'pid' query parameter unspecified or unable to determine PID of current process.");
return;
}

Expand Down Expand Up @@ -278,41 +266,39 @@ public void doGet(HttpServletRequest request, HttpServletResponse response) thro
response.setHeader("Refresh", (duration + refreshDelay) + "; URL=" + relativeUrl + '?'
+ ProfileOutputServlet.FILE_QUERY_PARAM + '=' + outputFile.getName());

out.println("Profiled PID: " + pid);
out.println("Started [" + event.getInternalName()
response.getWriter().println("Profiled PID: " + pid);
response.getWriter().println("Started [" + event.getInternalName()
+ "] profiling. This page will automatically redirect to "
+ relativeUrl + " after " + duration + " seconds.\n\ncommand:\n" + Joiner.on(" ").join(cmd));
out.flush();
response.getWriter().flush();
} finally {
profilerLock.unlock();
}
} else {
setResponseHeader(response);
response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
out.println("Unable to acquire lock. Another instance of profiler might be running.");
LOG.warn("Unable to acquire lock in {} seconds. Another instance of profiler might be running.",
lockTimeoutSecs);
response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
"Unable to acquire lock. Another instance of profiler might be running.");
}
} catch (InterruptedException e) {
LOG.warn("Interrupted while acquiring profile lock.", e);
response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
}
} else {
setResponseHeader(response);
response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
out.println("Another instance of profiler is already running.");
response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
"Another instance of profiler is already running.");
}
out.close();
}

/**
* Get the path of the profiler script to be executed.
* Before async-profiler 3.0, the script was named profiler.sh, and after 3.0 it's bin/asprof
*
* @return
*/
private String getProfilerScriptPath() {
Path defaultPath = Paths.get(asyncProfilerHome + "/bin/asprof");
return Files.exists(defaultPath)? defaultPath.toString() : asyncProfilerHome + "/profiler.sh";
return Files.exists(defaultPath) ? defaultPath.toString() : asyncProfilerHome + "/profiler.sh";
}

private Integer getInteger(final HttpServletRequest req, final String param, final Integer defaultValue) {
Expand Down