Author: remy.maucherat(a)jboss.com Date: 2010-11-23 05:18:39 -0500 (Tue, 23 Nov 2010) New Revision: 1584 Modified: trunk/java/org/apache/catalina/servlets/CGIServlet.java Log: - Port CGI updates. Modified: trunk/java/org/apache/catalina/servlets/CGIServlet.java =================================================================== --- trunk/java/org/apache/catalina/servlets/CGIServlet.java 2010-11-23 10:13:25 UTC (rev 1583) +++ trunk/java/org/apache/catalina/servlets/CGIServlet.java 2010-11-23 10:18:39 UTC (rev 1584) @@ -41,7 +41,6 @@ import javax.servlet.ServletContext; import javax.servlet.ServletException; import javax.servlet.ServletOutputStream; -import javax.servlet.UnavailableException; import javax.servlet.http.Cookie; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; @@ -123,7 +122,7 @@ * or an instance where the specification cited differs from Best * Community Practice (BCP). * Such instances should be well-documented here. Please email the - * <a href="mailto:tomcat-dev@jakarta.apache.org">Jakarta Tomcat group [tomcat-dev(a)jakarta.apache.org]&lt;/a&gt; + * <a href="mailto:dev@tomcat.apache.org">Tomcat group [dev(a)tomcat.apache.org]&lt;/a&gt; * with amendments. * * </p> @@ -179,7 +178,7 @@ * </p> * <p> * - * <b>Metavariable Values</b>: According to the CGI specificion, + * <b>Metavariable Values</b>: According to the CGI specification, * implementations may choose to represent both null or missing values in an * implementation-specific manner, but must define that manner. This * implementation chooses to always define all required metavariables, but @@ -192,7 +191,7 @@ * * <b>NPH -- Non-parsed-header implementation</b>: This implementation does * not support the CGI NPH concept, whereby server ensures that the data - * supplied to the script are preceisely as supplied by the client and + * supplied to the script are precisely as supplied by the client and * unaltered by the server. * </p> * <p> @@ -227,15 +226,13 @@ * <li> Better documentation * <li> Confirm use of ServletInputStream.available() in CGIRunner.run() is * not needed - * <li> Make checking for "." and ".." in servlet & cgi PATH_INFO less - * draconian * <li> [add more to this TODO list] * </ul> * </p> * * @author Martin T Dengler [root(a)martindengler.com] * @author Amy Roh - * @version $Revision$, $Date$ + * @version $Id$ * @since Tomcat 4.0 * */ @@ -245,6 +242,8 @@ /* some vars below copied from Craig R. McClanahan's InvokerServlet */ + private static final long serialVersionUID = 1L; + /** the debugging detail level for this servlet. */ private int debug = 0; @@ -260,9 +259,15 @@ private String cgiExecutable = "perl"; /** the encoding to use for parameters */ - private String parameterEncoding = System.getProperty("file.encoding", - "UTF-8"); + private String parameterEncoding = + System.getProperty("file.encoding", "UTF-8"); + /** + * The time (in milliseconds) to wait for the reading of stderr to complete + * before terminating the CGI process. + */ + private long stderrTimeout = 2000; + /** object used to ensure multiple threads don't try to expand same file */ static Object expandFileLock = new Object(); @@ -284,18 +289,11 @@ * interferes with the servlet's normal * operation */ + @Override public void init(ServletConfig config) throws ServletException { super.init(config); - // Verify that we were not accessed using the invoker servlet - String servletName = getServletConfig().getServletName(); - if (servletName == null) - servletName = ""; - if (servletName.startsWith("org.apache.catalina.INVOKER.")) - throw new UnavailableException - ("Cannot invoke CGIServlet through the invoker"); - // Set our properties from the initialization parameters if (getServletConfig().getInitParameter("debug") != null) debug = Integer.parseInt(getServletConfig().getInitParameter("debug")); @@ -315,10 +313,14 @@ parameterEncoding = getServletConfig().getInitParameter("parameterEncoding"); } + if (getServletConfig().getInitParameter("stderrTimeout") != null) { + stderrTimeout = Long.parseLong(getServletConfig().getInitParameter( + "stderrTimeout")); + } + } - /** * Prints out important Servlet API and container information * @@ -335,7 +337,9 @@ * */ protected void printServletEnvironment(ServletOutputStream out, - HttpServletRequest req, HttpServletResponse res) throws IOException { + HttpServletRequest req, + @SuppressWarnings("unused") HttpServletResponse res) + throws IOException { // Document the properties from ServletRequest out.println("<h1>ServletRequest Properties</h1>"); @@ -521,11 +525,9 @@ out.println("<hr>"); - } - /** * Provides CGI Gateway service -- delegates to <code>doGet</code> * @@ -538,13 +540,13 @@ * @see javax.servlet.http.HttpServlet * */ + @Override protected void doPost(HttpServletRequest req, HttpServletResponse res) throws IOException, ServletException { doGet(req, res); } - /** * Provides CGI Gateway service * @@ -557,6 +559,7 @@ * @see javax.servlet.http.HttpServlet * */ + @Override protected void doGet(HttpServletRequest req, HttpServletResponse res) throws ServletException, IOException { @@ -620,13 +623,6 @@ } //doGet - - /** For future testing use only; does nothing right now */ - public static void main(String[] args) { - System.out.println("$Header$"); - } - - /** * Encapsulates the CGI environment and rules to derive * that environment from the servlet container and request information. @@ -634,7 +630,7 @@ * <p> * </p> * - * @version $Revision$, $Date$ + * @version $Id$ * @since Tomcat 4.0 * */ @@ -700,7 +696,6 @@ } - /** * Uses the ServletContext to set some CGI variables * @@ -714,7 +709,6 @@ } - /** * Uses the HttpServletRequest to set most CGI variables * @@ -880,23 +874,23 @@ } if (!currentLocation.isFile()) { return new String[] { null, null, null, null }; - } else { - if (debug >= 2) { - log("findCGI: FOUND cgi at " + currentLocation); - } - path = currentLocation.getAbsolutePath(); - name = currentLocation.getName(); + } - if (".".equals(contextPath)) { - scriptname = servletPath; - } else { - scriptname = contextPath + servletPath; - } - if (!servletPath.equals(cginame)) { - scriptname = scriptname + cginame; - } + if (debug >= 2) { + log("findCGI: FOUND cgi at " + currentLocation); } + path = currentLocation.getAbsolutePath(); + name = currentLocation.getName(); + if (".".equals(contextPath)) { + scriptname = servletPath; + } else { + scriptname = contextPath + servletPath; + } + if (!servletPath.equals(cginame)) { + scriptname = scriptname + cginame; + } + if (debug >= 1) { log("findCGI calc: name=" + name + ", path=" + path + ", scriptname=" + scriptname + ", cginame=" + cginame); @@ -906,10 +900,10 @@ /** * Constructs the CGI environment to be supplied to the invoked CGI - * script; relies heavliy on Servlet API methods and findCGI + * script; relies heavily on Servlet API methods and findCGI * * @param req request associated with the CGI - * invokation + * Invocation * * @return true if environment was set OK, false if there * was a problem and no environment was set @@ -1184,6 +1178,7 @@ * @return HTML string containing CGI environment info * */ + @Override public String toString() { StringBuilder sb = new StringBuilder(); @@ -1241,7 +1236,6 @@ } - /** * Gets derived command string * @@ -1253,7 +1247,6 @@ } - /** * Gets derived CGI working directory * @@ -1265,7 +1258,6 @@ } - /** * Gets derived CGI environment * @@ -1277,7 +1269,6 @@ } - /** * Gets derived CGI query parameters * @@ -1289,7 +1280,6 @@ } - /** * Gets validity status * @@ -1302,7 +1292,6 @@ } - /** * Converts null strings to blank strings ("") * @@ -1315,7 +1304,6 @@ } - /** * Converts null strings to another string * @@ -1330,7 +1318,6 @@ } - /** * Converts blank strings to another string * @@ -1347,14 +1334,9 @@ } - } //class CGIEnvironment - - - - /** * Encapsulates the knowledge of how to run a CGI script, given the * script's desired environment and (optionally) input/output streams @@ -1368,7 +1350,7 @@ * <p> * * The CGI environment and settings are derived from the information - * passed to the constuctor. + * passed to the constructor. * * </p> * <p> @@ -1377,7 +1359,7 @@ * and <code>setResponse</code> methods, respectively. * </p> * - * @version $Revision$, $Date$ + * @version $Id$ */ protected class CGIRunner { @@ -1404,8 +1386,6 @@ private boolean readyToRun = false; - - /** * Creates a CGIRunner and initializes its environment, working * directory, and query parameters. @@ -1418,7 +1398,7 @@ * @param env Hashtable with the desired script environment * @param wd File with the script's desired working directory * @param params ArrayList with the script's query command line - * paramters as strings + * parameters as strings */ protected CGIRunner(String command, Hashtable<String,String> env, File wd, ArrayList<String> params) { @@ -1430,7 +1410,6 @@ } - /** * Checks & sets ready status */ @@ -1447,7 +1426,6 @@ } - /** * Gets ready status * @@ -1459,7 +1437,6 @@ } - /** * Sets HttpServletResponse object used to set headers and send * output to @@ -1473,7 +1450,6 @@ } - /** * Sets standard input to be passed on to the invoked cgi script * @@ -1486,7 +1462,6 @@ } - /** * Converts a Hashtable to a String array by converting each * key/value pair in the Hashtable to a String in the form @@ -1513,7 +1488,6 @@ } - /** * Executes a CGI script with the desired environment, current working * directory, and input/output streams @@ -1546,7 +1520,10 @@ * segments</u>: * This implementation does not allow "<code>.</code>" and * "<code>..</code>" in the the path, and such characters - * will result in an IOException being thrown; + * will result in an IOException being thrown (this should + * never happen since Tomcat normalises the requestURI + * before determining the contextPath, servletPath and + * pathInfo); * <LI> <u>Implementation limitations</u>: This implementation * does not impose any limitations except as documented * above. This implementation may be limited by the @@ -1588,50 +1565,32 @@ } /* original content/structure of this section taken from - * http://developer.java.sun.com/developer/ - * bugParade/bugs/4216884.html + * http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4216884 * with major modifications by Martin Dengler */ Runtime rt = null; BufferedReader cgiHeaderReader = null; InputStream cgiOutput = null; BufferedReader commandsStdErr = null; + Thread errReaderThread = null; BufferedOutputStream commandsStdIn = null; Process proc = null; int bufRead = -1; - //create query arguments - StringBuilder cmdAndArgs = new StringBuilder(); - if (command.indexOf(" ") < 0) { - cmdAndArgs.append(command); - } else { - // Spaces used as delimiter, so need to use quotes - cmdAndArgs.append("\""); - cmdAndArgs.append(command); - cmdAndArgs.append("\""); - } + String[] cmdAndArgs = new String[params.size() + 2]; + + cmdAndArgs[0] = cgiExecutable; + + cmdAndArgs[1] = command; + //create query arguments for (int i=0; i < params.size(); i++) { - cmdAndArgs.append(" "); - String param = params.get(i); - if (param.indexOf(" ") < 0) { - cmdAndArgs.append(param); - } else { - // Spaces used as delimiter, so need to use quotes - cmdAndArgs.append("\""); - cmdAndArgs.append(param); - cmdAndArgs.append("\""); - } + cmdAndArgs[i + 2] = params.get(i); } - StringBuilder command = new StringBuilder(cgiExecutable); - command.append(" "); - command.append(cmdAndArgs.toString()); - cmdAndArgs = command; - try { rt = Runtime.getRuntime(); - proc = rt.exec(cmdAndArgs.toString(), hashToStringArray(env), wd); + proc = rt.exec(cmdAndArgs, hashToStringArray(env), wd); String sContentLength = env.get("CONTENT_LENGTH"); @@ -1644,8 +1603,7 @@ /* we want to wait for the process to exit, Process.waitFor() * is useless in our situation; see - * http://developer.java.sun.com/developer/ - * bugParade/bugs/4223650.html + * http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=4223650 */ boolean isRunning = true; @@ -1653,11 +1611,13 @@ (new InputStreamReader(proc.getErrorStream())); final BufferedReader stdErrRdr = commandsStdErr ; - new Thread() { + errReaderThread = new Thread() { + @Override public void run () { sendToLog(stdErrRdr) ; } - }.start() ; + }; + errReaderThread.start(); InputStream cgiHeaderStream = new HTTPHeaderInputStream(proc.getInputStream()); @@ -1709,7 +1669,9 @@ // such as a socket disconnect on the servlet side; otherwise, the // external process could hang if (bufRead != -1) { - while ((bufRead = cgiOutput.read(bBuf)) != -1) {} + while ((bufRead = cgiOutput.read(bBuf)) != -1) { + // NOOP - just read the data + } } } @@ -1721,6 +1683,7 @@ try { Thread.sleep(500); } catch (InterruptedException ignored) { + // Ignore } } } //replacement for Process.waitFor() @@ -1747,6 +1710,14 @@ log ("Exception closing output stream " + ioe); } } + // Make sure the error stream reader has finished + if (errReaderThread != null) { + try { + errReaderThread.join(stderrTimeout); + } catch (InterruptedException e) { + log ("Interupted waiting for stderr reader thread"); + } + } if (debug > 4) { log ("Running finally block"); } @@ -1862,6 +1833,7 @@ /** * @see java.io.InputStream#read() */ + @Override public int read() throws IOException { if (state == STATE_HEADER_END) { return -1;