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-53511] Improve discovery and readability of WebClient #3618

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ THE SOFTWARE.
<dependency>
<groupId>commons-io</groupId>
<artifactId>commons-io</artifactId>
<version>2.4</version>
<version>2.6</version>
</dependency>

<dependency>
Expand Down
2 changes: 1 addition & 1 deletion test-pom/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ THE SOFTWARE.
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>jenkins-test-harness</artifactId>
<version>2.41.1</version>
<version>2.42</version>
<scope>test</scope>
<exclusions>
<exclusion>
Expand Down
10 changes: 6 additions & 4 deletions test/src/test/java/hudson/AboutJenkinsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
import org.jvnet.hudson.test.MockAuthorizationStrategy;
import org.jvnet.hudson.test.SmokeTest;

import java.net.HttpURLConnection;

import static org.hamcrest.Matchers.containsString;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
Expand All @@ -55,19 +57,19 @@ public void onlyAdminCanReadAbout() throws Exception {
.grant(Jenkins.READ).everywhere().to(USER)
);

JenkinsRule.WebClient wc = j.createWebClient();
wc.getOptions().setThrowExceptionOnFailingStatusCode(false);
JenkinsRule.WebClient wc = j.createWebClient()
.withThrowExceptionOnFailingStatusCode(false);

{ // user cannot see it
wc.login(USER);
HtmlPage page = wc.goTo("about/");
assertEquals(403, page.getWebResponse().getStatusCode());
assertEquals(HttpURLConnection.HTTP_FORBIDDEN, page.getWebResponse().getStatusCode());
}

{ // admin can access it
wc.login(ADMIN);
HtmlPage page = wc.goTo("about/");
assertEquals(200, page.getWebResponse().getStatusCode());
assertEquals(HttpURLConnection.HTTP_OK, page.getWebResponse().getStatusCode());
assertThat(page.getWebResponse().getContentAsString(), containsString("Mavenized dependencies"));
}
}
Expand Down
48 changes: 16 additions & 32 deletions test/src/test/java/hudson/TcpSlaveAgentListenerTest.java
Original file line number Diff line number Diff line change
@@ -1,34 +1,20 @@
package hudson;

import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
import com.gargoylesoftware.htmlunit.Page;
import com.gargoylesoftware.htmlunit.TextPage;
import com.gargoylesoftware.htmlunit.html.HtmlPage;
import hudson.remoting.Base64;

import java.io.IOException;
import java.security.KeyFactory;
import java.security.KeyPair;
import java.security.KeyPairGenerator;
import java.security.NoSuchAlgorithmException;
import java.security.cert.X509Certificate;
import java.security.interfaces.RSAPrivateKey;
import java.security.interfaces.RSAPublicKey;
import java.security.spec.InvalidKeySpecException;
import java.security.spec.PKCS8EncodedKeySpec;
import javax.annotation.Nullable;
import java.net.HttpURLConnection;
import java.net.URL;

import jenkins.model.Jenkins;
import jenkins.model.identity.InstanceIdentityProvider;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.JenkinsRule.WebClient;
import org.jvnet.hudson.test.TestExtension;

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

