Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-70931] Remove Prototype Ajax.Request usages from hudson-behavior.js #7951

Merged
merged 18 commits into from May 11, 2023
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -24,7 +24,7 @@ THE SOFTWARE.
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form" xmlns:st="jelly:stapler" xmlns:t="/hudson/tools">
<f:entry title="${%Command}" field="command">
<f:textarea checkMethod="post"/>
<f:textarea />
</f:entry>
<f:entry title="${%Tool Home}" field="toolHome">
<f:textbox/>
Expand Down
Expand Up @@ -24,7 +24,7 @@ THE SOFTWARE.
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form" xmlns:st="jelly:stapler" xmlns:t="/hudson/tools">
<f:entry title="${%Download URL for binary archive}" field="url">
<f:textbox checkMethod="post"/>
<f:textbox />
</f:entry>
<f:entry title="${%Subdirectory of extracted archive}" field="subdir">
<f:textbox/>
Expand Down
Expand Up @@ -26,7 +26,7 @@ THE SOFTWARE.
<j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form">
<f:section title="${%Serve resource files from another domain}">
<f:entry title="${%Resource Root URL}" field="url">
<f:textbox checkMethod="post"/>
<f:textbox />
</f:entry>
</f:section>
</j:jelly>
5 changes: 0 additions & 5 deletions core/src/main/resources/lib/form/combobox.jelly
Expand Up @@ -62,11 +62,6 @@ THE SOFTWARE.
If @field is specified, this will be inferred automatically,
which is the recommended approach.
</st:attribute>
<st:attribute name="checkMethod" use="optional" type="String">
Specify 'get' (must be lowercase) to change the HTTP method used for the AJAX requests to @checkUrl from a POST to a GET.
If any other value is specified then requests will use POST.
The historical default was GET and 'post' had to be specified to change that, but this was changed in Jenkins 2.285.
</st:attribute>
</st:documentation>
<f:prepareDatabinding/>
${descriptor.calcFillSettings(field,attrs)} <!-- this figures out the 'fillUrl' and 'fillDependsOn' attribute -->
Expand Down
5 changes: 0 additions & 5 deletions core/src/main/resources/lib/form/number.jelly
Expand Up @@ -80,11 +80,6 @@ THE SOFTWARE.
If @field is specified, this will be inferred automatically,
which is the recommended approach.
</st:attribute>
<st:attribute name="checkMethod" use="optional" type="String">
Specify 'get' (must be lowercase) to change the HTTP method used for the AJAX requests to @checkUrl from a POST to a GET.
If any other value is specified then requests will use POST.
The historical default was GET and 'post' had to be specified to change that, but this was changed in Jenkins 2.285.
</st:attribute>
</st:documentation>
<f:prepareDatabinding />

Expand Down
3 changes: 0 additions & 3 deletions core/src/main/resources/lib/form/password.jelly
Expand Up @@ -83,7 +83,6 @@ THE SOFTWARE.
name="${attrs.name ?: '_.'+attrs.field}"
value="${value}"
type="hidden"
checkMethod="post"
ATTRIBUTES="${attrs}" EXCEPT="field clazz value"/>
<div class="hidden-password-placeholder">
<div class="hidden-password-legend">
Expand All @@ -103,7 +102,6 @@ THE SOFTWARE.
value="${value}"
type="text"
oninput="this.setAttribute('type', 'password'); return true;"
checkMethod="post"
ATTRIBUTES="${attrs}" EXCEPT="field clazz value"/>
<!-- TODO consider customizedFields -->
</j:otherwise>
Expand All @@ -115,7 +113,6 @@ THE SOFTWARE.
name="${attrs.name ?: '_.'+attrs.field}"
value="${h.getPasswordValue(resolvedValue)}"
type="password"
checkMethod="post"
ATTRIBUTES="${attrs}" EXCEPT="field clazz value"/>
</j:otherwise>
</j:choose>
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/resources/lib/form/readOnlyTextbox.jelly
Expand Up @@ -70,7 +70,7 @@ THE SOFTWARE.
id="${attrs.id}"
type="text"
readonly="readonly"
checkUrl="${attrs.checkUrl}" checkDependsOn="${attrs.checkDependsOn}" checkMethod="${attrs.checkMethod}"
checkUrl="${attrs.checkUrl}" checkDependsOn="${attrs.checkDependsOn}"
checkMessage="${attrs.checkMessage}"
onchange="${attrs.onchange}" onkeyup="${attrs.onkeyup}"/>
</j:jelly>
5 changes: 0 additions & 5 deletions core/src/main/resources/lib/form/select.jelly
Expand Up @@ -51,11 +51,6 @@ THE SOFTWARE.
If @field is specified, this will be inferred automatically,
which is the recommended approach.
</st:attribute>
<st:attribute name="checkMethod" use="optional" type="String">
Specify 'get' (must be lowercase) to change the HTTP method used for the AJAX requests to @checkUrl from a POST to a GET.
If any other value is specified then requests will use POST.
The historical default was GET and 'post' had to be specified to change that, but this was changed in Jenkins 2.285.
</st:attribute>
</st:documentation>

