From 8c67b732b55e09dae25bf117e126971869c9bd95 Mon Sep 17 00:00:00 2001
From: Daniel Beck <daniel-beck@users.noreply.github.com>
Date: Sat, 16 Jul 2022 23:27:14 +0200
Subject: [PATCH 01/15] [JENKINS-60866] Make st:bind tag work without inline JS

---
 .../stapler/bind/BoundObjectTable.java        | 18 +++++++++
 .../org/kohsuke/stapler/jelly/BindTag.java    | 37 +++++++++++++------
 2 files changed, 43 insertions(+), 12 deletions(-)

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 8102f22dcc..b4ac50380b 100644
--- a/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java
+++ b/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java
@@ -24,6 +24,7 @@
 package org.kohsuke.stapler.bind;
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import java.io.PrintWriter;
 import org.kohsuke.stapler.Ancestor;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.HttpResponses;
@@ -187,6 +188,23 @@ public HttpResponse doEnableLogging() {
         }
     }
 
+    public static final class BindScript {
+
+        private final Bound bound;
+        private final String variableName;
+
+        public BindScript(Bound bound, String variableName) {
+            this.bound = bound;
+            this.variableName = variableName;
+        }
+
+        public void doIndex(StaplerResponse rsp) throws IOException, ServletException {
+            rsp.setContentType("application/javascript");
+            final PrintWriter writer = rsp.getWriter();
+            writer.append(variableName).append(" = ").append(bound.getProxyScript()).append(";");
+        }
+    }
+
     private static final class WellKnownObjectHandle extends Bound {
         private final String url;
         private final Object target;
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 ed82bd9d32..0359b926f8 100644
--- a/jelly/src/main/java/org/kohsuke/stapler/jelly/BindTag.java
+++ b/jelly/src/main/java/org/kohsuke/stapler/jelly/BindTag.java
@@ -29,8 +29,10 @@
 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.kohsuke.stapler.framework.adjunct.AdjunctsInPage;
 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.
@@ -70,22 +72,33 @@ public void doTag(XMLOutput out) throws JellyTagException {
         a.doTag(out);
 
         try {
-            String expr;
             if (javaObject==null) {
-                expr = "null";
+                if (varName == null) {
+                    out.write("null");
+                } else {
+                    // TODO un-inline?
+                    out.startElement("script");
+                    out.write(varName + "=null;");
+                    out.endElement("script");
+                }
             } else {
                 Bound h = WebApp.getCurrent().boundObjectTable.bind(javaObject);
-                expr = h.getProxyScript();
-            }
 
-            if (varName==null) {
-                // this mode (of writing just the expression) needs to be used with caution because
-                // the adjunct tag above might produce <script> tag.
-                out.write(expr);
-            } else {
-                out.startElement("script");
-                out.write(varName + "=" + expr + ";");
-                out.endElement("script");
+                if (varName==null) {
+                    // this mode (of writing just the expression) needs to be used with caution because
+                    // the adjunct tag above might produce <script> tag.
+                    // Doing this is deprecated as it cannot be done with CSP enabled
+                    out.write(h.getProxyScript());
+                } else {
+                    // TODO Find a better solution for CSP compliant bind scripts than binding another object
+                    Bound script = WebApp.getCurrent().boundObjectTable.bind(new BoundObjectTable.BindScript(h, varName));
+
+                    final AttributesImpl attributes = new AttributesImpl();
+                    attributes.addAttribute("", "src", "src", "", script.getURL());
+                    attributes.addAttribute("", "type", "type", "", "application/javascript");
+                    out.startElement("script", attributes);
+                    out.endElement("script");
+                }
             }
         } catch (SAXException e) {
             throw new JellyTagException(e);

From 6e13cf825dfed39d12da5197fa8ed204178262ee Mon Sep 17 00:00:00 2001
From: Daniel Beck <daniel-beck@users.noreply.github.com>
Date: Sat, 16 Jul 2022 23:33:02 +0200
Subject: [PATCH 02/15] Also have a separate script file if the bound object is
 null

---
 .../stapler/bind/BoundObjectTable.java        |  7 +++--
 .../org/kohsuke/stapler/jelly/BindTag.java    | 26 ++++++++++---------
 2 files changed, 19 insertions(+), 14 deletions(-)

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 b4ac50380b..f4aea6e262 100644
--- a/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java
+++ b/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java
@@ -23,6 +23,8 @@
 
 package org.kohsuke.stapler.bind;
 
+import edu.umd.cs.findbugs.annotations.CheckForNull;
+import edu.umd.cs.findbugs.annotations.NonNull;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.PrintWriter;
 import org.kohsuke.stapler.Ancestor;
@@ -193,7 +195,7 @@ public static final class BindScript {
         private final Bound bound;
         private final String variableName;
 
-        public BindScript(Bound bound, String variableName) {
+        public BindScript(@CheckForNull Bound bound, @NonNull String variableName) {
             this.bound = bound;
             this.variableName = variableName;
         }
@@ -201,7 +203,8 @@ public BindScript(Bound bound, String variableName) {
         public void doIndex(StaplerResponse rsp) throws IOException, ServletException {
             rsp.setContentType("application/javascript");
             final PrintWriter writer = rsp.getWriter();
-            writer.append(variableName).append(" = ").append(bound.getProxyScript()).append(";");
+            final String script = bound == null ? "null" : bound.getProxyScript();
+            writer.append(variableName).append(" = ").append(script).append(";");
         }
     }
 
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 0359b926f8..6c1677d0d4 100644
--- a/jelly/src/main/java/org/kohsuke/stapler/jelly/BindTag.java
+++ b/jelly/src/main/java/org/kohsuke/stapler/jelly/BindTag.java
@@ -23,6 +23,7 @@
 
 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;
@@ -76,10 +77,7 @@ public void doTag(XMLOutput out) throws JellyTagException {
                 if (varName == null) {
                     out.write("null");
                 } else {
-                    // TODO un-inline?
-                    out.startElement("script");
-                    out.write(varName + "=null;");
-                    out.endElement("script");
+                    writeScriptTag(out, null);
                 }
             } else {
                 Bound h = WebApp.getCurrent().boundObjectTable.bind(javaObject);
@@ -90,14 +88,7 @@ public void doTag(XMLOutput out) throws JellyTagException {
                     // Doing this is deprecated as it cannot be done with CSP enabled
                     out.write(h.getProxyScript());
                 } else {
-                    // TODO Find a better solution for CSP compliant bind scripts than binding another object
-                    Bound script = WebApp.getCurrent().boundObjectTable.bind(new BoundObjectTable.BindScript(h, varName));
-
-                    final AttributesImpl attributes = new AttributesImpl();
-                    attributes.addAttribute("", "src", "src", "", script.getURL());
-                    attributes.addAttribute("", "type", "type", "", "application/javascript");
-                    out.startElement("script", attributes);
-                    out.endElement("script");
+                    writeScriptTag(out, h);
                 }
             }
         } catch (SAXException e) {
@@ -105,6 +96,17 @@ public void doTag(XMLOutput out) throws JellyTagException {
         }
     }
 
+    private void writeScriptTag(XMLOutput out, @CheckForNull Bound bound) throws SAXException {
+        // TODO Find a better solution for CSP compliant bind scripts than binding another object
+        Bound script = WebApp.getCurrent().boundObjectTable.bind(new BoundObjectTable.BindScript(bound, varName));
+
+        final AttributesImpl attributes = new AttributesImpl();
+        attributes.addAttribute("", "src", "src", "", script.getURL());
+        attributes.addAttribute("", "type", "type", "", "application/javascript");
+        out.startElement("script", attributes);
+        out.endElement("script");
+    }
+
     /**
      * Ensures that we have the dependencies properly available to run bind.js.
      *

From 63c818e936a0792e0c55fbdc831a4da62f1eb897 Mon Sep 17 00:00:00 2001
From: Daniel Beck <daniel-beck@users.noreply.github.com>
Date: Sun, 17 Jul 2022 10:42:15 +0200
Subject: [PATCH 03/15] Add support for JavaScript proxy

---
 .../java/org/kohsuke/stapler/RequestImpl.java |  7 ++
 .../org/kohsuke/stapler/StaplerRequest.java   | 32 ++++++++
 .../java/org/kohsuke/stapler/bind/Bound.java  | 81 +++++++++++++------
 .../stapler/bind/BoundObjectTable.java        | 54 +++++++++++++
 .../org/kohsuke/stapler/jelly/BindTag.java    | 10 +--
 5 files changed, 156 insertions(+), 28 deletions(-)

diff --git a/core/src/main/java/org/kohsuke/stapler/RequestImpl.java b/core/src/main/java/org/kohsuke/stapler/RequestImpl.java
index f1a222048b..42dded3745 100644
--- a/core/src/main/java/org/kohsuke/stapler/RequestImpl.java
+++ b/core/src/main/java/org/kohsuke/stapler/RequestImpl.java
@@ -37,6 +37,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;
@@ -156,6 +157,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.getBoundMethods());
+    }
+
     @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 6f75356738..e6ff09d953 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;
@@ -40,6 +41,7 @@
 
 import net.sf.json.JSONObject;
 import net.sf.json.JSONArray;
+import org.apache.commons.lang.StringUtils;
 import org.kohsuke.stapler.bind.BoundObjectTable;
 import org.kohsuke.stapler.json.SubmittedForm;
 import org.kohsuke.stapler.lang.Klass;
@@ -515,6 +517,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(since = "TODO")
     String createJavaScriptProxy(Object toBeExported);
+
+    /**
+     * Return value of {@link #createJavaScriptProxyParameters(Object)}
+     */
+    final class RenderOnDemandParameters {
+        public final String proxyMethod; // TODO Is this useful to not need hard-coding 'makeStaplerProxy' in callers?
+        public final String url;
+        public final String crumb;
+        public final Set<String> methods;
+
+        public RenderOnDemandParameters(String proxyMethod, String url, String crumb, Set<String> methods) {
+            this.proxyMethod = proxyMethod;
+            this.url = url;
+            this.crumb = crumb;
+            this.methods = methods;
+        }
+
+        public String getMethodsString() { // TODO Probably better off in the caller?
+            return StringUtils.join(methods, ",");
+        }
+    }
+
+    /**
+     * 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 299328a223..78e2ca268e 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 org.apache.commons.lang.StringUtils;
 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,33 +65,66 @@ 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 this {@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
+     */
+    public static String getProxyScriptUrl(String variableName, Bound bound) {
+        if (bound == null) {
+            // TODO Should this hardocde a non-UUID to ensure null bind?
+            return Stapler.getCurrentRequest().getContextPath() + BoundObjectTable.PREFIX + variableName + "/null";
+        } else {
+            return bound.getProxyScriptURL(variableName);
+        }
+    }
+
+    /**
+     * Returns the URL for the standalone proxy script.
+     * @param variableName the name of the JS variable to assign
+     * @return
+     */
+    public final String getProxyScriptURL(String variableName) {
+        // TODO Probably wrong with WithWellKnownURL?
+        return getURL().replace(BoundObjectTable.PREFIX, BoundObjectTable.SCRIPT_PREFIX + variableName + "/");
+    }
 
-        boolean first=true;
-        for (Method m : getTarget().getClass().getMethods()) {
-            Collection<String> names;
+    private static Set<String> getBoundMethods(Class<?> clazz) {
+        Set<String> 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 names;
+    }
+
+    /**
+     * Returns a list of all JS bound methods of the target's type.
+     * @return a list of all JS bound methods of the target's type
+     */
+    public final Set<String> getBoundMethods() {
+        return getBoundMethods(getTarget().getClass());
+    }
+
+    public static String getProxyScript(String url, Class<?> clazz) {
+        StringBuilder buf = new StringBuilder("makeStaplerProxy('").append(url).append("','").append(
+                WebApp.getCurrent().getCrumbIssuer().issueCrumb()
+        ).append("',[").append(
+                StringUtils.join(getBoundMethods(clazz).stream().map(n -> "'" + n + "'").toArray(String[]::new), ",")).append("])");
         return buf.toString();
     }
 
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 f4aea6e262..e671bca7dd 100644
--- a/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java
+++ b/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java
@@ -27,6 +27,7 @@
 import edu.umd.cs.findbugs.annotations.NonNull;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.PrintWriter;
+import org.apache.commons.lang.StringUtils;
 import org.kohsuke.stapler.Ancestor;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.HttpResponses;
@@ -53,6 +54,55 @@
  * @author Kohsuke Kawaguchi
  */
 public class BoundObjectTable implements StaplerFallback {
+
+    /**
+     * Represents a script that can be rendered
+     */
+    private final class RenderableScript {
+        private final String variableName;
+
+        public RenderableScript(String variableName) {
+            this.variableName = variableName;
+        }
+
+        public void doDynamic(StaplerRequest req, StaplerResponse rsp) throws IOException {
+            final String id = StringUtils.removeStart(req.getRestOfPath(), "/");
+            rsp.setContentType("application/javascript");
+            final PrintWriter writer = rsp.getWriter();
+            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(variableName).append(" = null;");
+                return;
+            }
+            final String script = Bound.getProxyScript(Stapler.getCurrentRequest().getContextPath() + PREFIX + id, object.getClass());
+            writer.append(variableName).append(" = ").append(script).append(";");
+        }
+    }
+
+    private final class ScriptRenderer {
+        private boolean isValidIdentifier(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 RenderableScript getDynamic(String variableName) {
+            if (isValidIdentifier(variableName)) {
+                return new RenderableScript(variableName);
+            } else {
+                return null;
+            }
+        }
+    }
+    public ScriptRenderer getScript() {
+        return new ScriptRenderer();
+    }
+
     @Override
     public Table getStaplerFallback() {
         return resolve(false);
@@ -190,6 +240,9 @@ public HttpResponse doEnableLogging() {
         }
     }
 
+    /**
+     * Supports
+     */
     public static final class BindScript {
 
         private final Bound bound;
@@ -267,6 +320,7 @@ private WeakRef(Object referent) {
     }
 
     public static final String PREFIX = "/$stapler/bound/";
+    public 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 6c1677d0d4..9c96b3acfc 100644
--- a/jelly/src/main/java/org/kohsuke/stapler/jelly/BindTag.java
+++ b/jelly/src/main/java/org/kohsuke/stapler/jelly/BindTag.java
@@ -30,7 +30,6 @@
 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.kohsuke.stapler.framework.adjunct.AdjunctsInPage;
 import org.xml.sax.SAXException;
 import org.xml.sax.helpers.AttributesImpl;
@@ -97,11 +96,12 @@ public void doTag(XMLOutput out) throws JellyTagException {
     }
 
     private void writeScriptTag(XMLOutput out, @CheckForNull Bound bound) throws SAXException {
-        // TODO Find a better solution for CSP compliant bind scripts than binding another object
-        Bound script = WebApp.getCurrent().boundObjectTable.bind(new BoundObjectTable.BindScript(bound, varName));
-
         final AttributesImpl attributes = new AttributesImpl();
-        attributes.addAttribute("", "src", "src", "", script.getURL());
+        if (bound == null) {
+            attributes.addAttribute("", "src", "src", "", Bound.getProxyScriptUrl(varName, null));
+        } else {
+            attributes.addAttribute("", "src", "src", "", bound.getProxyScriptURL(varName));
+        }
         attributes.addAttribute("", "type", "type", "", "application/javascript");
         out.startElement("script", attributes);
         out.endElement("script");

From 0a41330158f020f24523075be8e4691f560c3bed Mon Sep 17 00:00:00 2001
From: Daniel Beck <daniel-beck@users.noreply.github.com>
Date: Sun, 17 Jul 2022 10:55:32 +0200
Subject: [PATCH 04/15] Make this work for SECURITY-595 in Jenkins

---
 .../stapler/bind/BoundObjectTable.java        | 65 ++++++++-----------
 1 file changed, 26 insertions(+), 39 deletions(-)

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 e671bca7dd..1dcbc8924b 100644
--- a/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java
+++ b/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java
@@ -55,52 +55,39 @@
  */
 public class BoundObjectTable implements StaplerFallback {
 
-    /**
-     * Represents a script that can be rendered
-     */
-    private final class RenderableScript {
-        private final String variableName;
+    private static boolean isValidIdentifier(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 RenderableScript(String variableName) {
-            this.variableName = variableName;
+    public void doScript(StaplerRequest req, StaplerResponse rsp) throws IOException {
+        final String variableNameAndId = StringUtils.removeStart(req.getRestOfPath(), "/");
+        if (!variableNameAndId.contains("/")) {
+            return;
         }
+        final String variableName = variableNameAndId.split("/")[0];
+        final String id = variableNameAndId.split("/")[1];
 
-        public void doDynamic(StaplerRequest req, StaplerResponse rsp) throws IOException {
-            final String id = StringUtils.removeStart(req.getRestOfPath(), "/");
-            rsp.setContentType("application/javascript");
-            final PrintWriter writer = rsp.getWriter();
-            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(variableName).append(" = null;");
-                return;
-            }
-            final String script = Bound.getProxyScript(Stapler.getCurrentRequest().getContextPath() + PREFIX + id, object.getClass());
-            writer.append(variableName).append(" = ").append(script).append(";");
+        if (!isValidIdentifier(variableName)) {
+            return;
         }
-    }
 
-    private final class ScriptRenderer {
-        private boolean isValidIdentifier(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_]*$");
+        rsp.setContentType("application/javascript");
+        final PrintWriter writer = rsp.getWriter();
+        final Table table = resolve(false);
+        if (table == null) {
+            rsp.sendError(404);
+            return;
         }
-        public RenderableScript getDynamic(String variableName) {
-            if (isValidIdentifier(variableName)) {
-                return new RenderableScript(variableName);
-            } else {
-                return null;
-            }
+        Object object = table.resolve(id);
+        if (object == null) {
+            /* Support null bound objects */
+            writer.append(variableName).append(" = null;");
+            return;
         }
-    }
-    public ScriptRenderer getScript() {
-        return new ScriptRenderer();
+        final String script = Bound.getProxyScript(Stapler.getCurrentRequest().getContextPath() + PREFIX + id, object.getClass());
+        writer.append(variableName).append(" = ").append(script).append(";");
     }
 
     @Override

From 6298feb6eb8f352f70e587695d434c50135b98a1 Mon Sep 17 00:00:00 2001
From: Daniel Beck <daniel-beck@users.noreply.github.com>
Date: Sun, 17 Jul 2022 11:03:27 +0200
Subject: [PATCH 05/15] Delete unused code

---
 .../stapler/bind/BoundObjectTable.java        | 23 -------------------
 1 file changed, 23 deletions(-)

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 1dcbc8924b..16c0285d80 100644
--- a/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java
+++ b/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java
@@ -23,8 +23,6 @@
 
 package org.kohsuke.stapler.bind;
 
-import edu.umd.cs.findbugs.annotations.CheckForNull;
-import edu.umd.cs.findbugs.annotations.NonNull;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.PrintWriter;
 import org.apache.commons.lang.StringUtils;
@@ -227,27 +225,6 @@ public HttpResponse doEnableLogging() {
         }
     }
 
-    /**
-     * Supports
-     */
-    public static final class BindScript {
-
-        private final Bound bound;
-        private final String variableName;
-
-        public BindScript(@CheckForNull Bound bound, @NonNull String variableName) {
-            this.bound = bound;
-            this.variableName = variableName;
-        }
-
-        public void doIndex(StaplerResponse rsp) throws IOException, ServletException {
-            rsp.setContentType("application/javascript");
-            final PrintWriter writer = rsp.getWriter();
-            final String script = bound == null ? "null" : bound.getProxyScript();
-            writer.append(variableName).append(" = ").append(script).append(";");
-        }
-    }
-
     private static final class WellKnownObjectHandle extends Bound {
         private final String url;
         private final Object target;

From 5db6eea25a223405d64ab29aab93d55b51b95bb8 Mon Sep 17 00:00:00 2001
From: Daniel Beck <daniel-beck@users.noreply.github.com>
Date: Mon, 14 Aug 2023 12:03:03 +0200
Subject: [PATCH 06/15] As requested, don't document when this was deprecated

---
 core/src/main/java/org/kohsuke/stapler/StaplerRequest.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java b/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java
index e6ff09d953..0583e084db 100644
--- a/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java
+++ b/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java
@@ -520,7 +520,7 @@ public interface StaplerRequest extends HttpServletRequest {
      *
      * @deprecated Use {@link #createJavaScriptProxyParameters(Object)} and invoke {@code makeStaplerProxy} yourself.
      */
-    @Deprecated(since = "TODO")
+    @Deprecated
     String createJavaScriptProxy(Object toBeExported);
 
     /**

From 5887e70d9632202cb11f4a4543ee11559e2051a7 Mon Sep 17 00:00:00 2001
From: Daniel Beck <daniel-beck@users.noreply.github.com>
Date: Mon, 14 Aug 2023 12:04:20 +0200
Subject: [PATCH 07/15] Fix typo

---
 core/src/main/java/org/kohsuke/stapler/bind/Bound.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 78e2ca268e..94b167455e 100644
--- a/core/src/main/java/org/kohsuke/stapler/bind/Bound.java
+++ b/core/src/main/java/org/kohsuke/stapler/bind/Bound.java
@@ -69,7 +69,7 @@ public final String getProxyScript() {
     }
 
     /**
-     * Returns the URl to the proxy script for this {@link org.kohsuke.stapler.bind.Bound}
+     * Returns the URL to the proxy script for this {@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

From 09b893a8b493ca2fe7d4786c05631bbb5d138386 Mon Sep 17 00:00:00 2001
From: Daniel Beck <daniel-beck@users.noreply.github.com>
Date: Tue, 15 Aug 2023 20:54:30 +0200
Subject: [PATCH 08/15] Make WithWellKnownURL with with CSP-compliant Stapler
 proxy

---
 .../java/org/kohsuke/stapler/bind/Bound.java  | 24 ++++--
 .../stapler/bind/BoundObjectTable.java        | 83 +++++++++++++++----
 .../org/kohsuke/stapler/jelly/BindTag.java    |  2 +-
 3 files changed, 84 insertions(+), 25 deletions(-)

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 94b167455e..ac338d810a 100644
--- a/core/src/main/java/org/kohsuke/stapler/bind/Bound.java
+++ b/core/src/main/java/org/kohsuke/stapler/bind/Bound.java
@@ -74,10 +74,10 @@ public final String getProxyScript() {
      * @param bound the bound object, or {@code null} if none.
      * @return
      */
-    public static String getProxyScriptUrl(String variableName, Bound bound) {
+    public static String getProxyScriptURL(String variableName, Bound bound) {
         if (bound == null) {
-            // TODO Should this hardocde a non-UUID to ensure null bind?
-            return Stapler.getCurrentRequest().getContextPath() + BoundObjectTable.PREFIX + variableName + "/null";
+            // TODO Should this hardcode a non-UUID to ensure null bind?
+            return Stapler.getCurrentRequest().getContextPath() + BoundObjectTable.SCRIPT_PREFIX + "/null?var=" + variableName;
         } else {
             return bound.getProxyScriptURL(variableName);
         }
@@ -89,8 +89,9 @@ public static String getProxyScriptUrl(String variableName, Bound bound) {
      * @return
      */
     public final String getProxyScriptURL(String variableName) {
-        // TODO Probably wrong with WithWellKnownURL?
-        return getURL().replace(BoundObjectTable.PREFIX, BoundObjectTable.SCRIPT_PREFIX + variableName + "/");
+        final String methodsList = StringUtils.join(getBoundMethods(getTarget().getClass()).toArray(String[]::new), ",");
+        // 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;
     }
 
     private static Set<String> getBoundMethods(Class<?> clazz) {
@@ -121,10 +122,21 @@ public final Set<String> getBoundMethods() {
     }
 
     public static String getProxyScript(String url, Class<?> clazz) {
+        return getProxyScript(url, getBoundMethods(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
+     */
+    public static String getProxyScript(String url, String[] methods) {
         StringBuilder buf = new StringBuilder("makeStaplerProxy('").append(url).append("','").append(
                 WebApp.getCurrent().getCrumbIssuer().issueCrumb()
         ).append("',[").append(
-                StringUtils.join(getBoundMethods(clazz).stream().map(n -> "'" + n + "'").toArray(String[]::new), ",")).append("])");
+                StringUtils.join(Arrays.stream(methods).sorted().map(it -> "'" + it + "'").toArray(), ",")).append("])");
         return buf.toString();
     }
 
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 16c0285d80..01152e15a7 100644
--- a/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java
+++ b/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java
@@ -25,10 +25,13 @@
 
 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;
@@ -53,39 +56,83 @@
  */
 public class BoundObjectTable implements StaplerFallback {
 
-    private static boolean isValidIdentifier(String variableName) {
+    private 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 void doScript(StaplerRequest req, StaplerResponse rsp) throws IOException {
-        final String variableNameAndId = StringUtils.removeStart(req.getRestOfPath(), "/");
-        if (!variableNameAndId.contains("/")) {
+    private static boolean isValidJavaIdentifier(String name) {
+        if (name == null || StringUtils.isBlank(name)) {
+            return false;
+        }
+        if (!Character.isJavaIdentifierStart(name.charAt(0))) {
+            return false;
+        }
+        return name.substring(1).chars().allMatch(Character::isJavaIdentifierPart); // TODO More restrictive
+    }
+
+    /**
+     * 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;
         }
-        final String variableName = variableNameAndId.split("/")[0];
-        final String id = variableNameAndId.split("/")[1];
 
-        if (!isValidIdentifier(variableName)) {
+        if (!isValidJavaScriptIdentifier(var)) {
+            LOGGER.log(Level.FINE, () -> "Rejecting invalid JavaScript identifier: " + var);
             return;
         }
 
         rsp.setContentType("application/javascript");
         final PrintWriter writer = rsp.getWriter();
-        final Table table = resolve(false);
-        if (table == null) {
-            rsp.sendError(404);
+
+        if ("null".equals(boundUrl)) {
+            /* This is the case when the object was null in the first place */
+            writer.append(var).append(" = null;");
             return;
         }
-        Object object = table.resolve(id);
-        if (object == null) {
-            /* Support null bound objects */
-            writer.append(variableName).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) {
+                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);
+                return;
+            }
+            script = Bound.getProxyScript(boundUrl, methodsArray);
         }
-        final String script = Bound.getProxyScript(Stapler.getCurrentRequest().getContextPath() + PREFIX + id, object.getClass());
-        writer.append(variableName).append(" = ").append(script).append(";");
+        writer.append(var).append(" = ").append(script).append(";");
     }
 
     @Override
@@ -284,7 +331,7 @@ private WeakRef(Object referent) {
     }
 
     public static final String PREFIX = "/$stapler/bound/";
-    public static final String SCRIPT_PREFIX = "/$stapler/bound/script/";
+    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 3a5b9d9f16..c20dad9950 100644
--- a/jelly/src/main/java/org/kohsuke/stapler/jelly/BindTag.java
+++ b/jelly/src/main/java/org/kohsuke/stapler/jelly/BindTag.java
@@ -96,7 +96,7 @@ public void doTag(XMLOutput out) throws JellyTagException {
     private void writeScriptTag(XMLOutput out, @CheckForNull Bound bound) throws SAXException {
         final AttributesImpl attributes = new AttributesImpl();
         if (bound == null) {
-            attributes.addAttribute("", "src", "src", "", Bound.getProxyScriptUrl(varName, null));
+            attributes.addAttribute("", "src", "src", "", Bound.getProxyScriptURL(varName, null));
         } else {
             attributes.addAttribute("", "src", "src", "", bound.getProxyScriptURL(varName));
         }

From a7d1323217522d0976725633403d9d2e738b55be Mon Sep 17 00:00:00 2001
From: Daniel Beck <daniel-beck@users.noreply.github.com>
Date: Wed, 16 Aug 2023 10:24:49 +0200
Subject: [PATCH 09/15] Rename "methods" to (JavaScript)UrlNames to reduce
 ambiguity

---
 .../src/main/java/org/kohsuke/stapler/RequestImpl.java |  2 +-
 .../main/java/org/kohsuke/stapler/StaplerRequest.java  | 10 +++++-----
 core/src/main/java/org/kohsuke/stapler/bind/Bound.java | 10 +++++-----
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/core/src/main/java/org/kohsuke/stapler/RequestImpl.java b/core/src/main/java/org/kohsuke/stapler/RequestImpl.java
index a88ff5dc84..d875f5eeb9 100644
--- a/core/src/main/java/org/kohsuke/stapler/RequestImpl.java
+++ b/core/src/main/java/org/kohsuke/stapler/RequestImpl.java
@@ -185,7 +185,7 @@ public String createJavaScriptProxy(Object toBeExported) {
     @Override
     public RenderOnDemandParameters createJavaScriptProxyParameters(Object toBeExported) {
         final Bound bound = getBoundObjectTable().bind(toBeExported);
-        return new RenderOnDemandParameters("makeStaplerProxy", bound.getURL(), getWebApp().getCrumbIssuer().issueCrumb(), bound.getBoundMethods());
+        return new RenderOnDemandParameters("makeStaplerProxy", bound.getURL(), getWebApp().getCrumbIssuer().issueCrumb(), bound.getBoundJavaScriptUrlNames());
     }
 
     @Override
diff --git a/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java b/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java
index 0583e084db..d0dc264fb5 100644
--- a/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java
+++ b/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java
@@ -530,17 +530,17 @@ final class RenderOnDemandParameters {
         public final String proxyMethod; // TODO Is this useful to not need hard-coding 'makeStaplerProxy' in callers?
         public final String url;
         public final String crumb;
-        public final Set<String> methods;
+        public final Set<String> urlNames;
 
-        public RenderOnDemandParameters(String proxyMethod, String url, String crumb, Set<String> methods) {
+        public RenderOnDemandParameters(String proxyMethod, String url, String crumb, Set<String> urlNames) {
             this.proxyMethod = proxyMethod;
             this.url = url;
             this.crumb = crumb;
-            this.methods = methods;
+            this.urlNames = urlNames;
         }
 
-        public String getMethodsString() { // TODO Probably better off in the caller?
-            return StringUtils.join(methods, ",");
+        public String getUrlNames() { // TODO Probably better off in the caller?
+            return StringUtils.join(urlNames, ",");
         }
     }
 
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 ac338d810a..ea7a2955da 100644
--- a/core/src/main/java/org/kohsuke/stapler/bind/Bound.java
+++ b/core/src/main/java/org/kohsuke/stapler/bind/Bound.java
@@ -89,12 +89,12 @@ public static String getProxyScriptURL(String variableName, Bound bound) {
      * @return
      */
     public final String getProxyScriptURL(String variableName) {
-        final String methodsList = StringUtils.join(getBoundMethods(getTarget().getClass()).toArray(String[]::new), ",");
+        final String methodsList = StringUtils.join(getBoundJavaScriptUrlNames(getTarget().getClass()).toArray(String[]::new), ",");
         // 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;
     }
 
-    private static Set<String> getBoundMethods(Class<?> clazz) {
+    private static Set<String> getBoundJavaScriptUrlNames(Class<?> clazz) {
         Set<String> names = new HashSet<>();
         for (Method m : clazz.getMethods()) {
             if (m.getName().startsWith("js")) {
@@ -117,12 +117,12 @@ private static Set<String> getBoundMethods(Class<?> clazz) {
      * Returns a list of all JS bound methods of the target's type.
      * @return a list of all JS bound methods of the target's type
      */
-    public final Set<String> getBoundMethods() {
-        return getBoundMethods(getTarget().getClass());
+    public final Set<String> getBoundJavaScriptUrlNames() {
+        return getBoundJavaScriptUrlNames(getTarget().getClass());
     }
 
     public static String getProxyScript(String url, Class<?> clazz) {
-        return getProxyScript(url, getBoundMethods(clazz).toArray(String[]::new));
+        return getProxyScript(url, getBoundJavaScriptUrlNames(clazz).toArray(String[]::new));
     }
 
     /**

From 7bc33e0cc8538b866b9300e28c3dda532bdaf588 Mon Sep 17 00:00:00 2001
From: Daniel Beck <daniel-beck@users.noreply.github.com>
Date: Wed, 16 Aug 2023 10:28:47 +0200
Subject: [PATCH 10/15] Remove obsolete TODO

---
 core/src/main/java/org/kohsuke/stapler/bind/Bound.java | 1 -
 1 file changed, 1 deletion(-)

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 ea7a2955da..b1f736263d 100644
--- a/core/src/main/java/org/kohsuke/stapler/bind/Bound.java
+++ b/core/src/main/java/org/kohsuke/stapler/bind/Bound.java
@@ -76,7 +76,6 @@ public final String getProxyScript() {
      */
     public static String getProxyScriptURL(String variableName, Bound bound) {
         if (bound == null) {
-            // TODO Should this hardcode a non-UUID to ensure null bind?
             return Stapler.getCurrentRequest().getContextPath() + BoundObjectTable.SCRIPT_PREFIX + "/null?var=" + variableName;
         } else {
             return bound.getProxyScriptURL(variableName);

From 34d7e67885eaec10d759fa46ef6252b9f2040e22 Mon Sep 17 00:00:00 2001
From: Daniel Beck <daniel-beck@users.noreply.github.com>
Date: Mon, 21 Aug 2023 14:07:08 +0200
Subject: [PATCH 11/15] Fix "/null" restOfPath, note behavior with unexpected
 branch

---
 .../main/java/org/kohsuke/stapler/bind/BoundObjectTable.java  | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

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 01152e15a7..502c3c4559 100644
--- a/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java
+++ b/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java
@@ -97,7 +97,7 @@ public void doScript(StaplerRequest req, StaplerResponse rsp, @QueryParameter St
         rsp.setContentType("application/javascript");
         final PrintWriter writer = rsp.getWriter();
 
-        if ("null".equals(boundUrl)) {
+        if ("/null".equals(boundUrl)) {
             /* This is the case when the object was null in the first place */
             writer.append(var).append(" = null;");
             return;
@@ -123,6 +123,8 @@ public void doScript(StaplerRequest req, StaplerResponse rsp, @QueryParameter St
             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(",");

From 16f23e39438b731851dc446c81d32ec9c6c84589 Mon Sep 17 00:00:00 2001
From: Daniel Beck <daniel-beck@users.noreply.github.com>
Date: Tue, 22 Aug 2023 22:00:01 +0200
Subject: [PATCH 12/15] Restrict identifiers further, allow fallback with
 unsafe var names

---
 .../org/kohsuke/stapler/StaplerRequest.java   |  4 +--
 .../stapler/bind/BoundObjectTable.java        | 13 ++++++---
 .../org/kohsuke/stapler/jelly/BindTag.java    | 27 +++++++++++++++----
 3 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java b/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java
index d0dc264fb5..e26d2688e2 100644
--- a/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java
+++ b/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java
@@ -527,7 +527,7 @@ public interface StaplerRequest extends HttpServletRequest {
      * Return value of {@link #createJavaScriptProxyParameters(Object)}
      */
     final class RenderOnDemandParameters {
-        public final String proxyMethod; // TODO Is this useful to not need hard-coding 'makeStaplerProxy' in callers?
+        public final String proxyMethod;
         public final String url;
         public final String crumb;
         public final Set<String> urlNames;
@@ -539,7 +539,7 @@ public RenderOnDemandParameters(String proxyMethod, String url, String crumb, Se
             this.urlNames = urlNames;
         }
 
-        public String getUrlNames() { // TODO Probably better off in the caller?
+        public String getUrlNames() {
             return StringUtils.join(urlNames, ",");
         }
     }
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 502c3c4559..b3defcd70d 100644
--- a/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java
+++ b/core/src/main/java/org/kohsuke/stapler/bind/BoundObjectTable.java
@@ -56,20 +56,23 @@
  */
 public class BoundObjectTable implements StaplerFallback {
 
-    private static boolean isValidJavaScriptIdentifier(String variableName) {
+    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_]*$");
     }
 
-    private static boolean isValidJavaIdentifier(String name) {
+    public static boolean isValidJavaIdentifier(String name) {
         if (name == null || StringUtils.isBlank(name)) {
             return false;
         }
-        if (!Character.isJavaIdentifierStart(name.charAt(0))) {
+        if (!Character.isJavaIdentifierStart(name.charAt(0)) || Character.codePointAt(name, 0) > 255) {
             return false;
         }
-        return name.substring(1).chars().allMatch(Character::isJavaIdentifierPart); // TODO More restrictive
+        // 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);
     }
 
     /**
@@ -130,6 +133,8 @@ public void doScript(StaplerRequest req, StaplerResponse rsp, @QueryParameter St
             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);
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 c20dad9950..f14b08dac7 100644
--- a/jelly/src/main/java/org/kohsuke/stapler/jelly/BindTag.java
+++ b/jelly/src/main/java/org/kohsuke/stapler/jelly/BindTag.java
@@ -30,6 +30,7 @@
 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;
 
@@ -70,21 +71,30 @@ public void doTag(XMLOutput out) throws JellyTagException {
         a.doTag(out);
 
         try {
-            if (javaObject==null) {
+            if (javaObject == null) {
                 if (varName == null) {
+                    // Legacy mode if no 'var' is specified and we bound 'null': Write 'null' expression inline, without <script> wrapper.
                     out.write("null");
                 } else {
+                    // Modern: Write a script tag whose 'src' points to DataBoundTable#doScript, with a special suffix indicating the null bound object.
                     writeScriptTag(out, null);
                 }
             } else {
                 Bound h = WebApp.getCurrent().boundObjectTable.bind(javaObject);
 
-                if (varName==null) {
-                    // this mode (of writing just the expression) needs to be used with caution because
-                    // the adjunct tag above might produce <script> tag.
-                    // Doing this is deprecated as it cannot be done with CSP enabled
+                if (varName == null) {
+                    // Legacy mode if no 'var' is specified: Write the expression inline, without <script> wrapper.
+                    // Doing this is deprecated as it cannot be done with Content-Security-Policy unless 'unsafe-inline' is allowed.
+                    // Additionally, this mode needs to be used with caution because the adjunct tag above might produce a <script> tag.
                     out.write(h.getProxyScript());
+                } else if (!BoundObjectTable.isValidJavaScriptIdentifier(varName)) {
+                    // Legacy mode if 'var' is not a safe variable name: Write the expression inline within <script> wrapper.
+                    // Doing this is deprecated as it cannot be done with Content-Security-Policy unless 'unsafe-inline' is allowed.
+                    out.startElement("script");
+                    out.write(varName + "=" + h.getProxyScript() + ";");
+                    out.endElement("script");
                 } else {
+                    // Modern: Write a script tag whose 'src' points to DataBoundTable#doScript
                     writeScriptTag(out, h);
                 }
             }
@@ -93,6 +103,13 @@ public void doTag(XMLOutput out) throws JellyTagException {
         }
     }
 
+    /**
+     * Writes a &lt;script&gt; tag whose {@code src} attribute points to
+     * {@link BoundObjectTable#doScript(org.kohsuke.stapler.StaplerRequest, org.kohsuke.stapler.StaplerResponse, String, String)}.
+     * @param out XML output
+     * @param bound Wrapper for the bound object
+     * @throws SAXException if something goes wrong writing XML
+     */
     private void writeScriptTag(XMLOutput out, @CheckForNull Bound bound) throws SAXException {
         final AttributesImpl attributes = new AttributesImpl();
         if (bound == null) {

From 70afd882b468b43f1eb1adb2c1e0cd45e576f51b Mon Sep 17 00:00:00 2001
From: Daniel Beck <daniel-beck@users.noreply.github.com>
Date: Mon, 12 Feb 2024 20:18:23 +0100
Subject: [PATCH 13/15] Simplify string joining

---
 .../java/org/kohsuke/stapler/StaplerRequest.java     |  3 +--
 .../main/java/org/kohsuke/stapler/bind/Bound.java    | 12 +++++-------
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java b/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java
index e26d2688e2..8138849e40 100644
--- a/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java
+++ b/core/src/main/java/org/kohsuke/stapler/StaplerRequest.java
@@ -41,7 +41,6 @@
 
 import net.sf.json.JSONObject;
 import net.sf.json.JSONArray;
-import org.apache.commons.lang.StringUtils;
 import org.kohsuke.stapler.bind.BoundObjectTable;
 import org.kohsuke.stapler.json.SubmittedForm;
 import org.kohsuke.stapler.lang.Klass;
@@ -540,7 +539,7 @@ public RenderOnDemandParameters(String proxyMethod, String url, String crumb, Se
         }
 
         public String getUrlNames() {
-            return StringUtils.join(urlNames, ",");
+            return String.join(",", urlNames);
         }
     }
 
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 b1f736263d..e2be76bf1b 100644
--- a/core/src/main/java/org/kohsuke/stapler/bind/Bound.java
+++ b/core/src/main/java/org/kohsuke/stapler/bind/Bound.java
@@ -25,7 +25,7 @@
 
 import java.util.HashSet;
 import java.util.Set;
-import org.apache.commons.lang.StringUtils;
+import java.util.stream.Collectors;
 import org.kohsuke.stapler.HttpResponse;
 import org.kohsuke.stapler.MetaClass;
 import org.kohsuke.stapler.Stapler;
@@ -88,7 +88,7 @@ public static String getProxyScriptURL(String variableName, Bound bound) {
      * @return
      */
     public final String getProxyScriptURL(String variableName) {
-        final String methodsList = StringUtils.join(getBoundJavaScriptUrlNames(getTarget().getClass()).toArray(String[]::new), ",");
+        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;
     }
@@ -132,11 +132,9 @@ public static String getProxyScript(String url, Class<?> clazz) {
      * @return
      */
     public static String getProxyScript(String url, String[] methods) {
-        StringBuilder buf = new StringBuilder("makeStaplerProxy('").append(url).append("','").append(
-                WebApp.getCurrent().getCrumbIssuer().issueCrumb()
-        ).append("',[").append(
-                StringUtils.join(Arrays.stream(methods).sorted().map(it -> "'" + it + "'").toArray(), ",")).append("])");
-        return buf.toString();
+        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) {

From 0d1c34f282cfeba4af69e875b098377571888ed6 Mon Sep 17 00:00:00 2001
From: Daniel Beck <daniel-beck@users.noreply.github.com>
Date: Tue, 13 Feb 2024 13:49:32 +0100
Subject: [PATCH 14/15] Javadoc improvements

---
 core/src/main/java/org/kohsuke/stapler/bind/Bound.java | 10 ++++++----
 .../main/java/org/kohsuke/stapler/jelly/BindTag.java   |  2 +-
 2 files changed, 7 insertions(+), 5 deletions(-)

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 e2be76bf1b..dfd29b3a54 100644
--- a/core/src/main/java/org/kohsuke/stapler/bind/Bound.java
+++ b/core/src/main/java/org/kohsuke/stapler/bind/Bound.java
@@ -69,10 +69,11 @@ public final String getProxyScript() {
     }
 
     /**
-     * Returns the URL to the proxy script for this {@link org.kohsuke.stapler.bind.Bound}
+     * 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
+     * @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) {
@@ -83,9 +84,10 @@ public static String getProxyScriptURL(String variableName, Bound bound) {
     }
 
     /**
-     * Returns the URL for the standalone proxy script.
+     * 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
+     * @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()));
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 f14b08dac7..39113d5006 100644
--- a/jelly/src/main/java/org/kohsuke/stapler/jelly/BindTag.java
+++ b/jelly/src/main/java/org/kohsuke/stapler/jelly/BindTag.java
@@ -104,7 +104,7 @@ public void doTag(XMLOutput out) throws JellyTagException {
     }
 
     /**
-     * Writes a &lt;script&gt; tag whose {@code src} attribute points to
+     * Writes a {@code <script>} tag whose {@code src} attribute points to
      * {@link BoundObjectTable#doScript(org.kohsuke.stapler.StaplerRequest, org.kohsuke.stapler.StaplerResponse, String, String)}.
      * @param out XML output
      * @param bound Wrapper for the bound object

From dbb41c611e64589e4026686288a1c3f84115ccf1 Mon Sep 17 00:00:00 2001
From: Daniel Beck <daniel-beck@users.noreply.github.com>
Date: Wed, 14 Feb 2024 09:55:21 +0100
Subject: [PATCH 15/15] More Javadoc fixes

---
 core/src/main/java/org/kohsuke/stapler/bind/Bound.java | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

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 dfd29b3a54..36b6b79741 100644
--- a/core/src/main/java/org/kohsuke/stapler/bind/Bound.java
+++ b/core/src/main/java/org/kohsuke/stapler/bind/Bound.java
@@ -115,8 +115,8 @@ private static Set<String> getBoundJavaScriptUrlNames(Class<?> clazz) {
     }
 
     /**
-     * Returns a list of all JS bound methods of the target's type.
-     * @return a list of all JS bound methods of the target's type
+     * 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<String> getBoundJavaScriptUrlNames() {
         return getBoundJavaScriptUrlNames(getTarget().getClass());
@@ -131,7 +131,7 @@ public static String getProxyScript(String url, Class<?> clazz) {
      *
      * @param url URL to proxied object
      * @param methods list of method names
-     * @return
+     * @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();