public class TcpSlaveAgentListenerTest {

Expand All @@ -37,15 +23,15 @@ public class TcpSlaveAgentListenerTest {

@Test
public void headers() throws Exception {
WebClient wc = r.createWebClient()
.withThrowExceptionOnFailingStatusCode(false);

r.getInstance().setSlaveAgentPort(-1);
try {
r.createWebClient().goTo("tcpSlaveAgentListener");
fail("Should get 404");
} catch (FailingHttpStatusCodeException e) {
assertThat(e.getStatusCode(), is(404));
}
wc.assertFails("tcpSlaveAgentListener", HttpURLConnection.HTTP_NOT_FOUND);

r.getInstance().setSlaveAgentPort(0);
Page p = r.createWebClient().goTo("tcpSlaveAgentListener", "text/plain");
Page p = wc.goTo("tcpSlaveAgentListener", "text/plain");
assertEquals(HttpURLConnection.HTTP_OK, p.getWebResponse().getStatusCode());
assertThat(p.getWebResponse().getResponseHeaderValue("X-Instance-Identity"), notNullValue());
}

Expand All @@ -54,15 +40,13 @@ public void diagnostics() throws Exception {
r.getInstance().setSlaveAgentPort(0);
int p = r.jenkins.getTcpSlaveAgentListener().getPort();
WebClient wc = r.createWebClient();
TextPage text = (TextPage) wc.getPage("http://localhost:"+p+"/");

TextPage text = wc.getPage(new URL("http://localhost:" + p + "/"));
String c = text.getContent();
assertThat(c,containsString(Jenkins.VERSION));
assertThat(c, containsString(Jenkins.VERSION));

try {
wc.getPage("http://localhost:"+p+"/xxx");
fail("Expected 404");
} catch (FailingHttpStatusCodeException e) {
assertThat(e.getStatusCode(),equalTo(404));
}
wc.setThrowExceptionOnFailingStatusCode(false);
Page page = wc.getPage(new URL("http://localhost:" + p + "/xxx"));
assertEquals(HttpURLConnection.HTTP_NOT_FOUND, page.getWebResponse().getStatusCode());
}
}
12 changes: 7 additions & 5 deletions test/src/test/java/hudson/cli/CLITest.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.io.File;
import java.io.IOException;
import java.io.PrintWriter;
import java.net.HttpURLConnection;
import java.nio.file.Files;
import java.util.Arrays;
import java.util.List;
Expand Down Expand Up @@ -234,11 +235,12 @@ public void redirectToEndpointShouldBeFollowed() throws Exception {
sshd.start();

// Sanity check
JenkinsRule.WebClient wc = r.createWebClient();
wc.getOptions().setRedirectEnabled(false);
wc.getOptions().setThrowExceptionOnFailingStatusCode(false);
JenkinsRule.WebClient wc = r.createWebClient()
.withRedirectEnabled(false)
.withThrowExceptionOnFailingStatusCode(false);

WebResponse rsp = wc.goTo("cli-proxy/").getWebResponse();
assertEquals(rsp.getContentAsString(), 302, rsp.getStatusCode());
assertEquals(rsp.getContentAsString(), HttpURLConnection.HTTP_MOVED_TEMP, rsp.getStatusCode());
assertEquals(rsp.getContentAsString(), null, rsp.getResponseHeaderValue("X-Jenkins"));
assertEquals(rsp.getContentAsString(), null, rsp.getResponseHeaderValue("X-Jenkins-CLI-Port"));
assertEquals(rsp.getContentAsString(), null, rsp.getResponseHeaderValue("X-SSH-Endpoint"));
Expand Down Expand Up @@ -308,7 +310,7 @@ public HttpResponses.HttpResponseException doDynamic(StaplerRequest req, Stapler
public void generateResponse(StaplerRequest req, StaplerResponse rsp, Object node) throws IOException, ServletException {
rsp.setHeader("Location", url);
rsp.setContentType("text/html");
rsp.setStatus(302);
rsp.setStatus(HttpURLConnection.HTTP_MOVED_TEMP);
PrintWriter w = rsp.getWriter();
w.append("Redirect to ").append(url);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
import com.gargoylesoftware.htmlunit.HttpMethod;
import com.gargoylesoftware.htmlunit.Page;
import com.gargoylesoftware.htmlunit.WebRequest;
import com.gargoylesoftware.htmlunit.util.NameValuePair;
import hudson.model.User;
import hudson.security.GlobalMatrixAuthorizationStrategy;
import jenkins.model.Jenkins;
import jenkins.security.apitoken.ApiTokenPropertyConfiguration;
import jenkins.security.apitoken.ApiTokenTestHelper;
import org.junit.Rule;
import org.junit.Test;
Expand All @@ -24,6 +23,7 @@
import com.gargoylesoftware.htmlunit.ElementNotFoundException;

import java.io.IOException;
import java.net.HttpURLConnection;
import java.net.URL;
import java.util.Collections;

Expand Down Expand Up @@ -63,7 +63,8 @@ public void flow() throws Exception {
public void noAccessForNonAdmin() throws Exception {
ApiTokenTestHelper.enableLegacyBehavior();

JenkinsRule.WebClient wc = j.createWebClient();
JenkinsRule.WebClient wc = j.createWebClient()
.withThrowExceptionOnFailingStatusCode(false);

// TODO: Use MockAuthorizationStrategy in later versions
JenkinsRule.DummySecurityRealm realm = j.createDummySecurityRealm();
Expand All @@ -85,26 +86,19 @@ public void noAccessForNonAdmin() throws Exception {
HudsonHomeDiskUsageMonitor mon = HudsonHomeDiskUsageMonitor.get();

wc.withBasicApiToken(bob);
try {
wc.getPage(request);
fail();
} catch (FailingHttpStatusCodeException e) {
assertEquals(403, e.getStatusCode());
}
Page p = wc.getPage(request);
assertEquals(HttpURLConnection.HTTP_FORBIDDEN, p.getWebResponse().getStatusCode());

assertTrue(mon.isEnabled());

WebRequest requestReadOnly = new WebRequest(new URL(wc.getContextPath() + "administrativeMonitor/hudsonHomeIsFull"), HttpMethod.GET);
try {
wc.getPage(requestReadOnly);
fail();
} catch (FailingHttpStatusCodeException e) {
assertEquals(403, e.getStatusCode());
}
p = wc.getPage(requestReadOnly);
assertEquals(HttpURLConnection.HTTP_FORBIDDEN, p.getWebResponse().getStatusCode());

wc.withBasicApiToken(administrator);
wc.getPage(request);
p = wc.getPage(request);
assertEquals(HttpURLConnection.HTTP_OK, p.getWebResponse().getStatusCode());
assertFalse(mon.isEnabled());

}

/**
Expand Down
44 changes: 22 additions & 22 deletions test/src/test/java/hudson/jobs/CreateItemTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,22 @@
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.*;

import hudson.AbortException;
import com.gargoylesoftware.htmlunit.Page;
import hudson.model.Failure;
import hudson.model.Item;
import hudson.model.ItemGroup;
import hudson.model.listeners.ItemListener;

import java.net.HttpURLConnection;
import java.net.URL;
import java.text.MessageFormat;

import org.acegisecurity.AccessDeniedException;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;

import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
import com.gargoylesoftware.htmlunit.HttpMethod;
import com.gargoylesoftware.htmlunit.WebRequest;
import hudson.model.FreeStyleProject;
Expand All @@ -54,7 +54,6 @@
* @author Christopher Simons
*/
public class CreateItemTest {
private static final int ERROR_PRESET = (-1);
@Rule
public JenkinsRule rule = new JenkinsRule();

Expand All @@ -78,15 +77,13 @@ public void testCreateItemFromCopy() throws Exception {

WebRequest request = new WebRequest(apiURL, HttpMethod.POST);
deleteContentTypeHeader(request);
int result = ERROR_PRESET;
try {
result = rule.createWebClient()
.getPage(request).getWebResponse().getStatusCode();
} catch (FailingHttpStatusCodeException e) {
result = e.getResponse().getStatusCode();
}

assertEquals("Creating job from copy should succeed.", 200, result);
Page p = rule.createWebClient()
.withThrowExceptionOnFailingStatusCode(false)
.getPage(request);
assertEquals("Creating job from copy should succeed.",
HttpURLConnection.HTTP_OK,
p.getWebResponse().getStatusCode());
}

@Issue("JENKINS-34691")
Expand All @@ -104,15 +101,14 @@ public void vetoCreateItemFromCopy() throws Exception {

WebRequest request = new WebRequest(apiURL, HttpMethod.POST);
deleteContentTypeHeader(request);
int result = ERROR_PRESET;
try {
result = rule.createWebClient()
.getPage(request).getWebResponse().getStatusCode();
} catch (FailingHttpStatusCodeException e) {
result = e.getResponse().getStatusCode();
}

assertEquals("Creating job from copy should fail.", 400, result);
Page p = rule.createWebClient()
.withThrowExceptionOnFailingStatusCode(false)
.getPage(request);

assertEquals("Creating job from copy should fail.",
HttpURLConnection.HTTP_BAD_REQUEST,
p.getWebResponse().getStatusCode());
assertThat(rule.jenkins.getItem("newJob"), nullValue());
}

Expand All @@ -125,9 +121,13 @@ public void createWithFolderPaths() throws Exception {
rule.jenkins.setCrumbIssuer(null);
rule.createFolder("d1").createProject(FreeStyleProject.class, "p");
MockFolder d2 = rule.createFolder("d2");
rule.createWebClient().getPage(new WebRequest(new URL(d2.getAbsoluteUrl() + "createItem?mode=copy&name=p2&from=../d1/p"), HttpMethod.POST));

JenkinsRule.WebClient wc = rule.createWebClient();

wc.getPage(new WebRequest(new URL(d2.getAbsoluteUrl() + "createItem?mode=copy&name=p2&from=../d1/p"), HttpMethod.POST));
assertNotNull(d2.getItem("p2"));
rule.createWebClient().getPage(new WebRequest(new URL(d2.getAbsoluteUrl() + "createItem?mode=copy&name=p3&from=/d1/p"), HttpMethod.POST));

wc.getPage(new WebRequest(new URL(d2.getAbsoluteUrl() + "createItem?mode=copy&name=p3&from=/d1/p"), HttpMethod.POST));
assertNotNull(d2.getItem("p3"));
}

Expand Down
16 changes: 8 additions & 8 deletions test/src/test/java/hudson/model/AbstractItemTest.java
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
package hudson.model;

import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
import com.gargoylesoftware.htmlunit.HttpMethod;
import com.gargoylesoftware.htmlunit.Page;
import com.gargoylesoftware.htmlunit.WebRequest;
import com.gargoylesoftware.htmlunit.util.NameValuePair;
import hudson.security.ACL;
import hudson.security.ACLContext;
import hudson.security.AccessDeniedException2;
import hudson.util.FormValidation;
import java.io.File;
import java.net.HttpURLConnection;
import java.net.URL;
import java.util.Arrays;
import jenkins.model.Jenkins;
Expand Down Expand Up @@ -117,18 +118,17 @@ public void renameViaRestApi() throws Exception {
WebRequest wr = new WebRequest(w.createCrumbedUrl(p.getUrl() + "confirmRename"), HttpMethod.POST);
wr.setRequestParameters(Arrays.asList(new NameValuePair("newName", "bar")));
w.login("alice", "alice");
assertThat(getPath(w.getPage(wr).getUrl()), equalTo(p.getUrl()));
Page page = w.getPage(wr);
assertThat(getPath(page.getUrl()), equalTo(p.getUrl()));
assertThat(p.getName(), equalTo("bar"));

wr = new WebRequest(w.createCrumbedUrl(p.getUrl() + "confirmRename"), HttpMethod.POST);
wr.setRequestParameters(Arrays.asList(new NameValuePair("newName", "baz")));
w.login("bob", "bob");
try {
assertThat(getPath(w.getPage(wr).getUrl()), equalTo(p.getUrl()));
fail("Expecting HTTP 403 Forbidden");
} catch (FailingHttpStatusCodeException e) {
assertThat(e.getStatusCode(), equalTo(403));
}

w.setThrowExceptionOnFailingStatusCode(false);
page = w.getPage(wr);
assertEquals(HttpURLConnection.HTTP_FORBIDDEN, page.getWebResponse().getStatusCode());
assertThat(p.getName(), equalTo("bar"));
}

Expand Down
Loading