diff --git a/core/src/main/java/org/kohsuke/stapler/RequestImpl.java b/core/src/main/java/org/kohsuke/stapler/RequestImpl.java index 9796ace1d..5d822ba68 100644 --- a/core/src/main/java/org/kohsuke/stapler/RequestImpl.java +++ b/core/src/main/java/org/kohsuke/stapler/RequestImpl.java @@ -41,6 +41,7 @@ import org.apache.commons.fileupload.disk.DiskFileItemFactory; import org.apache.commons.fileupload.servlet.ServletFileUpload; import org.jvnet.tiger_types.Lister; +import org.kohsuke.stapler.bind.Bound; import org.kohsuke.stapler.bind.BoundObjectTable; import org.kohsuke.stapler.lang.Klass; import org.kohsuke.stapler.lang.MethodRef; @@ -183,6 +184,12 @@ public String createJavaScriptProxy(Object toBeExported) { return getBoundObjectTable().bind(toBeExported).getProxyScript(); } + @Override + public RenderOnDemandParameters createJavaScriptProxyParameters(Object toBeExported) { + final Bound bound = getBoundObjectTable().bind(toBeExported); + return new RenderOnDemandParameters("makeStaplerProxy", bound.getURL(), getWebApp().getCrumbIssuer().issueCrumb(), bound.getBoundJavaScriptUrlNames()); + } + @Override public Stapler getStapler() { return stapler; diff --git a/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java b/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java index 6f7535673..8138849e4 100644 --- a/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java +++ b/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java @@ -23,6 +23,7 @@ package org.kohsuke.stapler; +import java.util.Set; import org.apache.commons.beanutils.BeanUtils; import org.apache.commons.beanutils.ConvertUtils; import org.apache.commons.fileupload.FileItem; @@ -515,6 +516,36 @@ public interface StaplerRequest extends HttpServletRequest { * a proxy on the client side. * * Short cut for {@code getBoundObjectTable().bind(toBeExported).getProxyScript()} + * + * @deprecated Use {@link #createJavaScriptProxyParameters(Object)} and invoke {@code makeStaplerProxy} yourself. */ + @Deprecated String createJavaScriptProxy(Object toBeExported); + + /** + * Return value of {@link #createJavaScriptProxyParameters(Object)} + */ + final class RenderOnDemandParameters { + public final String proxyMethod; + public final String url; + public final String crumb; + public final Set urlNames; + + public RenderOnDemandParameters(String proxyMethod, String url, String crumb, Set urlNames) { + this.proxyMethod = proxyMethod; + this.url = url; + this.crumb = crumb; + this.urlNames = urlNames; + } + + public String getUrlNames() { + return String.join(",", urlNames); + } + } + + /** + * Exports the given Java object as a JavaScript proxy and returns the parameters needed to call + * {@code makeStaplerProxy}. + */ + RenderOnDemandParameters createJavaScriptProxyParameters(Object toBeExported); } diff --git a/core/src/main/java/org/kohsuke/stapler/bind/Bound.java b/core/src/main/java/org/kohsuke/stapler/bind/Bound.java index 299328a22..36b6b7974 100644 --- a/core/src/main/java/org/kohsuke/stapler/bind/Bound.java +++ b/core/src/main/java/org/kohsuke/stapler/bind/Bound.java @@ -23,14 +23,16 @@ package org.kohsuke.stapler.bind; +import java.util.HashSet; +import java.util.Set; +import java.util.stream.Collectors; import org.kohsuke.stapler.HttpResponse; import org.kohsuke.stapler.MetaClass; +import org.kohsuke.stapler.Stapler; import org.kohsuke.stapler.WebApp; import java.lang.reflect.Method; import java.util.Arrays; -import java.util.Collection; -import java.util.Set; /** * Handles to the object bound via {@link BoundObjectTable}. @@ -63,34 +65,78 @@ public abstract class Bound implements HttpResponse { * talks back to the bound object that this handle represents. */ public final String getProxyScript() { - StringBuilder buf = new StringBuilder("makeStaplerProxy('").append(getURL()).append("','").append( - WebApp.getCurrent().getCrumbIssuer().issueCrumb() - ).append("',["); + return getProxyScript(getURL(), getTarget().getClass()); + } + + /** + * Returns the URL to the proxy script for the specified {@link org.kohsuke.stapler.bind.Bound}. + * + * @param variableName the variable to assign to the bound object + * @param bound the bound object, or {@code null} if none. + * @return the URL to the proxy script for the specified {@link org.kohsuke.stapler.bind.Bound}, starting with the context path + */ + public static String getProxyScriptURL(String variableName, Bound bound) { + if (bound == null) { + return Stapler.getCurrentRequest().getContextPath() + BoundObjectTable.SCRIPT_PREFIX + "/null?var=" + variableName; + } else { + return bound.getProxyScriptURL(variableName); + } + } + + /** + * Returns the URL for the standalone proxy script of this {@link org.kohsuke.stapler.bind.Bound}. + * + * @param variableName the name of the JS variable to assign + * @return the URL for the standalone proxy script of this {@link org.kohsuke.stapler.bind.Bound}, starting with the context path + */ + public final String getProxyScriptURL(String variableName) { + final String methodsList = String.join(",", getBoundJavaScriptUrlNames(getTarget().getClass())); + // The URL looks like it has some redundant elements, but only if it's not a WithWellKnownURL + return Stapler.getCurrentRequest().getContextPath() + BoundObjectTable.SCRIPT_PREFIX + getURL() + "?var=" + variableName + "&methods=" + methodsList; + } - boolean first=true; - for (Method m : getTarget().getClass().getMethods()) { - Collection names; + private static Set getBoundJavaScriptUrlNames(Class clazz) { + Set names = new HashSet<>(); + for (Method m : clazz.getMethods()) { if (m.getName().startsWith("js")) { - names = Set.of(camelize(m.getName().substring(2))); + names.add(camelize(m.getName().substring(2))); } else { JavaScriptMethod a = m.getAnnotation(JavaScriptMethod.class); - if (a!=null) { - names = Arrays.asList(a.name()); - if (names.isEmpty()) - names = Set.of(m.getName()); - } else - continue; - } - - for (String n : names) { - if (first) first = false; - else buf.append(','); - buf.append('\'').append(n).append('\''); + if (a != null) { + if (a.name().length == 0) { + names.add(m.getName()); + } else { + names.addAll(Arrays.asList(a.name())); + } + } } } - buf.append("])"); - - return buf.toString(); + return names; + } + + /** + * Returns a collection of all JS bound methods of the target's type. + * @return a collection of all JS bound methods of the target's type + */ + public final Set getBoundJavaScriptUrlNames() { + return getBoundJavaScriptUrlNames(getTarget().getClass()); + } + + public static String getProxyScript(String url, Class clazz) { + return getProxyScript(url, getBoundJavaScriptUrlNames(clazz).toArray(String[]::new)); + } + + /** + * Returns the Stapler proxy script for the specified URL and method names + * + * @param url URL to proxied object + * @param methods list of method names + * @return the Stapler proxy script for the specified URL and method names + */ + public static String getProxyScript(String url, String[] methods) { + final String crumb = WebApp.getCurrent().getCrumbIssuer().issueCrumb(); + final String methodNamesList = Arrays.stream(methods).sorted().map(it -> "'" + it + "'").collect(Collectors.joining(",")); + return "makeStaplerProxy('" + url + "','" + crumb + "',[" + methodNamesList + "])"; } private static String camelize(String name) { diff --git a/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java b/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java index edd394642..2f94c1135 100644 --- a/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java +++ b/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java @@ -24,9 +24,14 @@ package org.kohsuke.stapler.bind; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import java.io.PrintWriter; +import java.util.Arrays; +import java.util.logging.Level; +import org.apache.commons.lang.StringUtils; import org.kohsuke.stapler.Ancestor; import org.kohsuke.stapler.HttpResponse; import org.kohsuke.stapler.HttpResponses; +import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.Stapler; import org.kohsuke.stapler.StaplerFallback; import org.kohsuke.stapler.StaplerRequest; @@ -51,6 +56,93 @@ * @author Kohsuke Kawaguchi */ public class BoundObjectTable implements StaplerFallback { + + public static boolean isValidJavaScriptIdentifier(String variableName) { + // Ultimately this will be used as a JS identifier, so we need (a subset of) what's valid there. + // The primary purpose of this check however is to prevent injection attacks. + return variableName.matches("^[a-zA-Z_][a-zA-Z0-9_]*$"); + } + + public static boolean isValidJavaIdentifier(String name) { + if (name == null || StringUtils.isBlank(name)) { + return false; + } + if (!Character.isJavaIdentifierStart(name.charAt(0)) || Character.codePointAt(name, 0) > 255) { + return false; + } + // Limit characters to legal Java identifier parts in Latin-1 that aren't ignorable + return name.substring(1) + .chars() + .allMatch(it -> Character.isJavaIdentifierPart(it) && !Character.isIdentifierIgnorable(it) && it < 256); + } + + /** + * This serves the script content for a bound object. Support CSP-compatible st:bind and similar methods of making + * objects accessible to JS. + * + * @param req + * @param rsp + * @param var the variable name to assign the Stapler proxy to + * @param methods the list of methods (needed for {@link WithWellKnownURL}) + * @throws IOException + */ + public void doScript(StaplerRequest req, StaplerResponse rsp, @QueryParameter String var, @QueryParameter String methods) throws IOException { + final String boundUrl = req.getRestOfPath(); + + if (var == null) { + return; + } + + if (!isValidJavaScriptIdentifier(var)) { + LOGGER.log(Level.FINE, () -> "Rejecting invalid JavaScript identifier: " + var); + return; + } + + rsp.setContentType("application/javascript"); + final PrintWriter writer = rsp.getWriter(); + + if ("/null".equals(boundUrl)) { + /* This is the case when the object was null in the first place */ + writer.append(var).append(" = null;"); + return; + } + + final String script; + + /* If this is not a WithWellKnownURL, look UUID up in bound object table and return null if not found. */ + final String contextAndPrefix = Stapler.getCurrentRequest().getContextPath() + PREFIX; + if (boundUrl.startsWith(contextAndPrefix)) { + final String id = boundUrl.replace(contextAndPrefix, ""); + final Table table = resolve(false); + if (table == null) { + rsp.sendError(404); + return; + } + Object object = table.resolve(id); + if (object == null) { + /* Support null bound objects */ + writer.append(var).append(" = null;"); + return; + } + script = Bound.getProxyScript(boundUrl, object.getClass()); + } else { + if (methods == null) { + /* This will result in an empty file rather than an explicit null assignment, + but it's unexpected to have a WithWellKnownURL without ?methods query parameter. */ + return; + } + final String[] methodsArray = methods.split(","); + if (Arrays.stream(methodsArray).anyMatch(it -> !isValidJavaIdentifier(it))) { + LOGGER.log(Level.FINE, () -> "Rejecting method list that includes an invalid Java identifier: " + methods); + // TODO Alternatively, filter out invalid method names and only include valid ones. + // Could help with non-malicious but encoding related issues + return; + } + script = Bound.getProxyScript(boundUrl, methodsArray); + } + writer.append(var).append(" = ").append(script).append(";"); + } + @Override public Table getStaplerFallback() { return resolve(false); @@ -264,6 +356,7 @@ private Object writeReplace() { } public static final String PREFIX = "/$stapler/bound/"; + static final String SCRIPT_PREFIX = "/$stapler/bound/script"; /** * True to activate debug logging of session fragments. diff --git a/jelly/src/main/java/org/kohsuke/stapler/jelly/BindTag.java b/jelly/src/main/java/org/kohsuke/stapler/jelly/BindTag.java index ece7d455a..39113d500 100644 --- a/jelly/src/main/java/org/kohsuke/stapler/jelly/BindTag.java +++ b/jelly/src/main/java/org/kohsuke/stapler/jelly/BindTag.java @@ -23,13 +23,16 @@ package org.kohsuke.stapler.jelly; +import edu.umd.cs.findbugs.annotations.CheckForNull; import org.apache.commons.jelly.JellyTagException; import org.apache.commons.jelly.XMLOutput; import org.jvnet.maven.jellydoc.annotation.NoContent; import org.jvnet.maven.jellydoc.annotation.Required; import org.kohsuke.stapler.WebApp; import org.kohsuke.stapler.bind.Bound; +import org.kohsuke.stapler.bind.BoundObjectTable; import org.xml.sax.SAXException; +import org.xml.sax.helpers.AttributesImpl; /** * Binds a server-side object to client side so that JavaScript can call into server. @@ -68,25 +71,54 @@ public void doTag(XMLOutput out) throws JellyTagException { a.doTag(out); try { - String expr; - if (javaObject==null) { - expr = "null"; + if (javaObject == null) { + if (varName == null) { + // Legacy mode if no 'var' is specified and we bound 'null': Write 'null' expression inline, without