Skip to content

Commit 555a752

Browse files
stephenclanwen
authored andcommitted
[FIXED JENKINS-39590] Switch to GitHub.parseEventPayload for event parsing
1 parent 2b7bbc9 commit 555a752

File tree

3 files changed

+65
-48
lines changed

3 files changed

+65
-48
lines changed

pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@
9393
<dependency>
9494
<groupId>org.jenkins-ci.plugins</groupId>
9595
<artifactId>github-api</artifactId>
96-
<version>1.69</version>
96+
<version>1.80</version>
9797
</dependency>
9898

9999
<dependency>

src/main/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventSubscriber.java

Lines changed: 41 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,14 @@
88
import hudson.Extension;
99
import hudson.model.Item;
1010
import hudson.security.ACL;
11+
import java.io.IOException;
12+
import java.io.StringReader;
13+
import java.net.URL;
1114
import jenkins.model.Jenkins;
12-
import net.sf.json.JSONObject;
1315
import org.jenkinsci.plugins.github.extension.GHEventsSubscriber;
1416
import org.kohsuke.github.GHEvent;
17+
import org.kohsuke.github.GHEventPayload;
18+
import org.kohsuke.github.GitHub;
1519
import org.slf4j.Logger;
1620
import org.slf4j.LoggerFactory;
1721