<f:prepareDatabinding/>
Expand Down
7 changes: 1 addition & 6 deletions core/src/main/resources/lib/form/textarea.jelly
Expand Up @@ -57,11 +57,6 @@ THE SOFTWARE.
If @field is specified, this will be inferred automatically,
which is the recommended approach.
</st:attribute>
<st:attribute name="checkMethod" use="optional" type="String">
Specify 'get' (must be lowercase) to change the HTTP method used for the AJAX requests to @checkUrl from a POST to a GET.
If any other value is specified then requests will use POST.
The historical default was GET and 'post' had to be specified to change that, but this was changed in Jenkins 2.285.
</st:attribute>
<st:attribute name="codemirror-mode">
Turns this text area into CodeMirror-assisted code editing text area.
This attribute specifies the mode of CodeMirror, such as "text/x-java".
Expand Down Expand Up @@ -93,7 +88,7 @@ THE SOFTWARE.
<textarea id="${attrs.id}" style="${attrs.style}"
name ="${name}"
class="jenkins-input ${attrs.checkUrl!=null?'validated':''} ${attrs['codemirror-mode']!=null?'codemirror':''} ${attrs.class}"
checkUrl="${attrs.checkUrl}" checkDependsOn="${attrs.checkDependsOn}" checkMethod="${attrs.checkMethod}"
checkUrl="${attrs.checkUrl}" checkDependsOn="${attrs.checkDependsOn}"
rows="${h.determineRows(value)}"
readonly="${attrs.readonly}"
codemirror-mode="${attrs['codemirror-mode']}"
Expand Down
5 changes: 0 additions & 5 deletions core/src/main/resources/lib/form/textbox.jelly
Expand Up @@ -77,11 +77,6 @@ THE SOFTWARE.
If @field is specified, this will be inferred automatically,
which is the recommended approach.
</st:attribute>
<st:attribute name="checkMethod" use="optional" type="String">
Specify 'get' (must be lowercase) to change the HTTP method used for the AJAX requests to @checkUrl from a POST to a GET.
If any other value is specified then requests will use POST.
The historical default was GET and 'post' had to be specified to change that, but this was changed in Jenkins 2.285.
</st:attribute>
<st:attribute name="autoCompleteUrl">
Url to use for auto-completion

Expand Down
102 changes: 0 additions & 102 deletions test/src/test/java/hudson/tools/ZipExtractionInstallerTest.java
Expand Up @@ -24,33 +24,16 @@

package hudson.tools;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsString;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;

import com.gargoylesoftware.htmlunit.HttpMethod;
import com.gargoylesoftware.htmlunit.WebRequest;
import com.gargoylesoftware.htmlunit.html.HtmlPage;
import com.gargoylesoftware.htmlunit.html.HtmlTextInput;
import com.gargoylesoftware.htmlunit.javascript.JavaScriptEngine;
import com.gargoylesoftware.htmlunit.javascript.host.xml.XMLHttpRequest;
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.Nullable;
import hudson.model.JDK;
import hudson.model.User;
import hudson.util.FormValidation;
import java.lang.reflect.Field;
import java.net.HttpURLConnection;
import java.net.URL;
import java.net.URLDecoder;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
import jenkins.model.Jenkins;
import net.sourceforge.htmlunit.corejs.javascript.Function;
import net.sourceforge.htmlunit.corejs.javascript.Scriptable;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
Expand Down Expand Up @@ -94,89 +77,4 @@ public void onlyAdminCanReachTheDoCheck() throws Exception {
userWc.login(USER);
assertEquals(HttpURLConnection.HTTP_FORBIDDEN, userWc.getPage(request).getWebResponse().getStatusCode());
}

