Skip to content

Commit 6460778

Browse files
daniel-beckbasil
andauthored
[JENKINS-60866][JENKINS-71513] Apply Stapler update for CSP-compliant st:bind and renderOnDemand (#6865)
* [JENKINS-60866] Apply Stapler update for CSP-compliant st:bind * [JENKINS-60866] Make renderOnDemand CSP-compliant * Thanks Spotless * Make Stapler incrementals work * Update Stapler to new incremental * Fixup bad merge * Update Stapler, add test demonstrating st:bind working * Address review feedback * Add test for null bind, update Stapler * Checkstyle * More tests * Use released Stapler --------- Co-authored-by: Daniel Beck <[email protected]> Co-authored-by: Basil Crow <[email protected]>
1 parent 885978d commit 6460778

File tree

12 files changed

+253
-3
lines changed

12 files changed

+253
-3
lines changed

bom/pom.xml

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ THE SOFTWARE.
4040
<properties>
4141
<asm.version>9.6</asm.version>
4242
<slf4jVersion>2.0.12</slf4jVersion>
43-
<stapler.version>1822.v120278426e1c</stapler.version>
43+
<stapler.version>1839.ved17667b_a_eb_5</stapler.version>
4444
<groovy.version>2.4.21</groovy.version>
4545
</properties>
4646

core/src/main/java/hudson/Functions.java

+9
Original file line numberDiff line numberDiff line change
@@ -2242,10 +2242,19 @@ public static boolean isWipeOutPermissionEnabled() {
22422242
return SystemProperties.getBoolean("hudson.security.WipeOutPermission");
22432243
}
22442244

2245+
@Deprecated
22452246
public static String createRenderOnDemandProxy(JellyContext context, String attributesToCapture) {
22462247
return Stapler.getCurrentRequest().createJavaScriptProxy(new RenderOnDemandClosure(context, attributesToCapture));
22472248
}
22482249

2250+
/**
2251+
* Called from renderOnDemand.jelly to generate the parameters for the proxy object generation.
2252+
*/
2253+
@Restricted(NoExternalUse.class)
2254+
public static StaplerRequest.RenderOnDemandParameters createRenderOnDemandProxyParameters(JellyContext context, String attributesToCapture) {
2255+
return Stapler.getCurrentRequest().createJavaScriptProxyParameters(new RenderOnDemandClosure(context, attributesToCapture));
2256+
}
2257+
22492258
public static String getCurrentDescriptorByNameUrl() {
22502259
return Descriptor.getCurrentDescriptorByNameUrl();
22512260
}

core/src/main/resources/lib/layout/renderOnDemand.jelly

+5-1
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,12 @@ THE SOFTWARE.
3636
</st:attribute>
3737
</st:documentation>
3838

39+
<j:set var="parameters" value="${h.createRenderOnDemandProxyParameters(context,attrs.capture)}"/>
3940
<x:element name="${attrs.tag?:'div'}">
4041
<x:attribute name="class">render-on-demand ${attrs.clazz}</x:attribute>
41-
<x:attribute name="proxy">${h.createRenderOnDemandProxy(context,attrs.capture)}</x:attribute>
42+
<x:attribute name="data-proxy-method">${parameters.proxyMethod}</x:attribute>
43+
<x:attribute name="data-proxy-url">${parameters.url}</x:attribute>
44+
<x:attribute name="data-proxy-crumb">${parameters.crumb}</x:attribute>
45+
<x:attribute name="data-proxy-url-names">${parameters.urlNames}</x:attribute>
4246
</x:element>
4347
</j:jelly>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
package org.kohsuke.stapler;
2+
3+
import static org.hamcrest.CoreMatchers.containsString;
4+
import static org.hamcrest.CoreMatchers.endsWith;
5+
import static org.hamcrest.CoreMatchers.is;
6+
import static org.hamcrest.CoreMatchers.startsWith;
7+
import static org.hamcrest.MatcherAssert.assertThat;
8+
import static org.junit.Assert.assertThrows;
9+
10+
import hudson.ExtensionList;
11+
import hudson.model.InvisibleAction;
12+
import hudson.model.RootAction;
13+
import java.util.Arrays;
14+
import java.util.List;
15+
import org.apache.commons.lang.StringUtils;
16+
import org.htmlunit.Page;
17+
import org.htmlunit.ScriptException;
18+
import org.htmlunit.html.HtmlPage;
19+
import org.junit.Rule;
20+
import org.junit.Test;
21+
import org.junit.runner.RunWith;
22+
import org.junit.runners.Parameterized;
23+
import org.jvnet.hudson.test.JenkinsRule;
24+
import org.jvnet.hudson.test.TestExtension;
25+
import org.kohsuke.stapler.bind.JavaScriptMethod;
26+
import org.kohsuke.stapler.bind.WithWellKnownURL;
27+
28+
@RunWith(Parameterized.class)
29+
public class BindTest {
30+
@Rule
31+
public JenkinsRule j = new JenkinsRule();
32+
33+
@Parameterized.Parameters
34+
public static List<String> contexts() {
35+
return Arrays.asList("/jenkins", "");
36+
}
37+
38+
public BindTest(String contextPath) {
39+
j.contextPath = contextPath;
40+
}
41+
42+
@Test
43+
public void bindNormal() throws Exception {
44+
final RootActionImpl root = ExtensionList.lookupSingleton(RootActionImpl.class);
45+
try (JenkinsRule.WebClient wc = j.createWebClient()) {
46+
final HtmlPage htmlPage = wc.goTo(root.getUrlName());
47+
final String scriptUrl = htmlPage
48+
.getElementsByTagName("script")
49+
.stream()
50+
.filter(it -> it.getAttribute("src").startsWith(j.contextPath + "/$stapler/bound/script" + j.contextPath + "/$stapler/bound/"))
51+
.findFirst()
52+
.orElseThrow()
53+
.getAttribute("src");
54+
55+
final Page script = wc.goTo(StringUtils.removeStart(scriptUrl, j.contextPath + "/"), "application/javascript");
56+
final String content = script.getWebResponse().getContentAsString();
57+
assertThat(content, startsWith("varname = makeStaplerProxy('" + j.contextPath + "/$stapler/bound/"));
58+
assertThat(content, endsWith("','test',['annotatedJsMethod1','byName1']);"));
59+
}
60+
assertThat(root.invocations, is(1));
61+
}
62+
63+
@Test
64+
public void bindWithWellKnownURL() throws Exception {
65+
final RootActionWithWellKnownURL root = ExtensionList.lookupSingleton(RootActionWithWellKnownURL.class);
66+
try (JenkinsRule.WebClient wc = j.createWebClient()) {
67+
final HtmlPage htmlPage = wc.goTo(root.getUrlName());
68+
final String scriptUrl = htmlPage
69+
.getElementsByTagName("script")
70+
.stream()
71+
.filter(it -> it.getAttribute("src").startsWith(j.contextPath + "/$stapler/bound/script" + j.contextPath + "/theWellKnownRoot?"))
72+
.findFirst()
73+
.orElseThrow()
74+
.getAttribute("src");
75+
76+
final Page script = wc.goTo(StringUtils.removeStart(scriptUrl, j.contextPath + "/"), "application/javascript");
77+
assertThat(script.getWebResponse().getContentAsString(), is("varname = makeStaplerProxy('" + j.contextPath + "/theWellKnownRoot','test',['annotatedJsMethod2','byName2']);"));
78+
}
79+
assertThat(root.invocations, is(1));
80+
}
81+
82+
@Test
83+
public void bindNull() throws Exception {
84+
final RootActionImpl root = ExtensionList.lookupSingleton(RootActionImpl.class);
85+
try (JenkinsRule.WebClient wc = j.createWebClient()) {
86+
final ScriptException exception = assertThrows(ScriptException.class, () -> wc.goTo(root.getUrlName() + "/null"));
87+
assertThat(exception.getFailingLineNumber(), is(2));
88+
assertThat(exception.getFailingColumnNumber(), is(0));
89+
assertThat(exception.getMessage(), containsString("TypeError: Cannot call method \"byName1\" of null"));
90+
91+
final HtmlPage htmlPage = exception.getPage();
92+
final String scriptUrl = htmlPage.getElementsByTagName("script").stream().filter(it -> it.getAttribute("src").equals(j.contextPath + "/$stapler/bound/script/null?var=varname")).findFirst().orElseThrow().getAttribute("src");
93+
94+
final Page script = wc.goTo(StringUtils.removeStart(scriptUrl, j.contextPath + "/"), "application/javascript");
95+
final String content = script.getWebResponse().getContentAsString();
96+
assertThat(content, is("varname = null;"));
97+
}
98+
assertThat(root.invocations, is(0));
99+
}
100+
101+
@Test
102+
public void bindUnsafe() throws Exception {
103+
final RootActionImpl root = ExtensionList.lookupSingleton(RootActionImpl.class);
104+
try (JenkinsRule.WebClient wc = j.createWebClient()) {
105+
final HtmlPage htmlPage = wc.goTo(root.getUrlName() + "/unsafe-var");
106+
final String content = htmlPage
107+
.getElementsByTagName("script")
108+
.stream()
109+
.filter(it -> it.getTextContent().contains("makeStaplerProxy"))
110+
.findFirst()
111+
.orElseThrow()
112+
.getTextContent();
113+
114+
assertThat(content, startsWith("window['varname']=makeStaplerProxy('" + j.contextPath + "/$stapler/bound/"));
115+
assertThat(content, endsWith("','test',['annotatedJsMethod1','byName1']);"));
116+
}
117+
assertThat(root.invocations, is(1));
118+
}
119+
120+
@Test
121+
public void bindInlineNull() throws Exception {
122+
final RootActionImpl root = ExtensionList.lookupSingleton(RootActionImpl.class);
123+
try (JenkinsRule.WebClient wc = j.createWebClient()) {
124+
final HtmlPage htmlPage = wc.goTo(root.getUrlName() + "/inline-null");
125+
final String content = htmlPage
126+
.getElementsByTagName("script")
127+
.stream()
128+
.filter(it -> it.getTextContent().contains("var inline"))
129+
.findFirst()
130+
.orElseThrow()
131+
.getTextContent();
132+
133+
assertThat(content, containsString("var inline = null"));
134+
}
135+
assertThat(root.invocations, is(0));
136+
}
137+
138+
@TestExtension
139+
public static class RootActionImpl extends InvisibleAction implements RootAction {
140+
private int invocations;
141+
142+
@Override
143+
public String getUrlName() {
144+
return "theRoot";
145+
}
146+
147+
@JavaScriptMethod
148+
public void annotatedJsMethod1(String foo) {}
149+
150+
public void jsByName1() {
151+
invocations++;
152+
}
153+
}
154+
155+
@TestExtension
156+
public static class RootActionWithWellKnownURL extends InvisibleAction implements RootAction, WithWellKnownURL {
157+
private int invocations;
158+
159+
@Override
160+
public String getUrlName() {
161+
return "theWellKnownRoot";
162+
}
163+
164+
@Override
165+
public String getWellKnownUrl() {
166+
return "/" + getUrlName();
167+
}
168+
169+
@JavaScriptMethod
170+
public void annotatedJsMethod2(String foo) {}
171+
172+
public void jsByName2() {
173+
invocations++;
174+
}
175+
}
176+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// eslint-disable-next-line no-undef
2+
varname.byName1();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?jelly escape-by-default='true'?>
2+
<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout" xmlns:st="jelly:stapler">
3+
<l:layout title="The Root">
4+
<l:main-panel>
5+
<h1>Root Action</h1>
6+
<st:bind var="varname" value="${it}"/>
7+
<st:adjunct includes="org.kohsuke.stapler.BindTest.RootActionImpl.adjunct"/>
8+
</l:main-panel>
9+
</l:layout>
10+
</j:jelly>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?jelly escape-by-default='true'?>
2+
<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout" xmlns:st="jelly:stapler">
3+
<l:layout title="The Root">
4+
<l:main-panel>
5+
<h1>Root Action</h1>
6+
<script>
7+
var inline = <st:bind value="${null}"/>
8+
</script>
9+
</l:main-panel>
10+
</l:layout>
11+
</j:jelly>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?jelly escape-by-default='true'?>
2+
<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout" xmlns:st="jelly:stapler">
3+
<l:layout title="The Root">
4+
<l:main-panel>
5+
<h1>Root Action with null</h1>
6+
<st:bind var="varname" value="${null}"/>
7+
<st:adjunct includes="org.kohsuke.stapler.BindTest.RootActionImpl.adjunct"/>
8+
</l:main-panel>
9+
</l:layout>
10+
</j:jelly>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?jelly escape-by-default='true'?>
2+
<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout" xmlns:st="jelly:stapler">
3+
<l:layout title="The Root">
4+
<l:main-panel>
5+
<h1>Root Action</h1>
6+
<st:bind var="window['varname']" value="${it}"/>
7+
<st:adjunct includes="org.kohsuke.stapler.BindTest.RootActionImpl.adjunct"/>
8+
</l:main-panel>
9+
</l:layout>
10+
</j:jelly>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// eslint-disable-next-line no-undef
2+
varname.byName2();
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?jelly escape-by-default='true'?>
2+
<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout" xmlns:st="jelly:stapler">
3+
<l:layout title="The Root">
4+
<l:main-panel>
5+
<h1>Root Action</h1>
6+
<st:bind var="varname" value="${it}"/>
7+
<st:adjunct includes="org.kohsuke.stapler.BindTest.RootActionWithWellKnownURL.adjunct"/>
8+
</l:main-panel>
9+
</l:layout>
10+
</j:jelly>

war/src/main/webapp/scripts/hudson-behavior.js

+7-1
Original file line numberDiff line numberDiff line change
@@ -997,7 +997,13 @@ function renderOnDemand(e, callback, noBehaviour) {
997997
if (!e || !e.classList.contains("render-on-demand")) {
998998
return;
999999
}
1000-
var proxy = eval(e.getAttribute("proxy"));
1000+
1001+
let proxyMethod = e.getAttribute("data-proxy-method");
1002+
let proxyUrl = e.getAttribute("data-proxy-url");
1003+
let proxyCrumb = e.getAttribute("data-proxy-crumb");
1004+
let proxyUrlNames = e.getAttribute("data-proxy-url-names").split(",");
1005+
1006+
var proxy = window[proxyMethod](proxyUrl, proxyCrumb, proxyUrlNames);
10011007
proxy.render(function (t) {
10021008
var contextTagName = e.parentNode.tagName;
10031009
var c;

0 commit comments

Comments
 (0)