-
Notifications
You must be signed in to change notification settings - Fork 403
Convert tests to com.sun.net.httpserver
#394
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ | |
| <parent> | ||
| <groupId>org.jenkins-ci.plugins</groupId> | ||
| <artifactId>plugin</artifactId> | ||
| <version>5.2</version> | ||
| <version>5.9</version> | ||
| <relativePath /> | ||
| </parent> | ||
|
|
||
|
|
@@ -39,7 +39,7 @@ | |
| <connection>scm:git:https://github.com/${gitHubRepo}.git</connection> | ||
| <developerConnection>scm:git:git@github.com:${gitHubRepo}.git</developerConnection> | ||
| <url>https://github.com/${gitHubRepo}</url> | ||
| <tag>v1.42.0</tag> | ||
| <tag>${scmTag}</tag> | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Matching the archetype, and also needed to get an incremental build for testing. |
||
| </scm> | ||
| <issueManagement> | ||
| <system>JIRA</system> | ||
|
|
@@ -52,7 +52,7 @@ | |
| <gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo> | ||
| <!-- https://www.jenkins.io/doc/developer/plugin-development/choosing-jenkins-baseline/ --> | ||
| <jenkins.baseline>2.479</jenkins.baseline> | ||
| <jenkins.version>${jenkins.baseline}.1</jenkins.version> | ||
| <jenkins.version>${jenkins.baseline}.3</jenkins.version> | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| <release.skipTests>false</release.skipTests> | ||
| <tagNameFormat>v@{project.version}</tagNameFormat> | ||
| <useBeta>true</useBeta> <!-- For Jenkins.MANAGE permission --> | ||
|
|
@@ -206,7 +206,7 @@ | |
| <dependency> | ||
| <groupId>io.jenkins.tools.bom</groupId> | ||
| <artifactId>bom-${jenkins.baseline}.x</artifactId> | ||
| <version>3559.vb_5b_81183b_d23</version> | ||
| <version>4488.v7fe26526366e</version> | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| <scope>import</scope> | ||
| <type>pom</type> | ||
| </dependency> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,6 @@ | |
| import com.google.common.base.Predicate; | ||
| import com.google.common.hash.Hashing; | ||
| import edu.umd.cs.findbugs.annotations.NonNull; | ||
| import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; | ||
| import okhttp3.Cache; | ||
| import org.apache.commons.io.FileUtils; | ||
| import org.jenkinsci.plugins.github.GitHubPlugin; | ||
|
|
@@ -96,7 +95,6 @@ public static Path getBaseCacheDir() { | |
| * | ||
| * @param configs active server configs to exclude caches from cleanup | ||
| */ | ||
| @SuppressFBWarnings(value = "RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE") | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was needed in a previous version of SpotBugs, but it is not needed in the very latest version of SpotBugs (pulled in via the latest plugin parent POM). The latest version actually complains about an unnecessary suppression, so remove it here to fix that warning. |
||
| public static void clearRedundantCaches(List<GitHubServerConfig> configs) { | ||
| Path baseCacheDir = getBaseCacheDir(); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,18 +5,16 @@ | |
| import com.cloudbees.plugins.credentials.CredentialsScope; | ||
| import com.cloudbees.plugins.credentials.CredentialsStore; | ||
| import com.cloudbees.plugins.credentials.domains.Domain; | ||
| import com.sun.net.httpserver.HttpExchange; | ||
| import com.sun.net.httpserver.HttpHandler; | ||
| import com.sun.net.httpserver.HttpServer; | ||
| import net.sf.json.JSONObject; | ||
| import org.htmlunit.HttpMethod; | ||
| import org.htmlunit.Page; | ||
| import org.htmlunit.WebRequest; | ||
| import hudson.security.GlobalMatrixAuthorizationStrategy; | ||
| import hudson.util.Secret; | ||
| import jenkins.model.Jenkins; | ||
| import net.sf.json.JSONObject; | ||
| import org.eclipse.jetty.server.Server; | ||
| import org.eclipse.jetty.server.ServerConnector; | ||
| import org.eclipse.jetty.ee9.servlet.DefaultServlet; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No more references to a specific EE version, making this test robust against future EE upgrades (including EE 10). |
||
| import org.eclipse.jetty.ee9.servlet.ServletContextHandler; | ||
| import org.eclipse.jetty.ee9.servlet.ServletHolder; | ||
| import org.jenkinsci.plugins.plaincredentials.impl.StringCredentialsImpl; | ||
| import org.junit.After; | ||
| import org.junit.Before; | ||
|
|
@@ -26,10 +24,13 @@ | |
| import org.jvnet.hudson.test.Issue; | ||
| import org.jvnet.hudson.test.JenkinsRule; | ||
|
|
||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| import java.io.IOException; | ||
| import java.io.OutputStream; | ||
| import java.net.HttpURLConnection; | ||
| import java.net.InetAddress; | ||
| import java.net.InetSocketAddress; | ||
| import java.net.URL; | ||
| import java.nio.charset.StandardCharsets; | ||
| import java.util.HashMap; | ||
|
|
||
| import static org.hamcrest.MatcherAssert.assertThat; | ||
|
|
@@ -46,7 +47,7 @@ public class GitHubServerConfigIntegrationTest { | |
| @Rule | ||
| public JenkinsRule j = new JenkinsRule(); | ||
|
|
||
| private Server server; | ||
| private HttpServer server; | ||
| private AttackerServlet attackerServlet; | ||
| private String attackerUrl; | ||
|
|
||
|
|
@@ -57,35 +58,16 @@ public void setupServer() throws Exception { | |
|
|
||
| @After | ||
| public void stopServer() { | ||
| try { | ||
| server.stop(); | ||
| } catch (Exception e) { | ||
| e.printStackTrace(); | ||
| } | ||
| server.stop(1); | ||
| } | ||
|
|
||
| private void setupAttackerServer() throws Exception { | ||
| this.server = new Server(); | ||
| ServerConnector serverConnector = new ServerConnector(this.server); | ||
| server.addConnector(serverConnector); | ||
|
|
||
| ServletContextHandler context = new ServletContextHandler(ServletContextHandler.NO_SESSIONS); | ||
| context.setContextPath("/*"); | ||
|
|
||
| this.server = HttpServer.create(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0); | ||
| this.attackerServlet = new AttackerServlet(); | ||
| ServletHolder servletHolder = new ServletHolder(attackerServlet); | ||
| context.addServlet(servletHolder, "/*"); | ||
|
|
||
| server.setHandler(context); | ||
|
|
||
| server.start(); | ||
|
|
||
| String host = serverConnector.getHost(); | ||
| if (host == null) { | ||
| host = "localhost"; | ||
| } | ||
|
|
||
| this.attackerUrl = "http://" + host + ":" + serverConnector.getLocalPort(); | ||
| this.server.createContext("/user", this.attackerServlet); | ||
| this.server.start(); | ||
| InetSocketAddress addr = this.server.getAddress(); | ||
| this.attackerUrl = String.format("http://%s:%d", addr.getHostString(), addr.getPort()); | ||
| } | ||
|
|
||
| @Test | ||
|
|
@@ -153,25 +135,30 @@ private void setupCredentials(String credentialId, String secret) throws Excepti | |
| store.addCredentials(domain, credentials); | ||
| } | ||
|
|
||
| private static class AttackerServlet extends DefaultServlet { | ||
| private static class AttackerServlet implements HttpHandler { | ||
| public String secretCreds; | ||
|
|
||
| @Override | ||
| protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException { | ||
| switch (request.getRequestURI()) { | ||
| case "/user": | ||
| this.onUser(request, response); | ||
| break; | ||
| public void handle(HttpExchange he) throws IOException { | ||
| if ("GET".equals(he.getRequestMethod())) { | ||
| this.onUser(he); | ||
| } else { | ||
| he.sendResponseHeaders(HttpURLConnection.HTTP_BAD_METHOD, -1); | ||
| } | ||
| } | ||
|
|
||
| private void onUser(HttpServletRequest request, HttpServletResponse response) throws IOException { | ||
| secretCreds = request.getHeader("Authorization"); | ||
| response.getWriter().write(JSONObject.fromObject( | ||
| private void onUser(HttpExchange he) throws IOException { | ||
| secretCreds = he.getRequestHeaders().getFirst("Authorization"); | ||
| String response = JSONObject.fromObject( | ||
| new HashMap<String, Object>() {{ | ||
| put("login", "alice"); | ||
| }} | ||
| ).toString()); | ||
| ).toString(); | ||
| byte[] body = response.getBytes(StandardCharsets.UTF_8); | ||
| he.sendResponseHeaders(HttpURLConnection.HTTP_OK, body.length); | ||
| try (OutputStream os = he.getResponseBody()) { | ||
| os.write(body); | ||
| } | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not strictly necessary for this PR, but taking advantage of the build/test cycle to update this plugin's metadata while I am here.