Skip to content

Commit 150fc93

Browse files
stephenclanwen
stephenc
authored and
lanwen
committed
[JENKINS-39590] Narrow the exception handling to payload parsing
1 parent 555a752 commit 150fc93

File tree

2 files changed

+51
-49
lines changed

2 files changed

+51
-49
lines changed

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

Lines changed: 36 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -65,49 +65,50 @@ protected Set<GHEvent> events() {
6565
*/
6666
@Override
6767
protected void onEvent(GHEvent event, String payload) {
68+
GHEventPayload.Push push;
6869
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());
70+
push = GitHub.offline().parseEventPayload(new StringReader(payload), GHEventPayload.Push.class);
71+
} catch (IOException e) {
72+
LOGGER.warn("Received malformed PushEvent: " + payload, e);
73+
return;
74+
}
75+
URL repoUrl = push.getRepository().getUrl();
76+
final String pusherName = push.getPusher().getName();
77+
LOGGER.info("Received PushEvent for {}", repoUrl);
78+
final GitHubRepositoryName changedRepository = GitHubRepositoryName.create(repoUrl.toExternalForm());
7579

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-
}
80+
if (changedRepository != null) {
81+
// run in high privilege to see all the projects anonymous users don't see.
82+
// this is safe because when we actually schedule a build, it's a build that can
83+
// happen at some random time anyway.
84+
ACL.impersonate(ACL.SYSTEM, new Runnable() {
85+
@Override
86+
public void run() {
87+
for (Item job : Jenkins.getInstance().getAllItems(Item.class)) {
88+
GitHubTrigger trigger = triggerFrom(job, GitHubPushTrigger.class);
89+
if (trigger != null) {
90+
String fullDisplayName = job.getFullDisplayName();
91+
LOGGER.debug("Considering to poke {}", fullDisplayName);
92+
if (GitHubRepositoryNameContributor.parseAssociatedNames(job)
93+
.contains(changedRepository)) {
94+
LOGGER.info("Poked {}", fullDisplayName);
95+
trigger.onPost(pusherName);
96+
} else {
97+
LOGGER.debug("Skipped {} because it doesn't have a matching repository.",
98+
fullDisplayName);
9699
}
97100
}
98101
}
99-
});
100-
101-
for (GitHubWebHook.Listener listener : Jenkins.getInstance()
102-
.getExtensionList(GitHubWebHook.Listener.class)) {
103-
listener.onPushRepositoryChanged(pusherName, changedRepository);
104102
}
103+
});
105104

106-
} else {
107-
LOGGER.warn("Malformed repo url {}", repoUrl);
105+
for (GitHubWebHook.Listener listener : Jenkins.getInstance()
106+
.getExtensionList(GitHubWebHook.Listener.class)) {
107+
listener.onPushRepositoryChanged(pusherName, changedRepository);
108108
}
109-
} catch (IOException e) {
110-
LOGGER.warn("Received malformed PushEvent: " + payload, e);
109+
110+
} else {
111+
LOGGER.warn("Malformed repo url {}", repoUrl);
111112
}
112113
}
113114
}

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

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -61,23 +61,24 @@ protected Set<GHEvent> events() {
6161
*/
6262
@Override
6363
protected void onEvent(GHEvent event, String payload) {
64+
GHEventPayload.Ping ping;
6465
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()));
71-
} else {
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-
}
78-
}
66+
ping = GitHub.offline().parseEventPayload(new StringReader(payload), GHEventPayload.Ping.class);
7967
} catch (IOException e) {
8068
LOGGER.warn("Received malformed PingEvent: " + payload, e);
69+
return;
70+
}
71+
GHRepository repository = ping.getRepository();
72+
if (repository != null) {
73+
LOGGER.info("{} webhook received from repo <{}>!", event, repository.getHtmlUrl());
74+
monitor.resolveProblem(GitHubRepositoryName.create(repository.getHtmlUrl().toExternalForm()));
75+
} else {
76+
GHOrganization organization = ping.getOrganization();
77+
if (organization != null) {
78+
LOGGER.info("{} webhook received from org <{}>!", event, organization.getUrl());
79+
} else {
80+
LOGGER.warn("{} webhook received with unexpected payload", event);
81+
}
8182
}
8283
}
8384
}

0 commit comments

Comments
 (0)