From 7c0f7cc8f467da23898265aa15c72d30f710bc0b Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 3 Dec 2012 12:55:37 -0700 Subject: [PATCH 01/14] Fixing for JDK5. --- .../main/java/org/eclipse/jetty/util/ReadLineInputStream.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ReadLineInputStream.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ReadLineInputStream.java index 3946ca07483..035d5174ed4 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/ReadLineInputStream.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ReadLineInputStream.java @@ -69,7 +69,7 @@ public class ReadLineInputStream extends BufferedInputStream _skipLF=true; int m=markpos; markpos=-1; - return new String(buf,m,p-m-1,StringUtil.__UTF8_CHARSET); + return new String(buf,m,p-m-1,StringUtil.__UTF8); } if (b=='\n') @@ -83,7 +83,7 @@ public class ReadLineInputStream extends BufferedInputStream } int m=markpos; markpos=-1; - return new String(buf,m,pos-m-1,StringUtil.__UTF8_CHARSET); + return new String(buf,m,pos-m-1,StringUtil.__UTF8); } } } From 57849a905f4e35e1620a250e42b2af03336717ca Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 3 Dec 2012 13:04:03 -0700 Subject: [PATCH 02/14] Adding -Dtest.slow.destroy support to CookieDump to aid in testing of start.jar -DSTOP.WAIT=360 --stop --- .../src/main/java/com/acme/CookieDump.java | 18 ++++++++++++++++++ .../src/main/java/com/acme/HelloWorld.java | 4 ---- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/test-jetty-webapp/src/main/java/com/acme/CookieDump.java b/test-jetty-webapp/src/main/java/com/acme/CookieDump.java index 5e045227d77..46145898ecc 100644 --- a/test-jetty-webapp/src/main/java/com/acme/CookieDump.java +++ b/test-jetty-webapp/src/main/java/com/acme/CookieDump.java @@ -19,6 +19,7 @@ package com.acme; import java.io.IOException; import java.io.PrintWriter; +import java.util.concurrent.TimeUnit; import javax.servlet.ServletException; import javax.servlet.http.Cookie; @@ -121,4 +122,21 @@ public class CookieDump extends HttpServlet return string; } + @Override + public void destroy() + { + // For testing --stop with STOP.WAIT handling of the jetty-start behavior. + if (Boolean.getBoolean("test.slow.destroy")) + { + try + { + TimeUnit.SECONDS.sleep(10); + } + catch (InterruptedException e) + { + // ignore + } + } + super.destroy(); + } } diff --git a/test-jetty-webapp/src/main/java/com/acme/HelloWorld.java b/test-jetty-webapp/src/main/java/com/acme/HelloWorld.java index abacf87805c..67caa063480 100644 --- a/test-jetty-webapp/src/main/java/com/acme/HelloWorld.java +++ b/test-jetty-webapp/src/main/java/com/acme/HelloWorld.java @@ -67,8 +67,4 @@ public class HelloWorld extends HttpServlet getServletContext().log("exception",e); } } - - - - } From 7488452e8a95bca73a0e69694c34c5cac8c64661 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 3 Dec 2012 14:55:22 -0700 Subject: [PATCH 03/14] Adding some debugging --- .../src/main/java/org/eclipse/jetty/start/Main.java | 12 ++++++++---- .../main/java/org/eclipse/jetty/start/Monitor.java | 3 +-- .../src/main/java/com/acme/CookieDump.java | 1 + 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java b/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java index 8323e53852f..b066a569145 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java @@ -1040,11 +1040,15 @@ public class Main if (timeout > 0) { - System.err.println("Waiting"+(timeout > 0 ? (" "+timeout+"sec") : "")+" for jetty to stop"); + System.err.printf("Waiting %,d seconds for jetty to stop%n", timeout); LineNumberReader lin = new LineNumberReader(new InputStreamReader(s.getInputStream())); - String response=lin.readLine(); - if ("Stopped".equals(response)) - System.err.println("Stopped"); + String response; + while ((response = lin.readLine()) != null) + { + Config.debug("Received \"" + response + "\""); + if ("Stopped".equals(response)) + System.err.println("Server reports itself as Stopped"); + } } } finally diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/Monitor.java b/jetty-start/src/main/java/org/eclipse/jetty/start/Monitor.java index ff29f11df94..1f5667b09af 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/Monitor.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/Monitor.java @@ -17,7 +17,6 @@ // package org.eclipse.jetty.start; -import java.io.IOException; import java.io.InputStreamReader; import java.io.LineNumberReader; import java.net.InetAddress; @@ -114,6 +113,7 @@ public class Monitor extends Thread Config.debug("command=" + cmd); if ("stop".equals(cmd)) { + Config.debug("Child Process: " + _process); if (_process!=null) { //if we have a child process, wait for it to finish before we stop @@ -121,7 +121,6 @@ public class Monitor extends Thread { _process.destroy(); _process.waitFor(); - } catch (InterruptedException e) { diff --git a/test-jetty-webapp/src/main/java/com/acme/CookieDump.java b/test-jetty-webapp/src/main/java/com/acme/CookieDump.java index 46145898ecc..d5d32e3a9ce 100644 --- a/test-jetty-webapp/src/main/java/com/acme/CookieDump.java +++ b/test-jetty-webapp/src/main/java/com/acme/CookieDump.java @@ -128,6 +128,7 @@ public class CookieDump extends HttpServlet // For testing --stop with STOP.WAIT handling of the jetty-start behavior. if (Boolean.getBoolean("test.slow.destroy")) { + log("Simulating a slow destroy (10 seconds)",null); try { TimeUnit.SECONDS.sleep(10); From 7abd512b32df732f185c2a2e0752a70575df43f1 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 4 Dec 2012 10:18:54 -0700 Subject: [PATCH 04/14] 391623 - Making --stop with STOP.WAIT perform graceful shutdown + Moving jetty-start Monitor to jetty-server ShutdownMonitor and using ShutdownThread to perform a graceful shutdown instead. --- .../java/org/eclipse/jetty/server/Server.java | 6 +- .../eclipse/jetty/server/ShutdownMonitor.java | 235 ++++++++++++++++++ .../java/org/eclipse/jetty/start/Main.java | 83 +++++-- .../java/org/eclipse/jetty/start/Monitor.java | 157 ------------ 4 files changed, 296 insertions(+), 185 deletions(-) create mode 100644 jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java delete mode 100644 jetty-start/src/main/java/org/eclipse/jetty/start/Monitor.java diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java index c891d63066d..9fc62e9f57d 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java @@ -21,6 +21,7 @@ package org.eclipse.jetty.server; import java.io.IOException; import java.net.InetSocketAddress; import java.util.Enumeration; + import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -260,11 +261,14 @@ public class Server extends HandlerWrapper implements Attributes @Override protected void doStart() throws Exception { - if (getStopAtShutdown()) + if (getStopAtShutdown()) { ShutdownThread.register(this); + ShutdownMonitor.getInstance(); // initialize + } LOG.info("jetty-"+__version); HttpGenerator.setServerVersion(__version); + MultiException mex=new MultiException(); if (_threadPool==null) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java new file mode 100644 index 00000000000..25335475957 --- /dev/null +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java @@ -0,0 +1,235 @@ +// +// ======================================================================== +// Copyright (c) 1995-2012 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.server; + +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.LineNumberReader; +import java.io.OutputStream; +import java.net.InetAddress; +import java.net.ServerSocket; +import java.net.Socket; +import java.util.Properties; + +import org.eclipse.jetty.util.StringUtil; +import org.eclipse.jetty.util.thread.ShutdownThread; + +/** + * Shutdown/Stop Monitor thread. + *

+ * This thread listens on the port specified by the STOP.PORT system parameter (defaults to -1 for not listening) for + * request authenticated with the key given by the STOP.KEY system parameter (defaults to "eclipse") for admin requests. + *

+ * If the stop port is set to zero, then a random port is assigned and the port number is printed to stdout. + *

+ * Commands "stop" and "status" are currently supported. + */ +public class ShutdownMonitor extends Thread +{ + private static final ShutdownMonitor INSTANCE = new ShutdownMonitor(); + + public static ShutdownMonitor getInstance() + { + return INSTANCE; + } + + private final boolean DEBUG; + private final int port; + private final String key; + private final ServerSocket serverSocket; + + private ShutdownMonitor() + { + Properties props = System.getProperties(); + + // Use the same debug option as /jetty-start/ + this.DEBUG = props.containsKey("DEBUG"); + + // Use values passed thru /jetty-start/ + int stopPort = Integer.parseInt(props.getProperty("STOP.PORT","-1")); + String stopKey = props.getProperty("STOP.KEY",null); + + ServerSocket sock = null; + + try + { + if (stopPort < 0) + { + System.out.println("ShutdownMonitor not in use"); + sock = null; + return; + } + + setDaemon(true); + setName("ShutdownMonitor"); + + sock = new ServerSocket(stopPort,1,InetAddress.getByName("127.0.0.1")); + if (stopPort == 0) + { + // server assigned port in use + stopPort = sock.getLocalPort(); + System.out.printf("STOP.PORT=%d%n",stopPort); + } + + if (stopKey == null) + { + // create random key + stopKey = Long.toString((long)(Long.MAX_VALUE * Math.random() + this.hashCode() + System.currentTimeMillis()),36); + System.out.printf("STOP.KEY=%s%n",stopKey); + } + + } + catch (Exception e) + { + debug(e); + System.err.println("Error binding monitor port " + stopPort + ": " + e.toString()); + } + finally + { + // establish the port and key that are in use + this.port = stopPort; + this.key = stopKey; + + this.serverSocket = sock; + debug("STOP.PORT=%d", port); + debug("STOP.KEY=%s", key); + debug("%s", serverSocket); + } + + this.start(); + } + + @Override + public String toString() + { + return String.format("%s[port=%d]",this.getClass().getName(),port); + } + + private void debug(Throwable t) + { + if (DEBUG) + { + t.printStackTrace(System.err); + } + } + + private void debug(String format, Object... args) + { + if (DEBUG) + { + System.err.printf("[ShutdownMonitor] " + format + "%n",args); + } + } + + @Override + public void run() + { + while (true) + { + Socket socket = null; + try + { + socket = serverSocket.accept(); + + LineNumberReader lin = new LineNumberReader(new InputStreamReader(socket.getInputStream())); + String key = lin.readLine(); + if (!this.key.equals(key)) + { + System.err.println("Ignoring command with incorrect key"); + continue; + } + + OutputStream out = socket.getOutputStream(); + + String cmd = lin.readLine(); + debug("command=%s",cmd); + if ("stop".equals(cmd)) + { + // Graceful Shutdown + debug("Issuing graceful shutdown.."); + ShutdownThread.getInstance().run(); + + // Reply to client + debug("Informing client that we are stopped."); + out.write("Stopped\r\n".getBytes(StringUtil.__UTF8)); + out.flush(); + + // Shutdown Monitor + debug("Shutting down monitor"); + close(socket); + close(serverSocket); + + // Kill JVM + debug("Killing JVM"); + System.exit(0); + } + else if ("status".equals(cmd)) + { + // Reply to client + out.write("OK\r\n".getBytes(StringUtil.__UTF8)); + out.flush(); + } + } + catch (Exception e) + { + debug(e); + System.err.println(e.toString()); + } + finally + { + close(socket); + socket = null; + } + } + } + + private void close(Socket socket) + { + if (socket == null) + { + return; + } + + try + { + socket.close(); + } + catch (IOException ignore) + { + /* ignore */ + } + } + + private void close(ServerSocket server) + { + if (server == null) + { + return; + } + + try + { + server.close(); + } + catch (IOException ignore) + { + /* ignore */ + } + } +} diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java b/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java index b066a569145..4ab1764fb36 100644 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java +++ b/jetty-start/src/main/java/org/eclipse/jetty/start/Main.java @@ -53,12 +53,13 @@ import java.util.Set; /*-------------------------------------------*/ /** *

- * Main start class. This class is intended to be the main class listed in the MANIFEST.MF of the start.jar archive. It allows an application to be started with - * the command "java -jar start.jar". + * Main start class. This class is intended to be the main class listed in the MANIFEST.MF of the start.jar archive. It + * allows an application to be started with the command "java -jar start.jar". *

- * + * *

- * The behaviour of Main is controlled by the parsing of the {@link Config} "org/eclipse/start/start.config" file obtained as a resource or file. + * The behaviour of Main is controlled by the parsing of the {@link Config} "org/eclipse/start/start.config" file + * obtained as a resource or file. *

*/ public class Main @@ -91,7 +92,7 @@ public class Main Main main = new Main(); List arguments = main.expandCommandLine(args); List xmls = main.processCommandLine(arguments); - if (xmls!=null) + if (xmls != null) main.start(xmls); } catch (Throwable e) @@ -143,7 +144,7 @@ public class Main List parseStartIniFiles() { - List ini_args=new ArrayList(); + List ini_args = new ArrayList(); File start_ini = new File(_jettyHome,"start.ini"); if (start_ini.exists()) ini_args.addAll(loadStartIni(start_ini)); @@ -179,14 +180,14 @@ public class Main _showUsage = true; continue; } - + if ("--stop".equals(arg)) { int port = Integer.parseInt(Config.getProperty("STOP.PORT","-1")); String key = Config.getProperty("STOP.KEY",null); - int timeout = Integer.parseInt(Config.getProperty("STOP.WAIT", "0")); + int timeout = Integer.parseInt(Config.getProperty("STOP.WAIT","0")); stop(port,key,timeout); - return null; + return null; } if ("--version".equals(arg) || "-v".equals(arg) || "--info".equals(arg)) @@ -226,7 +227,7 @@ public class Main if (!startDir.exists() || !startDir.canWrite()) startDir = new File("."); - File startLog = new File(startDir, START_LOG_ROLLOVER_DATEFORMAT.format(new Date())); + File startLog = new File(startDir,START_LOG_ROLLOVER_DATEFORMAT.format(new Date())); if (!startLog.exists() && !startLog.createNewFile()) { @@ -246,7 +247,7 @@ public class Main PrintStream logger = new PrintStream(new FileOutputStream(startLog,false)); System.setOut(logger); System.setErr(logger); - System.out.println("Establishing "+ START_LOG_FILENAME + " on " + new Date()); + System.out.println("Establishing " + START_LOG_FILENAME + " on " + new Date()); continue; } @@ -492,11 +493,6 @@ public class Main /* ------------------------------------------------------------ */ public void start(List xmls) throws IOException, InterruptedException { - // Setup Start / Stop Monitoring - int port = Integer.parseInt(Config.getProperty("STOP.PORT","-1")); - String key = Config.getProperty("STOP.KEY",null); - Monitor monitor = new Monitor(port,key); - // Load potential Config (start.config) List configuredXmls = loadConfig(xmls); @@ -581,9 +577,8 @@ public class Main copyInThread(process.getErrorStream(),System.err); copyInThread(process.getInputStream(),System.out); copyInThread(System.in,process.getOutputStream()); - monitor.setProcess(process); process.waitFor(); - + System.exit(0); // exit JVM when child process ends. return; } @@ -688,11 +683,18 @@ public class Main cmd.addArg(x); } cmd.addRawArg("-Djetty.home=" + _jettyHome); + + // Special Stop/Shutdown properties + ensureSystemPropertySet("STOP.PORT"); + ensureSystemPropertySet("STOP.KEY"); + + // System Properties for (String p : _sysProps) { String v = System.getProperty(p); cmd.addEqualsArg("-D" + p,v); } + cmd.addArg("-cp"); cmd.addRawArg(classpath.toString()); cmd.addRawArg(_config.getMainClassname()); @@ -715,6 +717,34 @@ public class Main return cmd; } + /** + * Ensure that the System Properties are set (if defined as a System property, or start.config property, or + * start.ini property) + * + * @param key + * the key to be sure of + */ + private void ensureSystemPropertySet(String key) + { + if (_sysProps.contains(key)) + { + return; // done + } + + Properties props = Config.getProperties(); + if (props.containsKey(key)) + { + String val = props.getProperty(key,null); + if (val == null) + { + return; // no value to set + } + // setup system property + _sysProps.add(key); + System.setProperty(key,val); + } + } + private String findJavaBin() { File javaHome = new File(System.getProperty("java.home")); @@ -927,10 +957,10 @@ public class Main /** * Load Configuration. - * - * No specific configuration is real until a {@link Config#getCombinedClasspath(java.util.Collection)} is used to execute the {@link Class} specified by - * {@link Config#getMainClassname()} is executed. - * + * + * No specific configuration is real until a {@link Config#getCombinedClasspath(java.util.Collection)} is used to + * execute the {@link Class} specified by {@link Config#getMainClassname()} is executed. + * * @param xmls * the command line specified xml configuration options. * @return the list of xml configurations arriving via command line and start.config choices. @@ -1007,11 +1037,10 @@ public class Main */ public void stop(int port, String key) { - stop (port,key, 0); + stop(port,key,0); } - - public void stop (int port, String key, int timeout) + public void stop(int port, String key, int timeout) { int _port = port; String _key = key; @@ -1031,7 +1060,7 @@ public class Main Socket s = new Socket(InetAddress.getByName("127.0.0.1"),_port); if (timeout > 0) - s.setSoTimeout(timeout*1000); + s.setSoTimeout(timeout * 1000); try { OutputStream out = s.getOutputStream(); @@ -1040,7 +1069,7 @@ public class Main if (timeout > 0) { - System.err.printf("Waiting %,d seconds for jetty to stop%n", timeout); + System.err.printf("Waiting %,d seconds for jetty to stop%n",timeout); LineNumberReader lin = new LineNumberReader(new InputStreamReader(s.getInputStream())); String response; while ((response = lin.readLine()) != null) diff --git a/jetty-start/src/main/java/org/eclipse/jetty/start/Monitor.java b/jetty-start/src/main/java/org/eclipse/jetty/start/Monitor.java deleted file mode 100644 index 1f5667b09af..00000000000 --- a/jetty-start/src/main/java/org/eclipse/jetty/start/Monitor.java +++ /dev/null @@ -1,157 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2012 Mort Bay Consulting Pty. Ltd. -// ------------------------------------------------------------------------ -// All rights reserved. This program and the accompanying materials -// are made available under the terms of the Eclipse Public License v1.0 -// and Apache License v2.0 which accompanies this distribution. -// -// The Eclipse Public License is available at -// http://www.eclipse.org/legal/epl-v10.html -// -// The Apache License v2.0 is available at -// http://www.opensource.org/licenses/apache2.0.php -// -// You may elect to redistribute this code under either of these licenses. -// ======================================================================== -// - -package org.eclipse.jetty.start; -import java.io.InputStreamReader; -import java.io.LineNumberReader; -import java.net.InetAddress; -import java.net.ServerSocket; -import java.net.Socket; - -/*-------------------------------------------*/ -/** Monitor thread. - * This thread listens on the port specified by the STOP.PORT system parameter - * (defaults to -1 for not listening) for request authenticated with the key given by the STOP.KEY - * system parameter (defaults to "eclipse") for admin requests. - *

- * If the stop port is set to zero, then a random port is assigned and the port number - * is printed to stdout. - *

- * Commands "stop" and * "status" are currently supported. - * - */ -public class Monitor extends Thread -{ - private Process _process; - private final int _port; - private final String _key; - - ServerSocket _socket; - - public Monitor(int port,String key) - { - try - { - if(port<0) - return; - setDaemon(true); - setName("StopMonitor"); - _socket=new ServerSocket(port,1,InetAddress.getByName("127.0.0.1")); - if (port==0) - { - port=_socket.getLocalPort(); - System.out.println(port); - } - - if (key==null) - { - key=Long.toString((long)(Long.MAX_VALUE*Math.random()+this.hashCode()+System.currentTimeMillis()),36); - System.out.println("STOP.KEY="+key); - } - } - catch(Exception e) - { - Config.debug(e); - System.err.println("Error binding monitor port "+port+": "+e.toString()); - } - finally - { - _port=port; - _key=key; - } - - if (_socket!=null) - this.start(); - else - System.err.println("WARN: Not listening on monitor port: "+_port); - } - - public Process getProcess() - { - return _process; - } - - public void setProcess(Process process) - { - _process = process; - } - - @Override - public void run() - { - while (true) - { - Socket socket=null; - try{ - socket=_socket.accept(); - - LineNumberReader lin= - new LineNumberReader(new InputStreamReader(socket.getInputStream())); - String key=lin.readLine(); - if (!_key.equals(key)) - { - System.err.println("Ignoring command with incorrect key"); - continue; - } - - String cmd=lin.readLine(); - Config.debug("command=" + cmd); - if ("stop".equals(cmd)) - { - Config.debug("Child Process: " + _process); - if (_process!=null) - { - //if we have a child process, wait for it to finish before we stop - try - { - _process.destroy(); - _process.waitFor(); - } - catch (InterruptedException e) - { - System.err.println("Interrupted waiting for child to terminate"); - } - } - socket.getOutputStream().write("Stopped\r\n".getBytes()); - try {socket.close();}catch(Exception e){e.printStackTrace();} - try {_socket.close();}catch(Exception e){e.printStackTrace();} - System.exit(0); - } - else if ("status".equals(cmd)) - { - socket.getOutputStream().write("OK\r\n".getBytes()); - socket.getOutputStream().flush(); - } - } - catch(Exception e) - { - Config.debug(e); - System.err.println(e.toString()); - } - finally - { - if (socket!=null) - { - try{socket.close();}catch(Exception e){} - } - socket=null; - } - } - } - -} From 979d6dbbf9416b1a0ad965e2b8a3b11a2d208627 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Sat, 8 Dec 2012 10:03:50 +1100 Subject: [PATCH 05/14] 367638 throw exception for excess form keys --- .../java/org/eclipse/jetty/util/UrlEncoded.java | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java b/jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java index 05bd29686a7..b1a47f6c3a3 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/UrlEncoded.java @@ -224,10 +224,7 @@ public class UrlEncoded extends MultiMap implements Cloneable key = null; value=null; if (maxKeys>0 && map.size()>maxKeys) - { - LOG.warn("maxFormKeys limit exceeded keys>{}",maxKeys); - return; - } + throw new IllegalStateException("Form too many keys"); break; case '=': if (key!=null) @@ -396,10 +393,7 @@ public class UrlEncoded extends MultiMap implements Cloneable key = null; value=null; if (maxKeys>0 && map.size()>maxKeys) - { - LOG.warn("maxFormKeys limit exceeded keys>{}",maxKeys); - return; - } + throw new IllegalStateException("Form too many keys"); break; case '=': @@ -483,10 +477,7 @@ public class UrlEncoded extends MultiMap implements Cloneable key = null; value=null; if (maxKeys>0 && map.size()>maxKeys) - { - LOG.warn("maxFormKeys limit exceeded keys>{}",maxKeys); - return; - } + throw new IllegalStateException("Form too many keys"); break; case '=': @@ -611,6 +602,8 @@ public class UrlEncoded extends MultiMap implements Cloneable } key = null; value=null; + if (maxKeys>0 && map.size()>maxKeys) + throw new IllegalStateException("Form too many keys"); break; case '=': if (key!=null) From b1a7779bd22db5e6c41e6f756e8153771684465c Mon Sep 17 00:00:00 2001 From: Thomas Becker Date: Wed, 12 Dec 2012 19:27:31 +0100 Subject: [PATCH 06/14] 393220 remove dead code from ServletHandler and log ServletExceptions in warn instead of debug --- .../org/eclipse/jetty/servlet/ServletHandler.java | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java index b0812c544b1..9614a778e0a 100644 --- a/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java +++ b/jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java @@ -29,10 +29,8 @@ import java.util.Queue; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.ConcurrentMap; - import javax.servlet.Filter; import javax.servlet.FilterChain; -import javax.servlet.RequestDispatcher; import javax.servlet.Servlet; import javax.servlet.ServletContext; import javax.servlet.ServletException; @@ -479,18 +477,11 @@ public class ServletHandler extends ScopedHandler } else if (th instanceof ServletException) { - LOG.debug(th); + LOG.warn(th); Throwable cause=((ServletException)th).getRootCause(); if (cause!=null) th=cause; } - else if (th instanceof RuntimeIOException) - { - LOG.debug(th); - Throwable cause=(IOException)((RuntimeIOException)th).getCause(); - if (cause!=null) - th=cause; - } // handle or log exception if (th instanceof HttpException) From a47827182853292f13facdfe683b2d6d91412429 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 13 Dec 2012 07:59:21 +1100 Subject: [PATCH 07/14] JETTY-1533 handle URL with no path --- .../jetty/server/AbstractHttpConnection.java | 3 ++ .../jetty/webapp/WebAppContextTest.java | 32 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractHttpConnection.java b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractHttpConnection.java index 379906b12da..55d92d6be35 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractHttpConnection.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AbstractHttpConnection.java @@ -467,7 +467,10 @@ public abstract class AbstractHttpConnection extends AbstractConnection if (info==null && !_request.getMethod().equals(HttpMethods.CONNECT)) { if (_uri.getScheme()!=null && _uri.getHost()!=null) + { info="/"; + _request.setRequestURI(""); + } else throw new HttpException(400); } diff --git a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java index c6918c2cd87..af27b996a56 100644 --- a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java +++ b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java @@ -35,7 +35,9 @@ import javax.servlet.ServletResponse; import junit.framework.Assert; +import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.handler.ContextHandlerCollection; import org.eclipse.jetty.server.handler.HandlerList; import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.ResourceCollection; @@ -185,6 +187,36 @@ public class WebAppContextTest assertFalse(context.isProtectedTarget("/something-else/web-inf")); } + + @Test + public void testNullPath() throws Exception + { + Server server = new Server(0); + HandlerList handlers = new HandlerList(); + ContextHandlerCollection contexts = new ContextHandlerCollection(); + WebAppContext context = new WebAppContext(); + context.setBaseResource(Resource.newResource("./src/test/webapp")); + context.setContextPath("/"); + server.setHandler(handlers); + handlers.addHandler(contexts); + contexts.addHandler(context); + + LocalConnector connector = new LocalConnector(); + server.addConnector(connector); + + server.start(); + try + { + String response = connector.getResponses("GET http://localhost:8080 HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n"); + Assert.assertTrue(response.indexOf("200 OK")>=0); + } + finally + { + server.stop(); + } + } + + class ServletA extends GenericServlet { @Override From aacc8a1712bb1a55102ea5bd3da24a2e9b118905 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Fri, 14 Dec 2012 11:02:44 +1100 Subject: [PATCH 08/14] 396459 Log specific message for empty request body for multipart mime requests --- .../jetty/servlets/MultiPartFilter.java | 8 ++- .../jetty/servlets/MultipartFilterTest.java | 70 +++++++++++++++++++ .../jetty/util/ReadLineInputStream.java | 4 ++ 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/MultiPartFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/MultiPartFilter.java index e54d4b26877..f1855c3406f 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/MultiPartFilter.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/MultiPartFilter.java @@ -146,10 +146,12 @@ public class MultiPartFilter implements Filter { // Get first boundary String line=((ReadLineInputStream)in).readLine(); - if(line==null || !line.equals(boundary)) - { + + if (line == null || line.length() == 0) + throw new IOException("Missing content for multipart request"); + + if (!line.equals(boundary)) throw new IOException("Missing initial multi part boundary"); - } // Read each part boolean lastPart=false; diff --git a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/MultipartFilterTest.java b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/MultipartFilterTest.java index a9be27229b5..e61f793c5f3 100644 --- a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/MultipartFilterTest.java +++ b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/MultipartFilterTest.java @@ -472,6 +472,76 @@ public class MultipartFilterTest assertEquals(HttpServletResponse.SC_OK,response.getStatus()); } + + @Test + public void testNoBody() + throws Exception + { + String boundary="XyXyXy"; + // generated and parsed test + HttpTester request = new HttpTester(); + HttpTester response = new HttpTester(); + request.setMethod("POST"); + request.setVersion("HTTP/1.0"); + request.setHeader("Host","tester"); + request.setURI("/context/dump"); + request.setHeader("Content-Type","multipart/form-data; boundary="+boundary); + + response.parse(tester.getResponses(request.generate())); + assertTrue(response.getMethod()==null); + assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, response.getStatus()); + assertTrue(response.getReason().startsWith("Missing content")); + } + + @Test + public void testWhitespaceBodyWithCRLF() + throws Exception + { + String whitespace = " \n\n\n\r\n\r\n\r\n\r\n"; + + String boundary="XyXyXy"; + // generated and parsed test + HttpTester request = new HttpTester(); + HttpTester response = new HttpTester(); + request.setMethod("POST"); + request.setVersion("HTTP/1.0"); + request.setHeader("Host","tester"); + request.setURI("/context/dump"); + request.setHeader("Content-Type","multipart/form-data; boundary="+boundary); + request.setContent(whitespace); + + response.parse(tester.getResponses(request.generate())); + assertTrue(response.getMethod()==null); + assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, response.getStatus()); + assertTrue(response.getReason().startsWith("Missing initial")); + } + + + @Test + public void testWhitespaceBody() + throws Exception + { + String whitespace = " "; + + String boundary="XyXyXy"; + // generated and parsed test + HttpTester request = new HttpTester(); + HttpTester response = new HttpTester(); + request.setMethod("POST"); + request.setVersion("HTTP/1.0"); + request.setHeader("Host","tester"); + request.setURI("/context/dump"); + request.setHeader("Content-Type","multipart/form-data; boundary="+boundary); + request.setContent(whitespace); + + response.parse(tester.getResponses(request.generate())); + assertTrue(response.getMethod()==null); + assertEquals(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, response.getStatus()); + assertTrue(response.getReason().startsWith("Missing initial")); + } + + + /* * see the testParameterMap test diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ReadLineInputStream.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ReadLineInputStream.java index 035d5174ed4..366c6695cbd 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/ReadLineInputStream.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ReadLineInputStream.java @@ -51,7 +51,11 @@ public class ReadLineInputStream extends BufferedInputStream int b=super.read(); if (b==-1) { + int m=markpos; markpos=-1; + if (pos>m) + return new String(buf,m,pos-m,StringUtil.__UTF8); + return null; } From 43c9bba86d076cae33f4fa21a3effd127c03ae67 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Mon, 17 Dec 2012 18:40:50 +1100 Subject: [PATCH 09/14] 393158 java.lang.IllegalStateException when sending an empty InputStream --- .../jetty/client/AsyncHttpConnection.java | 2 + .../jetty/servlets/TransparentProxyTest.java | 140 ++++++++++++++++++ 2 files changed, 142 insertions(+) create mode 100644 jetty-servlets/src/test/java/org/eclipse/jetty/servlets/TransparentProxyTest.java diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/AsyncHttpConnection.java b/jetty-client/src/main/java/org/eclipse/jetty/client/AsyncHttpConnection.java index dbb75d897cd..f746237a619 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/AsyncHttpConnection.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/AsyncHttpConnection.java @@ -106,6 +106,8 @@ public class AsyncHttpConnection extends AbstractHttpConnection implements Async LOG.debug("complete {}",exchange); progress=true; _generator.complete(); + if (exchange.getStatus() < HttpExchange.STATUS_WAITING_FOR_RESPONSE) + exchange.setStatus(HttpExchange.STATUS_WAITING_FOR_RESPONSE); } else if (_generator.isEmpty()) { diff --git a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/TransparentProxyTest.java b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/TransparentProxyTest.java new file mode 100644 index 00000000000..5e19f1460ab --- /dev/null +++ b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/TransparentProxyTest.java @@ -0,0 +1,140 @@ +// +// ======================================================================== +// Copyright (c) 1995-2012 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + + +package org.eclipse.jetty.servlets; + + +import static org.junit.Assert.assertEquals; + +import java.io.IOException; +import java.net.HttpURLConnection; +import java.net.URL; + +import javax.servlet.ServletException; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.eclipse.jetty.server.Server; +import org.eclipse.jetty.server.nio.SelectChannelConnector; +import org.eclipse.jetty.servlet.ServletContextHandler; +import org.eclipse.jetty.servlet.ServletHolder; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + + +/** + * TransparentProxyTest + * + * + */ +public class TransparentProxyTest +{ + + + protected Server server; + protected Server proxyServer; + + public static class ServletA extends HttpServlet { + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { + resp.setContentType("text/plain"); + resp.getWriter().println("ok"); + } + } + + @Before + public void setUp () throws Exception + { + //set up the target server + server = new Server(); + SelectChannelConnector connector = new SelectChannelConnector(); + connector.setPort(8080); + server.addConnector(connector); + ServletContextHandler handler = new ServletContextHandler(server, "/"); + handler.addServlet(ServletA.class, "/a"); + server.setHandler(handler); + server.start(); + + + //set up the server that proxies to the target server + proxyServer = new Server(); + SelectChannelConnector proxyConnector = new SelectChannelConnector(); + proxyConnector.setPort(8081); + proxyServer.addConnector(proxyConnector); + ServletContextHandler proxyHandler = new ServletContextHandler(proxyServer, "/"); + proxyHandler.addServlet(new ServletHolder(new ProxyServlet.Transparent("/", "http", "127.0.0.1", 8080, "/")), "/"); + proxyServer.setHandler(proxyHandler); + proxyServer.start(); + + } + + + @After + public void tearDown() throws Exception + { + server.stop(); + proxyServer.stop(); + } + + + @Test + public void testDirectNoContentType() throws Exception + { + // Direct request without Content-Type set works + URL url = new URL("http://localhost:8080/a"); + HttpURLConnection con = (HttpURLConnection) url.openConnection(); + assertEquals(200, con.getResponseCode()); + } + + + @Test + public void testDirectWithContentType() throws Exception + { + // Direct request with Content-Type works + URL url = new URL("http://localhost:8080/a"); + HttpURLConnection con = (HttpURLConnection) url.openConnection(); + con.addRequestProperty("Content-Type", "application/x-www-form-urlencoded;charset=utf-8"); + assertEquals(200, con.getResponseCode()); + } + + @Test + public void testProxiedWithoutContentType() throws Exception + { + // Proxied request without Content-Type set works + URL url = new URL("http://localhost:8081/a"); + HttpURLConnection con = (HttpURLConnection) url.openConnection(); + assertEquals(200, con.getResponseCode()); + System.err.println (con.getContentType()); + } + + @Test + public void testProxiedWithContentType() throws Exception + { + // Proxied request with Content-Type set fails + + URL url = new URL("http://localhost:8081/a"); + HttpURLConnection con = (HttpURLConnection) url.openConnection(); + con.addRequestProperty("Content-Type", "application/x-www-form-urlencoded;charset=utf-8"); + assertEquals(200, con.getResponseCode()); + System.err.println(con.getContentType()); + + } +} From 128cf0a6b4c9beb3953dfc71bc8087b0f599da67 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 18 Dec 2012 18:19:17 -0700 Subject: [PATCH 10/14] 396886 - MultiPartFilter strips bad escaping on filename="..." --- .../jetty/servlets/MultiPartFilter.java | 13 ++-- .../jetty/servlets/MultipartFilterTest.java | 60 ++++++++++++++++--- .../jetty/util/QuotedStringTokenizer.java | 15 +++++ .../jetty/util/QuotedStringTokenizerTest.java | 21 ++++++- .../eclipse/jetty/testing/ServletTester.java | 6 ++ 5 files changed, 101 insertions(+), 14 deletions(-) diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/MultiPartFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/MultiPartFilter.java index f1855c3406f..36392aa0deb 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/MultiPartFilter.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/MultiPartFilter.java @@ -18,7 +18,6 @@ package org.eclipse.jetty.servlets; -import java.io.BufferedInputStream; import java.io.BufferedOutputStream; import java.io.BufferedReader; import java.io.ByteArrayOutputStream; @@ -55,7 +54,8 @@ import org.eclipse.jetty.util.MultiMap; import org.eclipse.jetty.util.QuotedStringTokenizer; import org.eclipse.jetty.util.ReadLineInputStream; import org.eclipse.jetty.util.StringUtil; -import org.eclipse.jetty.util.TypeUtil; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; /* ------------------------------------------------------------ */ /** @@ -79,6 +79,7 @@ import org.eclipse.jetty.util.TypeUtil; */ public class MultiPartFilter implements Filter { + private static final Logger LOG = Log.getLogger(MultiPartFilter.class); public final static String CONTENT_TYPE_SUFFIX=".org.eclipse.jetty.servlet.contentType"; private final static String FILES ="org.eclipse.jetty.servlet.MultiPartFilter.files"; private File tempdir; @@ -197,7 +198,8 @@ public class MultiPartFilter implements Filter throw new IOException("Missing content-disposition"); } - QuotedStringTokenizer tok=new QuotedStringTokenizer(content_disposition,";"); + LOG.debug("Content-Disposition: {}", content_disposition); + QuotedStringTokenizer tok=new QuotedStringTokenizer(content_disposition,";",false,true); String name=null; String filename=null; while(tok.hasMoreTokens()) @@ -233,6 +235,7 @@ public class MultiPartFilter implements Filter { if (filename!=null && filename.length()>0) { + LOG.debug("filename = \"{}\"", filename); file = File.createTempFile("MultiPart", "", tempdir); out = new FileOutputStream(file); if(_fileOutputBuffer>0) @@ -409,7 +412,9 @@ public class MultiPartFilter implements Filter /* ------------------------------------------------------------ */ private String value(String nameEqualsValue) { - return nameEqualsValue.substring(nameEqualsValue.indexOf('=')+1).trim(); + int idx = nameEqualsValue.indexOf('='); + String value = nameEqualsValue.substring(idx+1).trim(); + return QuotedStringTokenizer.unquoteOnly(value); } /* ------------------------------------------------------------------------------- */ diff --git a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/MultipartFilterTest.java b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/MultipartFilterTest.java index e61f793c5f3..e7f771d0cef 100644 --- a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/MultipartFilterTest.java +++ b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/MultipartFilterTest.java @@ -18,13 +18,13 @@ package org.eclipse.jetty.servlets; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.assertNotNull; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; import java.io.File; import java.io.FileInputStream; import java.io.IOException; +import java.io.PrintWriter; import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; @@ -36,7 +36,6 @@ import org.eclipse.jetty.servlet.FilterMapping; import org.eclipse.jetty.testing.HttpTester; import org.eclipse.jetty.testing.ServletTester; import org.eclipse.jetty.util.IO; -import org.eclipse.jetty.util.QuotedStringTokenizer; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -197,6 +196,42 @@ public class MultipartFilterTest assertEquals(HttpServletResponse.SC_OK,response.getStatus()); assertTrue(response.getContent().indexOf("brown cow")>=0); } + + @Test + public void testBadlyEncodedFilename() throws Exception + { + // generated and parsed test + HttpTester request = new HttpTester(); + HttpTester response = new HttpTester(); + + // test GET + request.setMethod("POST"); + request.setVersion("HTTP/1.0"); + request.setHeader("Host","tester"); + request.setURI("/context/dump"); + + String boundary="XyXyXy"; + request.setHeader("Content-Type","multipart/form-data; boundary="+boundary); + + + String content = "--" + boundary + "\r\n"+ + "Content-Disposition: form-data; name=\"fileup\"; filename=\"Taken on Aug 22 \\ 2012.jpg\"\r\n"+ + "Content-Type: application/octet-stream\r\n\r\n"+ + "How now brown cow."+ + "\r\n--" + boundary + "--\r\n\r\n"; + + request.setContent(content); + + response.parse(tester.getResponses(request.generate())); + + System.out.printf("Content: [%s]%n", response.getContent()); + + assertThat(response.getMethod(), nullValue()); + assertThat(response.getStatus(), is(HttpServletResponse.SC_OK)); + + assertThat(response.getContent(), containsString("Filename [Taken on Aug 22 \\ 2012.jpg]")); + assertThat(response.getContent(), containsString("How now brown cow.")); + } /* * Test multipart with parts encoded in base64 (RFC1521 section 5) @@ -553,7 +588,9 @@ public class MultipartFilterTest @Override protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { - assertEquals("How now brown cow.", req.getParameterMap().get("strupContent-Type: application/octet-stream")); + String content = (String)req.getParameterMap().get("\"strup\"Content-Type: application/octet-stream"); + + assertThat(content, containsString("How now brown cow.")); super.doPost(req, resp); } @@ -613,8 +650,17 @@ public class MultipartFilterTest @Override protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { - resp.getWriter().println((IO.toString(new FileInputStream((File)req.getAttribute("fileup"))))); + FileInputStream in = null; + try { + File file = (File)req.getAttribute("fileup"); + in = new FileInputStream(file); + + PrintWriter out = resp.getWriter(); + out.printf("Filename [%s]\r\n", req.getParameter("fileup")); + out.println(IO.toString(in)); + } finally { + IO.close(in); + } } - } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/QuotedStringTokenizer.java b/jetty-util/src/main/java/org/eclipse/jetty/util/QuotedStringTokenizer.java index 8dc0a816f8f..1dc416ec88a 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/QuotedStringTokenizer.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/QuotedStringTokenizer.java @@ -435,6 +435,10 @@ public class QuotedStringTokenizer if (escape) { escape=false; + if (!isValidEscaping(c)) + { + b.append('\\'); + } b.append(c); } else if (c=='\\') @@ -512,6 +516,10 @@ public class QuotedStringTokenizer ); break; default: + if (!isValidEscaping(c)) + { + b.append('\\'); + } b.append(c); } } @@ -527,6 +535,13 @@ public class QuotedStringTokenizer return b.toString(); } + + private static boolean isValidEscaping(char c) + { + return ((c == 'n') || (c == 'r') || (c == 't') || + (c == 'f') || (c == 'b') || (c == '\\') || + (c == '/') || (c == '"') || (c == 'u')); + } /* ------------------------------------------------------------ */ /** diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/QuotedStringTokenizerTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/QuotedStringTokenizerTest.java index 3407574bea9..989dc55cb33 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/QuotedStringTokenizerTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/QuotedStringTokenizerTest.java @@ -18,9 +18,7 @@ package org.eclipse.jetty.util; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; import org.junit.Test; @@ -190,4 +188,21 @@ public class QuotedStringTokenizerTest assertEquals("ba\\uXXXXaaa", QuotedStringTokenizer.unquoteOnly("\"ba\\\\uXXXXaaa\"")); } + /** + * When encountering a Content-Disposition line during a multi-part mime file + * upload, the filename="..." field can contain '\' characters that do not + * belong to a proper escaping sequence, this tests QuotedStringTokenizer to + * ensure that it preserves those slashes for where they cannot be escaped. + */ + @Test + public void testNextTokenOnContentDisposition() + { + String content_disposition = "form-data; name=\"fileup\"; filename=\"Taken on Aug 22 \\ 2012.jpg\""; + + QuotedStringTokenizer tok=new QuotedStringTokenizer(content_disposition,";",false,true); + + assertEquals("form-data", tok.nextToken().trim()); + assertEquals("name=\"fileup\"", tok.nextToken().trim()); + assertEquals("filename=\"Taken on Aug 22 \\ 2012.jpg\"", tok.nextToken().trim()); + } } diff --git a/test-jetty-servlet/src/main/java/org/eclipse/jetty/testing/ServletTester.java b/test-jetty-servlet/src/main/java/org/eclipse/jetty/testing/ServletTester.java index bd07b9cdb88..7df550a6850 100644 --- a/test-jetty-servlet/src/main/java/org/eclipse/jetty/testing/ServletTester.java +++ b/test-jetty-servlet/src/main/java/org/eclipse/jetty/testing/ServletTester.java @@ -97,6 +97,12 @@ public class ServletTester _server.start(); } + /* ------------------------------------------------------------ */ + public void join() throws Exception + { + _server.join(); + } + /* ------------------------------------------------------------ */ public void stop() throws Exception { From 720263151596d37014013c1a0033a8dc0fab9d5e Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Thu, 20 Dec 2012 13:48:45 +1100 Subject: [PATCH 11/14] 396886 MultiPartFilter strips bad escaping on filename="..." --- .../jetty/servlets/MultiPartFilter.java | 30 +++++++- .../jetty/servlets/MultipartFilterTest.java | 75 ++++++++++++++++++- .../jetty/util/QuotedStringTokenizer.java | 32 ++++++-- 3 files changed, 130 insertions(+), 7 deletions(-) diff --git a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/MultiPartFilter.java b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/MultiPartFilter.java index 36392aa0deb..b2ac85b3051 100644 --- a/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/MultiPartFilter.java +++ b/jetty-servlets/src/main/java/org/eclipse/jetty/servlets/MultiPartFilter.java @@ -211,7 +211,7 @@ public class MultiPartFilter implements Filter else if(tl.startsWith("name=")) name=value(t); else if(tl.startsWith("filename=")) - filename=value(t); + filename=filenameValue(t); } // Check disposition @@ -416,6 +416,34 @@ public class MultiPartFilter implements Filter String value = nameEqualsValue.substring(idx+1).trim(); return QuotedStringTokenizer.unquoteOnly(value); } + + + /* ------------------------------------------------------------ */ + private String filenameValue(String nameEqualsValue) + { + int idx = nameEqualsValue.indexOf('='); + String value = nameEqualsValue.substring(idx+1).trim(); + + if (value.matches(".??[a-z,A-Z]\\:\\\\[^\\\\].*")) + { + //incorrectly escaped IE filenames that have the whole path + //we just strip any leading & trailing quotes and leave it as is + char first=value.charAt(0); + if (first=='"' || first=='\'') + value=value.substring(1); + char last=value.charAt(value.length()-1); + if (last=='"' || last=='\'') + value = value.substring(0,value.length()-1); + + return value; + } + else + //unquote the string, but allow any backslashes that don't + //form a valid escape sequence to remain as many browsers + //even on *nix systems will not escape a filename containing + //backslashes + return QuotedStringTokenizer.unquoteOnly(value, true); + } /* ------------------------------------------------------------------------------- */ /** diff --git a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/MultipartFilterTest.java b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/MultipartFilterTest.java index e7f771d0cef..19e144c1fc4 100644 --- a/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/MultipartFilterTest.java +++ b/jetty-servlets/src/test/java/org/eclipse/jetty/servlets/MultipartFilterTest.java @@ -224,7 +224,7 @@ public class MultipartFilterTest response.parse(tester.getResponses(request.generate())); - System.out.printf("Content: [%s]%n", response.getContent()); + //System.out.printf("Content: [%s]%n", response.getContent()); assertThat(response.getMethod(), nullValue()); assertThat(response.getStatus(), is(HttpServletResponse.SC_OK)); @@ -232,7 +232,80 @@ public class MultipartFilterTest assertThat(response.getContent(), containsString("Filename [Taken on Aug 22 \\ 2012.jpg]")); assertThat(response.getContent(), containsString("How now brown cow.")); } + + @Test + public void testBadlyEncodedMSFilename() throws Exception + { + // generated and parsed test + HttpTester request = new HttpTester(); + HttpTester response = new HttpTester(); + // test GET + request.setMethod("POST"); + request.setVersion("HTTP/1.0"); + request.setHeader("Host","tester"); + request.setURI("/context/dump"); + + String boundary="XyXyXy"; + request.setHeader("Content-Type","multipart/form-data; boundary="+boundary); + + + String content = "--" + boundary + "\r\n"+ + "Content-Disposition: form-data; name=\"fileup\"; filename=\"c:\\this\\really\\is\\some\\path\\to\\a\\file.txt\"\r\n"+ + "Content-Type: application/octet-stream\r\n\r\n"+ + "How now brown cow."+ + "\r\n--" + boundary + "--\r\n\r\n"; + + request.setContent(content); + + response.parse(tester.getResponses(request.generate())); + + //System.out.printf("Content: [%s]%n", response.getContent()); + + assertThat(response.getMethod(), nullValue()); + assertThat(response.getStatus(), is(HttpServletResponse.SC_OK)); + + assertThat(response.getContent(), containsString("Filename [c:\\this\\really\\is\\some\\path\\to\\a\\file.txt]")); + assertThat(response.getContent(), containsString("How now brown cow.")); + } + + @Test + public void testCorrectlyEncodedMSFilename() throws Exception + { + // generated and parsed test + HttpTester request = new HttpTester(); + HttpTester response = new HttpTester(); + + // test GET + request.setMethod("POST"); + request.setVersion("HTTP/1.0"); + request.setHeader("Host","tester"); + request.setURI("/context/dump"); + + String boundary="XyXyXy"; + request.setHeader("Content-Type","multipart/form-data; boundary="+boundary); + + + String content = "--" + boundary + "\r\n"+ + "Content-Disposition: form-data; name=\"fileup\"; filename=\"c:\\\\this\\\\really\\\\is\\\\some\\\\path\\\\to\\\\a\\\\file.txt\"\r\n"+ + "Content-Type: application/octet-stream\r\n\r\n"+ + "How now brown cow."+ + "\r\n--" + boundary + "--\r\n\r\n"; + + request.setContent(content); + + response.parse(tester.getResponses(request.generate())); + + //System.out.printf("Content: [%s]%n", response.getContent()); + + assertThat(response.getMethod(), nullValue()); + assertThat(response.getStatus(), is(HttpServletResponse.SC_OK)); + + assertThat(response.getContent(), containsString("Filename [c:\\this\\really\\is\\some\\path\\to\\a\\file.txt]")); + assertThat(response.getContent(), containsString("How now brown cow.")); + } + + /* * Test multipart with parts encoded in base64 (RFC1521 section 5) */ diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/QuotedStringTokenizer.java b/jetty-util/src/main/java/org/eclipse/jetty/util/QuotedStringTokenizer.java index 1dc416ec88a..54ee74fe339 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/QuotedStringTokenizer.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/QuotedStringTokenizer.java @@ -409,12 +409,21 @@ public class QuotedStringTokenizer } } + + /* ------------------------------------------------------------ */ + public static String unquoteOnly(String s) + { + return unquoteOnly(s, false); + } + + /* ------------------------------------------------------------ */ /** Unquote a string, NOT converting unicode sequences * @param s The string to unquote. + * @param lenient if true, will leave in backslashes that aren't valid escapes * @return quoted string */ - public static String unquoteOnly(String s) + public static String unquoteOnly(String s, boolean lenient) { if (s==null) return null; @@ -435,7 +444,7 @@ public class QuotedStringTokenizer if (escape) { escape=false; - if (!isValidEscaping(c)) + if (lenient && !isValidEscaping(c)) { b.append('\\'); } @@ -453,13 +462,19 @@ public class QuotedStringTokenizer return b.toString(); } - + + /* ------------------------------------------------------------ */ + public static String unquote(String s) + { + return unquote(s,false); + } + /* ------------------------------------------------------------ */ /** Unquote a string. * @param s The string to unquote. * @return quoted string */ - public static String unquote(String s) + public static String unquote(String s, boolean lenient) { if (s==null) return null; @@ -516,7 +531,7 @@ public class QuotedStringTokenizer ); break; default: - if (!isValidEscaping(c)) + if (lenient && !isValidEscaping(c)) { b.append('\\'); } @@ -536,6 +551,13 @@ public class QuotedStringTokenizer return b.toString(); } + + /* ------------------------------------------------------------ */ + /** Check that char c (which is preceded by a backslash) is a valid + * escape sequence. + * @param c + * @return + */ private static boolean isValidEscaping(char c) { return ((c == 'n') || (c == 'r') || (c == 't') || From b65973afdd280459b8762678d5c5341abf2680ca Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 20 Dec 2012 15:32:36 -0700 Subject: [PATCH 12/14] Bug 391623 - Add option to --stop to wait for target jetty to stop * Reworked ShutdownMonitor to better support multiple servers + jetty-maven-plugin requirements. --- .../java/org/eclipse/jetty/server/Server.java | 2 +- .../eclipse/jetty/server/ShutdownMonitor.java | 258 ++++++++++++------ 2 files changed, 168 insertions(+), 92 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java index 9fc62e9f57d..7088cac6eb9 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java @@ -263,7 +263,7 @@ public class Server extends HandlerWrapper implements Attributes { if (getStopAtShutdown()) { ShutdownThread.register(this); - ShutdownMonitor.getInstance(); // initialize + ShutdownMonitor.getInstance().start(); // initialize } LOG.info("jetty-"+__version); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java index 25335475957..5bb51f2e6a6 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java @@ -33,8 +33,8 @@ import org.eclipse.jetty.util.thread.ShutdownThread; /** * Shutdown/Stop Monitor thread. *

- * This thread listens on the port specified by the STOP.PORT system parameter (defaults to -1 for not listening) for - * request authenticated with the key given by the STOP.KEY system parameter (defaults to "eclipse") for admin requests. + * This thread listens on the port specified by the STOP.PORT system parameter (defaults to -1 for not listening) for request authenticated with the key given + * by the STOP.KEY system parameter (defaults to "eclipse") for admin requests. *

* If the stop port is set to zero, then a random port is assigned and the port number is printed to stdout. *

@@ -42,83 +42,83 @@ import org.eclipse.jetty.util.thread.ShutdownThread; */ public class ShutdownMonitor extends Thread { - private static final ShutdownMonitor INSTANCE = new ShutdownMonitor(); - + // Implementation of safe lazy init, using Initialization on Demand Holder technique. + static class Holder + { + static ShutdownMonitor instance = new ShutdownMonitor(); + } + public static ShutdownMonitor getInstance() { - return INSTANCE; + return Holder.instance; } - - private final boolean DEBUG; - private final int port; - private final String key; - private final ServerSocket serverSocket; + private boolean DEBUG; + private int port; + private String key; + private boolean exitVm; + private ServerSocket serverSocket; + + /** + * Create a ShutdownMonitor using configuration from the System properties. + *

+ * STOP.PORT = the port to listen on (empty, null, or values less than 0 disable the stop ability)
+ * STOP.KEY = the magic key/passphrase to allow the stop (defaults to "eclipse")
+ *

+ * Note: server socket will only listen on localhost, and a successful stop will issue a System.exit() call. + */ private ShutdownMonitor() { Properties props = System.getProperties(); - // Use the same debug option as /jetty-start/ this.DEBUG = props.containsKey("DEBUG"); - - // Use values passed thru /jetty-start/ - int stopPort = Integer.parseInt(props.getProperty("STOP.PORT","-1")); - String stopKey = props.getProperty("STOP.KEY",null); - ServerSocket sock = null; + // Use values passed thru via /jetty-start/ + this.port = Integer.parseInt(props.getProperty("STOP.PORT","-1")); + this.key = props.getProperty("STOP.KEY","eclipse"); + this.exitVm = true; + } + + private void close(ServerSocket server) + { + if (server == null) + { + return; + } try { - if (stopPort < 0) - { - System.out.println("ShutdownMonitor not in use"); - sock = null; - return; - } - - setDaemon(true); - setName("ShutdownMonitor"); - - sock = new ServerSocket(stopPort,1,InetAddress.getByName("127.0.0.1")); - if (stopPort == 0) - { - // server assigned port in use - stopPort = sock.getLocalPort(); - System.out.printf("STOP.PORT=%d%n",stopPort); - } - - if (stopKey == null) - { - // create random key - stopKey = Long.toString((long)(Long.MAX_VALUE * Math.random() + this.hashCode() + System.currentTimeMillis()),36); - System.out.printf("STOP.KEY=%s%n",stopKey); - } - + server.close(); } - catch (Exception e) + catch (IOException ignore) { - debug(e); - System.err.println("Error binding monitor port " + stopPort + ": " + e.toString()); + /* ignore */ } - finally - { - // establish the port and key that are in use - this.port = stopPort; - this.key = stopKey; - - this.serverSocket = sock; - debug("STOP.PORT=%d", port); - debug("STOP.KEY=%s", key); - debug("%s", serverSocket); - } - - this.start(); } - - @Override - public String toString() + + private void close(Socket socket) { - return String.format("%s[port=%d]",this.getClass().getName(),port); + if (socket == null) + { + return; + } + + try + { + socket.close(); + } + catch (IOException ignore) + { + /* ignore */ + } + } + + private void debug(String format, Object... args) + { + if (DEBUG) + { + System.err.printf("[ShutdownMonitor] " + format + "%n",args); + } } private void debug(Throwable t) @@ -129,12 +129,24 @@ public class ShutdownMonitor extends Thread } } - private void debug(String format, Object... args) + public String getKey() { - if (DEBUG) - { - System.err.printf("[ShutdownMonitor] " + format + "%n",args); - } + return key; + } + + public int getPort() + { + return port; + } + + public ServerSocket getServerSocket() + { + return serverSocket; + } + + public boolean isExitVm() + { + return exitVm; } @Override @@ -169,15 +181,18 @@ public class ShutdownMonitor extends Thread debug("Informing client that we are stopped."); out.write("Stopped\r\n".getBytes(StringUtil.__UTF8)); out.flush(); - + // Shutdown Monitor debug("Shutting down monitor"); close(socket); close(serverSocket); - // Kill JVM - debug("Killing JVM"); - System.exit(0); + if (exitVm) + { + // Kill JVM + debug("Killing JVM"); + System.exit(0); + } } else if ("status".equals(cmd)) { @@ -199,37 +214,98 @@ public class ShutdownMonitor extends Thread } } - private void close(Socket socket) + public void setDebug(boolean flag) { - if (socket == null) + this.DEBUG = flag; + } + + public void setExitVm(boolean exitVm) + { + if (isAlive()) { - return; + throw new IllegalStateException("ShutdownMonitor already started"); } + this.exitVm = exitVm; + } + + public void setKey(String key) + { + if (isAlive()) + { + throw new IllegalStateException("ShutdownMonitor already started"); + } + this.key = key; + } + + public void setPort(int port) + { + if (isAlive()) + { + throw new IllegalStateException("ShutdownMonitor already started"); + } + this.port = port; + } + + public void start() + { + if (isAlive()) + { + System.out.printf("ShutdownMonitor already started"); + return; // cannot start it again + } + startListenSocket(); + super.start(); + } + + private void startListenSocket() + { + ServerSocket sock = null; try { - socket.close(); + if (this.port < 0) + { + System.out.println("ShutdownMonitor not in use (port < 0): " + port); + sock = null; + return; + } + + setDaemon(true); + setName("ShutdownMonitor"); + + sock = new ServerSocket(this.port,1,InetAddress.getByName("127.0.0.1")); + if (this.port == 0) + { + // server assigned port in use + this.port = sock.getLocalPort(); + System.out.printf("STOP.PORT=%d%n",this.port); + } + + if (this.key == null) + { + // create random key + this.key = Long.toString((long)(Long.MAX_VALUE * Math.random() + this.hashCode() + System.currentTimeMillis()),36); + System.out.printf("STOP.KEY=%s%n",this.key); + } } - catch (IOException ignore) + catch (Exception e) { - /* ignore */ + debug(e); + System.err.println("Error binding monitor port " + this.port + ": " + e.toString()); + } + finally + { + // establish the port and key that are in use + this.serverSocket = sock; + debug("STOP.PORT=%d",this.port); + debug("STOP.KEY=%s",this.key); + debug("%s",serverSocket); } } - - private void close(ServerSocket server) - { - if (server == null) - { - return; - } - try - { - server.close(); - } - catch (IOException ignore) - { - /* ignore */ - } + @Override + public String toString() + { + return String.format("%s[port=%d]",this.getClass().getName(),port); } } From a371a8963dd6b4257916bee79653910a13dca1fd Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 20 Dec 2012 17:30:46 -0700 Subject: [PATCH 13/14] Fixing NPE --- .../eclipse/jetty/server/ShutdownMonitor.java | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java index 5bb51f2e6a6..ecd82fc8aef 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java @@ -152,6 +152,11 @@ public class ShutdownMonitor extends Thread @Override public void run() { + if (serverSocket == null) + { + return; + } + while (true) { Socket socket = null; @@ -254,30 +259,32 @@ public class ShutdownMonitor extends Thread return; // cannot start it again } startListenSocket(); + if (serverSocket == null) + { + return; + } + super.start(); } private void startListenSocket() { - ServerSocket sock = null; - try { if (this.port < 0) { System.out.println("ShutdownMonitor not in use (port < 0): " + port); - sock = null; return; } setDaemon(true); setName("ShutdownMonitor"); - sock = new ServerSocket(this.port,1,InetAddress.getByName("127.0.0.1")); + this.serverSocket = new ServerSocket(this.port,1,InetAddress.getByName("127.0.0.1")); if (this.port == 0) { // server assigned port in use - this.port = sock.getLocalPort(); + this.port = serverSocket.getLocalPort(); System.out.printf("STOP.PORT=%d%n",this.port); } @@ -296,7 +303,6 @@ public class ShutdownMonitor extends Thread finally { // establish the port and key that are in use - this.serverSocket = sock; debug("STOP.PORT=%d",this.port); debug("STOP.KEY=%s",this.key); debug("%s",serverSocket); From 5e4711fdb07dc9928efde8e5bcd97e52834658e2 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Thu, 20 Dec 2012 17:32:34 -0700 Subject: [PATCH 14/14] Fixing confusing output --- .../org/eclipse/jetty/server/ShutdownMonitor.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java index ecd82fc8aef..67d23ae2929 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ShutdownMonitor.java @@ -269,14 +269,14 @@ public class ShutdownMonitor extends Thread private void startListenSocket() { + if (this.port < 0) + { + System.out.println("ShutdownMonitor not in use (port < 0): " + port); + return; + } + try { - if (this.port < 0) - { - System.out.println("ShutdownMonitor not in use (port < 0): " + port); - return; - } - setDaemon(true); setName("ShutdownMonitor");