Skip to content

Commit e6df3bd

Browse files
author
MerkushevKirill
committed
throw and catch config exception on mailformed hook url in global configuration
1 parent 09add52 commit e6df3bd

5 files changed

Lines changed: 107 additions & 14 deletions

File tree

‎src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import org.apache.commons.codec.binary.Base64;
2121
import org.apache.commons.jelly.XMLOutput;
2222
import org.jenkinsci.main.modules.instance_identity.InstanceIdentity;
23-
import org.jenkinsci.plugins.github.webhook.WebhookManager;
23+
import org.jenkinsci.plugins.github.internal.GHPluginConfigException;
2424
import org.kohsuke.stapler.DataBoundConstructor;
2525
import org.kohsuke.stapler.QueryParameter;
2626
import org.kohsuke.stapler.StaplerRequest;
@@ -152,10 +152,17 @@ public void start(AbstractProject<?, ?> project, boolean newInstance) {
152152
* Tries to register hook for current associated job.
153153
* Do this lazily to avoid blocking the UI thread.
154154
* Useful for using from groovy scripts.
155+
*
155156
* @since 1.11.2
156157
*/
157158
public void registerHooks() {
158-
URL hookUrl = getDescriptor().getHookUrl();
159+
URL hookUrl;
160+
try {
161+
hookUrl = getDescriptor().getHookUrl();
162+
} catch (GHPluginConfigException e) {
163+
LOGGER.log(Level.SEVERE, "Skip registration of GHHook ({0})", e.getMessage());
164+
return;
165+
}
159166
Runnable hookRegistrator = forHookUrl(hookUrl).registerFor(job);
160167
getDescriptor().queue.execute(hookRegistrator);
161168
}
@@ -207,6 +214,7 @@ public String getLog() throws IOException {
207214

208215
/**
209216
* Writes the annotated log to the given output.
217+
*
210218
* @since 1.350
211219
*/
212220
public void writeLogTo(XMLOutput out) throws IOException {
@@ -255,13 +263,15 @@ public void setManageHook(boolean v) {
255263
/**
256264
* Returns the URL that GitHub should post.
257265
*/
258-
public URL getHookUrl() {
266+
public URL getHookUrl() throws GHPluginConfigException {
259267
try {
260268
return hookUrl != null
261269
? new URL(hookUrl)
262270
: new URL(Jenkins.getInstance().getRootUrl() + GitHubWebHook.get().getUrlName() + '/');
263271
} catch (MalformedURLException e) {
264-
throw new RuntimeException("Hook url is malformed", e);
272+
throw new GHPluginConfigException(
273+
"Mailformed GH hook url in global configuration (%s)", e.getMessage()
274+
);
265275
}
266276
}
267277

‎src/main/java/org/jenkinsci/plugins/github/internal/GHPluginConfigException.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
package org.jenkinsci.plugins.github.internal;
2+
3+
/**
4+
* @author lanwen (Merkushev Kirill)
5+
*/
6+
public class GHPluginConfigException extends RuntimeException {
7+
public GHPluginConfigException(String message, Object... args) {
8+
super(String.format(message, args));
9+
}
10+
}

‎src/test/java/com/cloudbees/jenkins/GitHubPushTriggerConfigSubmitTest.java‎

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,30 +3,44 @@
33
import com.gargoylesoftware.htmlunit.html.HtmlForm;
44
import com.gargoylesoftware.htmlunit.html.HtmlPage;
55
import hudson.util.Secret;
6+
import org.jenkinsci.plugins.github.internal.GHPluginConfigException;
7+
import org.junit.Rule;
8+
import org.junit.Test;
9+
import org.jvnet.hudson.test.JenkinsRule;
10+
import org.jvnet.hudson.test.recipes.LocalData;
11+
import org.kohsuke.stapler.Stapler;
12+
613
import java.net.URL;
714
import java.util.List;
8-
import org.jvnet.hudson.test.HudsonTestCase;
9-
import org.kohsuke.stapler.Stapler;
15+
16+
import static org.junit.Assert.assertEquals;
17+
import static org.junit.Assert.assertFalse;
18+
import static org.junit.Assert.assertNotNull;
19+
import static org.junit.Assert.assertTrue;
1020

1121
/**
1222
* Test Class for {@link GitHubPushTrigger}.
1323
*
1424
* @author Seiji Sogabe
1525
*/
16-
public class GitHubPushTriggerConfigSubmitTest extends HudsonTestCase {
26+
public class GitHubPushTriggerConfigSubmitTest {
27+
28+
@Rule
29+
public JenkinsRule jenkins = new JenkinsRule();
1730

1831
private static final String WEBHOOK_URL = "http://jenkinsci.example.com/jenkins/github-webhook/";
1932

33+
@Test
2034
public void testConfigSubmit_AutoManageHook() throws Exception {
2135

22-
WebClient client = configureWebClient();
36+
JenkinsRule.WebClient client = configureWebClient();
2337
HtmlPage p = client.goTo("configure");
2438
HtmlForm f = p.getFormByName("config");
2539
f.getInputByValue("auto").setChecked(true);
2640
f.getInputByName("_.hasHookUrl").setChecked(true);
2741
f.getInputByName("_.hookUrl").setValueAttribute(WEBHOOK_URL);
2842
f.getInputByName("_.username").setValueAttribute("jenkins");
29-
submit(f);
43+
f.submit();
3044

3145
GitHubPushTrigger.DescriptorImpl d = getDescriptor();
3246
assertTrue(d.isManageHook());
@@ -39,24 +53,36 @@ public void testConfigSubmit_AutoManageHook() throws Exception {
3953
assertEquals("jenkins", credential.username);
4054
}
4155

56+
@Test
4257
public void testConfigSubmit_ManuallyManageHook() throws Exception {
43-
44-
WebClient client = configureWebClient();
58+
JenkinsRule.WebClient client = configureWebClient();
4559
HtmlPage p = client.goTo("configure");
4660
HtmlForm f = p.getFormByName("config");
4761
f.getInputByValue("none").setChecked(true);
48-
submit(f);
62+
f.submit();
4963

5064
GitHubPushTrigger.DescriptorImpl d = getDescriptor();
5165
assertFalse(d.isManageHook());
5266
}
5367

68+
@Test
69+
@LocalData
70+
public void shouldDontThrowExcMailformedHookUrl() {
71+
new GitHubPushTrigger().registerHooks();
72+
}
73+
74+
@Test(expected = GHPluginConfigException.class)
75+
@LocalData
76+
public void shouldThrowExcMailformedHookUrlGetter() {
77+
new GitHubPushTrigger().getDescriptor().getHookUrl();
78+
}
79+
5480
private GitHubPushTrigger.DescriptorImpl getDescriptor() {
5581
return (GitHubPushTrigger.DescriptorImpl) GitHubPushTrigger.DescriptorImpl.get();
5682
}
5783

58-
private WebClient configureWebClient() {
59-
WebClient client = new WebClient();
84+
private JenkinsRule.WebClient configureWebClient() {
85+
JenkinsRule.WebClient client = jenkins.createWebClient();
6086
client.setThrowExceptionOnFailingStatusCode(false);
6187
client.setCssEnabled(false);
6288
client.setJavaScriptEnabled(true);

‎src/test/resources/com/cloudbees/jenkins/GitHubPushTriggerConfigSubmitTest/com.cloudbees.jenkins.GitHubPushTrigger.xml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?xml version='1.0' encoding='UTF-8'?>
2+
<com.cloudbees.jenkins.GitHubPushTrigger_-DescriptorImpl plugin="github@1.11.4-SNAPSHOT">
3+
<manageHook>true</manageHook>
4+
<hookUrl>h</hookUrl>
5+
<credentials>
6+
<com.cloudbees.jenkins.Credential>
7+
<username>user</username>
8+
<apiUrl></apiUrl>
9+
<oauthAccessToken>some-oauth-token</oauthAccessToken>
10+
</com.cloudbees.jenkins.Credential>
11+
</credentials>
12+
</com.cloudbees.jenkins.GitHubPushTrigger_-DescriptorImpl>

‎src/test/resources/com/cloudbees/jenkins/GitHubPushTriggerConfigSubmitTest/config.xml‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?xml version='1.0' encoding='UTF-8'?>
2+
<hudson>
3+
<disabledAdministrativeMonitors/>
4+
<version>1.554.1</version>
5+
<numExecutors>2</numExecutors>
6+
<mode>NORMAL</mode>
7+
<useSecurity>true</useSecurity>
8+
<authorizationStrategy class="hudson.security.AuthorizationStrategy$Unsecured"/>
9+
<securityRealm class="hudson.security.SecurityRealm$None"/>
10+
<disableRememberMe>false</disableRememberMe>
11+
<projectNamingStrategy class="jenkins.model.ProjectNamingStrategy$DefaultProjectNamingStrategy"/>
12+
<workspaceDir>${JENKINS_HOME}/workspace/${ITEM_FULLNAME}</workspaceDir>
13+
<buildsDir>${ITEM_ROOTDIR}/builds</buildsDir>
14+
<jdks/>
15+
<viewsTabBar class="hudson.views.DefaultViewsTabBar"/>
16+
<myViewsTabBar class="hudson.views.DefaultMyViewsTabBar"/>
17+
<clouds/>
18+
<slaves/>
19+
<quietPeriod>5</quietPeriod>
20+
<scmCheckoutRetryCount>0</scmCheckoutRetryCount>
21+
<views>
22+
<hudson.model.AllView>
23+
<owner class="hudson" reference="../../.."/>
24+
<name>Все</name>
25+
<filterExecutors>false</filterExecutors>
26+
<filterQueue>false</filterQueue>
27+
<properties class="hudson.model.View$PropertyList"/>
28+
</hudson.model.AllView>
29+
</views>
30+
<primaryView>Все</primaryView>
31+
<slaveAgentPort>0</slaveAgentPort>
32+
<label></label>
33+
<nodeProperties/>
34+
<globalNodeProperties/>
35+
</hudson>

0 commit comments

Comments
 (0)