@Test
@Issue("SECURITY-794")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this removed? Not relevant anymore?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was a very hacky test relying on internals of htmlunit which changed when switching to fetch I wasn’t able to replicate it and it didn’t seem like a critical test

public void roundtrip() throws Exception {
final String VALID_URL = "https://www.google.com";
final String INVALID_URL = "only-crappy-letters";

ZipExtractionInstaller installer = new ZipExtractionInstaller("", VALID_URL, "");

j.jenkins.getJDKs().add(new JDK("test", tmp.getRoot().getAbsolutePath(), List.of(
new InstallSourceProperty(List.of(installer)))));

JenkinsRule.WebClient wc = j.createWebClient();

SpyingJavaScriptEngine jsEngine = new SpyingJavaScriptEngine(wc, "ZipExtractionInstaller/checkUrl", HttpMethod.POST);
wc.setJavaScriptEngine(jsEngine);

HtmlPage page = wc.goTo("configureTools");

XMLHttpRequest lastRequest = jsEngine.getLastRequest();
String body = URLDecoder.decode(getPrivateWebRequestField(lastRequest).getRequestBody(), StandardCharsets.UTF_8);
assertThat(body, containsString(VALID_URL));
assertEquals(FormValidation.ok().renderHtml(), lastRequest.getResponseText());

HtmlTextInput urlInput = page.getDocumentElement().getOneHtmlElementByAttribute("input", "value", VALID_URL);
urlInput.setAttribute("value", INVALID_URL);
j.submit(page.getFormByName("config"));

JDK jdk = j.jenkins.getJDK("test");
InstallSourceProperty isp = jdk.getProperties().get(InstallSourceProperty.class);
assertEquals(1, isp.installers.size());
assertEquals(INVALID_URL, isp.installers.get(ZipExtractionInstaller.class).getUrl());

wc.goTo("configureTools");

lastRequest = jsEngine.getLastRequest();
body = URLDecoder.decode(getPrivateWebRequestField(lastRequest).getRequestBody(), StandardCharsets.UTF_8);
assertThat(body, containsString(INVALID_URL));
assertThat(lastRequest.getResponseText(), containsString(Messages.ZipExtractionInstaller_malformed_url()));
}

private static class SpyingJavaScriptEngine extends JavaScriptEngine {
private List<XMLHttpRequest> storedRequests = new ArrayList<>();
private String urlToMatch;
private HttpMethod method;

SpyingJavaScriptEngine(JenkinsRule.WebClient wc, @Nullable String urlToMatch, @Nullable HttpMethod method) {
super(wc);
this.urlToMatch = urlToMatch;
this.method = method;
}

@Override
public Object callFunction(HtmlPage page, Function function, Scriptable scope, Scriptable thisObject, Object[] args) {
if (thisObject instanceof XMLHttpRequest) {
try {
WebRequest request = getPrivateWebRequestField((XMLHttpRequest) thisObject);
boolean correctUrl = urlToMatch == null || request.getUrl().toString().contains(urlToMatch);
boolean correctMethod = method == null || request.getHttpMethod().equals(method);
if (correctUrl && correctMethod) {
if (((XMLHttpRequest) thisObject).getReadyState() == 4) {
storedRequests.add((XMLHttpRequest) thisObject);
}
}
} catch (NoSuchFieldException | IllegalAccessException e) {
throw new LinkageError(e.getMessage(), e);
}
}
return super.callFunction(page, function, scope, thisObject, args);
}

@NonNull
public XMLHttpRequest getLastRequest() {
if (storedRequests.isEmpty()) {
fail("There is no available requests for the proposed url/method");
}
return storedRequests.get(storedRequests.size() - 1);
}
}

private static WebRequest getPrivateWebRequestField(XMLHttpRequest xmlHttpRequest) throws NoSuchFieldException, IllegalAccessException {
Field webRequest_Field = XMLHttpRequest.class.getDeclaredField("webRequest_");
webRequest_Field.setAccessible(true);
return (WebRequest) webRequest_Field.get(xmlHttpRequest);
}
}
1 change: 1 addition & 0 deletions war/.eslintrc.js
Expand Up @@ -49,6 +49,7 @@ module.exports = {
makeButton: "readonly",
notificationBar: "readonly",
object: "readonly",
objectToUrlFormEncoded: "readonly",
onSetupWizardInitialized: "readonly",
Prototype: "readonly",
refillOnChange: "readonly",
Expand Down