diff --git a/core/pom.xml b/core/pom.xml index 6201159fcf..2ce0995ec8 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -319,7 +319,7 @@ THE SOFTWARE. org.kohsuke.stapler stapler-jelly - 1.108 + 1.109 dom4j diff --git a/core/src/main/java/hudson/PluginManager.java b/core/src/main/java/hudson/PluginManager.java index 0220fd530a..1f498cc6e1 100644 --- a/core/src/main/java/hudson/PluginManager.java +++ b/core/src/main/java/hudson/PluginManager.java @@ -52,6 +52,8 @@ import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.WebApp; +import org.kohsuke.stapler.HttpResponse; +import org.kohsuke.stapler.HttpRedirect; /** * Manages {@link PluginWrapper}s. @@ -344,10 +346,8 @@ public final class PluginManager extends AbstractModelObject { if(n.startsWith("plugin.")) { n = n.substring(7); UpdateCenter.Plugin p = Hudson.getInstance().getUpdateCenter().getPlugin(n); - if(p==null) { - sendError("No such plugin: "+n,req,rsp); - return; - } + if(p==null) + throw new Failure("No such plugin: "+n); p.install(); } } @@ -378,7 +378,7 @@ public final class PluginManager extends AbstractModelObject { /** * Uploads a plugin. */ - public void doUploadPlugin( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException { + public HttpResponse doUploadPlugin(StaplerRequest req) throws IOException, ServletException { try { Hudson.getInstance().checkPermission(Hudson.ADMINISTER); @@ -387,16 +387,14 @@ public final class PluginManager extends AbstractModelObject { // Parse the request FileItem fileItem = (FileItem) upload.parseRequest(req).get(0); String fileName = Util.getFileName(fileItem.getName()); - if(!fileName.endsWith(".hpi")) { - sendError(hudson.model.Messages.Hudson_NotAPlugin(fileName),req,rsp); - return; - } + if(!fileName.endsWith(".hpi")) + throw new Failure(hudson.model.Messages.Hudson_NotAPlugin(fileName)); fileItem.write(new File(rootDir, fileName)); fileItem.delete(); pluginUploaded=true; - rsp.sendRedirect2("."); + return new HttpRedirect("."); } catch (IOException e) { throw e; } catch (Exception e) {// grrr. fileItem.write throws this diff --git a/core/src/main/java/hudson/logging/LogRecorderManager.java b/core/src/main/java/hudson/logging/LogRecorderManager.java index 4761c02513..b864b51362 100644 --- a/core/src/main/java/hudson/logging/LogRecorderManager.java +++ b/core/src/main/java/hudson/logging/LogRecorderManager.java @@ -28,12 +28,15 @@ import hudson.Functions; import hudson.model.AbstractModelObject; import hudson.model.Hudson; import hudson.model.RSS; +import hudson.model.Failure; import hudson.tasks.Mailer; import hudson.util.CopyOnWriteMap; import org.apache.commons.io.filefilter.WildcardFileFilter; import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.StaplerResponse; +import org.kohsuke.stapler.HttpResponse; +import org.kohsuke.stapler.HttpRedirect; import javax.servlet.ServletException; import java.io.File; @@ -96,24 +99,23 @@ public class LogRecorderManager extends AbstractModelObject { /** * Creates a new log recorder. */ - public void doNewLogRecorder( StaplerRequest req, StaplerResponse rsp, @QueryParameter String name) throws IOException, ServletException { + public HttpResponse doNewLogRecorder(@QueryParameter String name) { try { Hudson.checkGoodName(name); } catch (ParseException e) { - sendError(e, req, rsp); - return; + throw new Failure(e.getMessage()); } logRecorders.put(name,new LogRecorder(name)); // redirect to the config screen - rsp.sendRedirect2(name+"/configure"); + return new HttpRedirect(name+"/configure"); } /** * Configure the logging level. */ - public void doConfigLogger(StaplerResponse rsp, @QueryParameter String name, @QueryParameter String level) throws IOException { + public HttpResponse doConfigLogger(@QueryParameter String name, @QueryParameter String level) { Hudson.getInstance().checkPermission(Hudson.ADMINISTER); Level lv; if(level.equals("inherit")) @@ -121,7 +123,7 @@ public class LogRecorderManager extends AbstractModelObject { else lv = Level.parse(level.toUpperCase()); Logger.getLogger(name).setLevel(lv); - rsp.sendRedirect2("all"); + return new HttpRedirect("all"); } /** diff --git a/core/src/main/java/hudson/model/AbstractItem.java b/core/src/main/java/hudson/model/AbstractItem.java index cd086b5df5..89345b090a 100644 --- a/core/src/main/java/hudson/model/AbstractItem.java +++ b/core/src/main/java/hudson/model/AbstractItem.java @@ -242,11 +242,7 @@ public abstract class AbstractItem extends Actionable implements Item, HttpDelet */ public void doDoDelete( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException, InterruptedException { checkPermission(DELETE); - if(!"POST".equals(req.getMethod())) { - rsp.setStatus(SC_BAD_REQUEST); - sendError("Delete request has to be POST",req,rsp); - return; - } + requirePOST(); delete(); rsp.sendRedirect2(req.getContextPath()+"/"+getParent().getUrl()); } diff --git a/core/src/main/java/hudson/model/Computer.java b/core/src/main/java/hudson/model/Computer.java index f031898682..abd2629197 100644 --- a/core/src/main/java/hudson/model/Computer.java +++ b/core/src/main/java/hudson/model/Computer.java @@ -877,32 +877,28 @@ public /*transient*/ abstract class Computer extends Actionable implements Acces /** * Accepts the update to the node configuration. */ - public void doConfigSubmit( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException { - try { - checkPermission(Hudson.ADMINISTER); // TODO: new permission? + public void doConfigSubmit( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException, FormException { + checkPermission(Hudson.ADMINISTER); // TODO: new permission? - final Hudson app = Hudson.getInstance(); + final Hudson app = Hudson.getInstance(); - Node result = getNode().getDescriptor().newInstance(req, req.getSubmittedForm()); + Node result = getNode().getDescriptor().newInstance(req, req.getSubmittedForm()); - // replace the old Node object by the new one - synchronized (app) { - List nodes = new ArrayList(app.getNodes()); - int i = nodes.indexOf(getNode()); - if(i<0) { - sendError("This slave appears to be removed while you were editing the configuration",req,rsp); - return; - } - - nodes.set(i,result); - app.setNodes(nodes); + // replace the old Node object by the new one + synchronized (app) { + List nodes = new ArrayList(app.getNodes()); + int i = nodes.indexOf(getNode()); + if(i<0) { + sendError("This slave appears to be removed while you were editing the configuration",req,rsp); + return; } - // take the user back to the slave top page. - rsp.sendRedirect2("../"+result.getNodeName()+'/'); - } catch (FormException e) { - sendError(e,req,rsp); + nodes.set(i,result); + app.setNodes(nodes); } + + // take the user back to the slave top page. + rsp.sendRedirect2("../"+result.getNodeName()+'/'); } /** diff --git a/core/src/main/java/hudson/model/ComputerSet.java b/core/src/main/java/hudson/model/ComputerSet.java index f7480f03c3..20218badb8 100644 --- a/core/src/main/java/hudson/model/ComputerSet.java +++ b/core/src/main/java/hudson/model/ComputerSet.java @@ -255,8 +255,8 @@ public final class ComputerSet extends AbstractModelObject { * Really creates a new slave. */ public synchronized void doDoCreateItem( StaplerRequest req, StaplerResponse rsp, - @QueryParameter("name") String name, - @QueryParameter("type") String type ) throws IOException, ServletException { + @QueryParameter String name, + @QueryParameter String type ) throws IOException, ServletException, FormException { try { final Hudson app = Hudson.getInstance(); app.checkPermission(Hudson.ADMINISTER); // TODO: new permission? @@ -270,8 +270,6 @@ public final class ComputerSet extends AbstractModelObject { } catch (ParseException e) { rsp.setStatus(SC_BAD_REQUEST); sendError(e,req,rsp); - } catch (FormException e) { - sendError(e,req,rsp); } } @@ -311,7 +309,7 @@ public final class ComputerSet extends AbstractModelObject { /** * Accepts submission from the configuration page. */ - public final synchronized void doConfigSubmit( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException { + public final synchronized void doConfigSubmit( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException, FormException { BulkChange bc = new BulkChange(MONITORS_OWNER); try { Hudson.getInstance().checkPermission(Hudson.ADMINISTER); @@ -325,8 +323,6 @@ public final class ComputerSet extends AbstractModelObject { monitors.add(i); } rsp.sendRedirect2("."); - } catch (FormException e) { - sendError(e,req,rsp); } finally { bc.commit(); } diff --git a/core/src/main/java/hudson/model/Descriptor.java b/core/src/main/java/hudson/model/Descriptor.java index 975e97bef2..4e84e539c3 100644 --- a/core/src/main/java/hudson/model/Descriptor.java +++ b/core/src/main/java/hudson/model/Descriptor.java @@ -34,6 +34,7 @@ import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.Stapler; import org.kohsuke.stapler.StaplerResponse; import org.kohsuke.stapler.Ancestor; +import org.kohsuke.stapler.HttpResponse; import org.springframework.util.StringUtils; import org.jvnet.tiger_types.Types; import org.apache.commons.io.IOUtils; @@ -639,7 +640,7 @@ public abstract class Descriptor> implements Saveable { return find(Hudson.getInstance().getExtensionList(Descriptor.class),className); } - public static final class FormException extends Exception { + public static final class FormException extends Exception implements HttpResponse { private final String formField; public FormException(String message, String formField) { @@ -663,6 +664,11 @@ public abstract class Descriptor> implements Saveable { public String getFormField() { return formField; } + + public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object node) throws IOException, ServletException { + // for now, we can't really use the field name that caused the problem. + new Failure(getMessage()).generateResponse(req,rsp,node); + } } private static final Logger LOGGER = Logger.getLogger(Descriptor.class.getName()); diff --git a/core/src/main/java/hudson/model/Failure.java b/core/src/main/java/hudson/model/Failure.java new file mode 100644 index 0000000000..9962dc7a56 --- /dev/null +++ b/core/src/main/java/hudson/model/Failure.java @@ -0,0 +1,39 @@ +package hudson.model; + +import org.kohsuke.stapler.HttpResponse; +import org.kohsuke.stapler.StaplerRequest; +import org.kohsuke.stapler.StaplerResponse; + +import javax.servlet.ServletException; +import java.io.IOException; + +/** + * Represents an error induced by user, encountered during HTTP request processing. + * + *

+ * The error page is rendered into HTML, but without a stack trace. So only use + * this exception when the error condition is anticipated by the program, and where + * we nor users don't need to see the stack trace to figure out the root cause. + * + * @author Kohsuke Kawaguchi + * @since 1.321 + */ +public class Failure extends RuntimeException implements HttpResponse { + private final boolean pre; + + public Failure(String message) { + this(message,false); + } + + public Failure(String message, boolean pre) { + super(message); + this.pre = pre; + } + + public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object node) throws IOException, ServletException { + req.setAttribute("message",getMessage()); + if(pre) + req.setAttribute("pre",true); + rsp.forward( node instanceof AbstractModelObject ? node : Hudson.getInstance() ,"error",req); + } +} diff --git a/core/src/main/java/hudson/model/Hudson.java b/core/src/main/java/hudson/model/Hudson.java index 3008995bf0..3d8c62b52a 100644 --- a/core/src/main/java/hudson/model/Hudson.java +++ b/core/src/main/java/hudson/model/Hudson.java @@ -2167,7 +2167,7 @@ public final class Hudson extends Node implements ItemGroup, Stapl /** * Accepts submission from the configuration page. */ - public synchronized void doConfigSubmit( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException { + public synchronized void doConfigSubmit( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException, FormException { BulkChange bc = new BulkChange(this); try { checkPermission(ADMINISTER); @@ -2295,8 +2295,6 @@ public final class Hudson extends Node implements ItemGroup, Stapl rsp.sendRedirect(req.getContextPath()+'/'); // go to the top page else rsp.sendRedirect("configure"); // back to config - } catch (FormException e) { - sendError(e,req,rsp); } finally { bc.commit(); } @@ -2521,14 +2519,12 @@ public final class Hudson extends Node implements ItemGroup, Stapl return (T)copy((TopLevelItem)src,name); } - public synchronized void doCreateView( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException { + public synchronized void doCreateView( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException, FormException { try { checkPermission(View.CREATE); addView(View.create(req,rsp, this)); } catch (ParseException e) { sendError(e,req,rsp); - } catch (FormException e) { - sendError(e,req,rsp); } } @@ -2782,11 +2778,19 @@ public final class Hudson extends Node implements ItemGroup, Stapl return; } + restart(); + + rsp.sendRedirect2("."); + } + + /** + * Performs a restart. + */ + public void restart() throws ServletException, IOException { final Lifecycle lifecycle = Lifecycle.get(); if(!lifecycle.canRestart()) - sendError("Restart is not supported in this running mode.",req,rsp); + throw new Failure("Restart is not supported in this running mode."); servletContext.setAttribute("app",new HudsonIsRestarting()); - rsp.sendRedirect2("."); new Thread("restart thread") { @Override diff --git a/core/src/main/java/hudson/model/Job.java b/core/src/main/java/hudson/model/Job.java index a54a5264cf..c710333d02 100644 --- a/core/src/main/java/hudson/model/Job.java +++ b/core/src/main/java/hudson/model/Job.java @@ -898,7 +898,7 @@ public abstract class Job, RunT extends Run, RunT extends Run props = new ArrayList(); - int i = 0; - for (UserPropertyDescriptor d : UserProperty.all()) { - JSONObject o = json.getJSONObject("userProperty" + (i++)); - UserProperty p = getProperty(d.clazz); - if (p != null) { - p = p.reconfigure(req, o); - } else { - p = d.newInstance(req, o); - } - - p.setUser(this); - props.add(p); + List props = new ArrayList(); + int i = 0; + for (UserPropertyDescriptor d : UserProperty.all()) { + JSONObject o = json.getJSONObject("userProperty" + (i++)); + UserProperty p = getProperty(d.clazz); + if (p != null) { + p = p.reconfigure(req, o); + } else { + p = d.newInstance(req, o); } - this.properties = props; - save(); - - rsp.sendRedirect("."); - } catch (FormException e) { - sendError(e,req,rsp); + p.setUser(this); + props.add(p); } + this.properties = props; + + save(); + + rsp.sendRedirect("."); } public void doRssAll( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException { diff --git a/core/src/main/java/hudson/model/View.java b/core/src/main/java/hudson/model/View.java index af1ec2d4c9..efe354b00f 100644 --- a/core/src/main/java/hudson/model/View.java +++ b/core/src/main/java/hudson/model/View.java @@ -469,29 +469,25 @@ public abstract class View extends AbstractModelObject implements AccessControll * * Subtypes should override the {@link #submit(StaplerRequest)} method. */ - public final synchronized void doConfigSubmit( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException { + public final synchronized void doConfigSubmit( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException, FormException { + checkPermission(CONFIGURE); + + req.setCharacterEncoding("UTF-8"); + + submit(req); + + description = Util.nullify(req.getParameter("description")); + try { - checkPermission(CONFIGURE); - - req.setCharacterEncoding("UTF-8"); - - submit(req); - - description = Util.nullify(req.getParameter("description")); - - try { - rename(req.getParameter("name")); - } catch (ParseException e) { - sendError(e, req, rsp); - return; - } - - owner.save(); - - rsp.sendRedirect2("../"+name); - } catch (FormException e) { - sendError(e,req,rsp); + rename(req.getParameter("name")); + } catch (ParseException e) { + sendError(e, req, rsp); + return; } + + owner.save(); + + rsp.sendRedirect2("../"+name); } /**