@@ -61,43 +65,49 @@ protected Set<GHEvent> events() {
6165
*/
6266
@Override
6367
protected void onEvent(GHEvent event, String payload) {
64-
JSONObject json = JSONObject.fromObject(payload);
65-
String repoUrl = json.getJSONObject("repository").getString("url");
66-
final String pusherName = json.getJSONObject("pusher").getString("name");
68+
try {
69+
GHEventPayload.Push push =
70+
GitHub.offline().parseEventPayload(new StringReader(payload), GHEventPayload.Push.class);
71+
URL repoUrl = push.getRepository().getUrl();
72+
final String pusherName = push.getPusher().getName();
73+
LOGGER.info("Received PushEvent for {}", repoUrl);
74+
final GitHubRepositoryName changedRepository = GitHubRepositoryName.create(repoUrl.toExternalForm());
6775

68-
LOGGER.info("Received POST for {}", repoUrl);
69-
final GitHubRepositoryName changedRepository = GitHubRepositoryName.create(repoUrl);
70-
71-
if (changedRepository != null) {
72-
// run in high privilege to see all the projects anonymous users don't see.
73-
// this is safe because when we actually schedule a build, it's a build that can
74-
// happen at some random time anyway.
75-
ACL.impersonate(ACL.SYSTEM, new Runnable() {
76-
@Override
77-
public void run() {
78-
for (Item job : Jenkins.getInstance().getAllItems(Item.class)) {
79-
GitHubTrigger trigger = triggerFrom(job, GitHubPushTrigger.class);
80-
if (trigger != null) {
81-
LOGGER.debug("Considering to poke {}", job.getFullDisplayName());
82-
if (GitHubRepositoryNameContributor.parseAssociatedNames(job).contains(changedRepository)) {
83-
LOGGER.info("Poked {}", job.getFullDisplayName());
84-
trigger.onPost(pusherName);
85-
} else {
86-
LOGGER.debug("Skipped {} because it doesn't have a matching repository.",
87-
job.getFullDisplayName());
76+
if (changedRepository != null) {
77+
// run in high privilege to see all the projects anonymous users don't see.
78+
// this is safe because when we actually schedule a build, it's a build that can
79+
// happen at some random time anyway.
80+
ACL.impersonate(ACL.SYSTEM, new Runnable() {
81+
@Override
82+
public void run() {
83+
for (Item job : Jenkins.getInstance().getAllItems(Item.class)) {
84+
GitHubTrigger trigger = triggerFrom(job, GitHubPushTrigger.class);
85+
if (trigger != null) {
86+
String fullDisplayName = job.getFullDisplayName();
87+
LOGGER.debug("Considering to poke {}", fullDisplayName);
88+
if (GitHubRepositoryNameContributor.parseAssociatedNames(job)
89+
.contains(changedRepository)) {
90+
LOGGER.info("Poked {}", fullDisplayName);
91+
trigger.onPost(pusherName);
92+
} else {
93+
LOGGER.debug("Skipped {} because it doesn't have a matching repository.",
94+
fullDisplayName);
95+
}
8896
}
8997
}
9098
}
99+
});
100+
101+
for (GitHubWebHook.Listener listener : Jenkins.getInstance()
102+
.getExtensionList(GitHubWebHook.Listener.class)) {
103+
listener.onPushRepositoryChanged(pusherName, changedRepository);
91104
}
92-
});
93105

94-
for (GitHubWebHook.Listener listener : Jenkins.getInstance()
95-
.getExtensionList(GitHubWebHook.Listener.class)) {
96-
listener.onPushRepositoryChanged(pusherName, changedRepository);
106+
} else {
107+
LOGGER.warn("Malformed repo url {}", repoUrl);
97108
}
98-
99-
} else {
100-
LOGGER.warn("Malformed repo url {}", repoUrl);
109+
} catch (IOException e) {
110+
LOGGER.warn("Received malformed PushEvent: " + payload, e);
101111
}
102112
}
103113
}

src/main/java/org/jenkinsci/plugins/github/webhook/subscriber/PingGHEventSubscriber.java

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,21 @@
33
import com.cloudbees.jenkins.GitHubRepositoryName;
44
import hudson.Extension;
55
import hudson.model.Item;
6-
import net.sf.json.JSONObject;
6+
import java.io.IOException;
7+
import java.io.StringReader;
8+
import java.util.Set;
9+
import javax.inject.Inject;
710
import org.jenkinsci.plugins.github.admin.GitHubHookRegisterProblemMonitor;
811
import org.jenkinsci.plugins.github.extension.GHEventsSubscriber;
912
import org.kohsuke.github.GHEvent;
13+
import org.kohsuke.github.GHEventPayload;
14+
import org.kohsuke.github.GHOrganization;
15+
import org.kohsuke.github.GHRepository;
16+
import org.kohsuke.github.GitHub;
1017
import org.slf4j.Logger;
1118
import org.slf4j.LoggerFactory;
1219

13-
import javax.inject.Inject;
14-
import java.util.Set;
15-
1620
import static com.google.common.collect.Sets.immutableEnumSet;
17-
import static net.sf.json.JSONObject.fromObject;
1821
import static org.kohsuke.github.GHEvent.PING;
1922

2023
/**
@@ -35,7 +38,6 @@ public class PingGHEventSubscriber extends GHEventsSubscriber {
3538
* This subscriber is not applicable to any item
3639
*
3740
* @param project ignored
38-
*
3941
* @return always false
4042
*/
4143
@Override
@@ -59,18 +61,23 @@ protected Set<GHEvent> events() {
5961
*/
6062
@Override
6163
protected void onEvent(GHEvent event, String payload) {
62-
JSONObject parsedPayload = fromObject(payload);
63-
JSONObject repository = parsedPayload.optJSONObject("repository");
64-
if (repository != null) {
65-
LOGGER.info("{} webhook received from repo <{}>!", event, repository.getString("html_url"));
66-
monitor.resolveProblem(GitHubRepositoryName.create(repository.getString("html_url")));
67-
} else {
68-
JSONObject organization = parsedPayload.optJSONObject("organization");
69-
if (organization != null) {
70-
LOGGER.info("{} webhook received from org <{}>!", event, organization.getString("url"));
64+
try {
65+
GHEventPayload.Ping ping =
66+
GitHub.offline().parseEventPayload(new StringReader(payload), GHEventPayload.Ping.class);
67+
GHRepository repository = ping.getRepository();
68+
if (repository != null) {
69+
LOGGER.info("{} webhook received from repo <{}>!", event, repository.getHtmlUrl());
70+
monitor.resolveProblem(GitHubRepositoryName.create(repository.getHtmlUrl().toExternalForm()));
7171
} else {
72-
LOGGER.warn("{} webhook received with unexpected payload", event);
72+
GHOrganization organization = ping.getOrganization();
73+
if (organization != null) {
74+
LOGGER.info("{} webhook received from org <{}>!", event, organization.getUrl());
75+
} else {
76+
LOGGER.warn("{} webhook received with unexpected payload", event);
77+
}
7378
}
79+
} catch (IOException e) {
80+
LOGGER.warn("Received malformed PingEvent: " + payload, e);
7481
}
7582
}
7683
}

0 commit comments

Comments
 (0)