[FIXED JENKINS-46449] Better null safety

jenkinsci · Aug 29, 2017 · d3391ba · d3391ba
1 parent d147c66
commit d3391ba
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 3 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMFile.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMFile.java
@@ -78,7 +78,7 @@ private GitHubSCMFile(@NonNull GitHubSCMFile parent, String name, GHContent meta
     }
 
     private void checkOpen() throws IOException {
-        if (!closable.isOpen()) {
+        if (!closable.isOpen() || (!resolved && repo == null)) {
             throw new IOException("Closed");
         }
     }

diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMFileSystem.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMFileSystem.java
@@ -39,6 +39,7 @@
 import jenkins.scm.api.SCMRevision;
 import jenkins.scm.api.SCMSource;
 import org.kohsuke.github.GHRepository;
+import org.kohsuke.github.GHUser;
 import org.kohsuke.github.GitHub;
 import org.kohsuke.github.HttpException;
 
@@ -139,8 +140,22 @@ public SCMFileSystem build(@NonNull SCMSource source, @NonNull SCMHead head, @Ch
                 } else if (head instanceof PullRequestSCMHead) {
                     PullRequestSCMHead pr = (PullRequestSCMHead) head;
                     if (!pr.isMerge() && pr.getSourceRepo() != null) {
+                        GHUser user = github.getUser(pr.getSourceOwner());
+                        if (user == null) {
+                            // we need to release here as we are not throwing an exception or transferring
+                            // responsibility to FS
+                            Connector.release(github);
+                            return null;
+                        }
+                        GHRepository repo = user.getRepository(pr.getSourceRepo());
+                        if (repo == null) {
+                            // we need to release here as we are not throwing an exception or transferring
+                            // responsibility to FS
+                            Connector.release(github);
+                            return null;
+                        }
                         return new GitHubSCMFileSystem(
-                                github, github.getUser(pr.getSourceOwner()).getRepository(pr.getSourceRepo()),
+                                github, repo,
                                 pr.getSourceBranch(),
                                 rev);
                     }
@@ -153,7 +168,18 @@ public SCMFileSystem build(@NonNull SCMSource source, @NonNull SCMHead head, @Ch
                     return null;
                 }
 
-                GHRepository repo = github.getUser(src.getRepoOwner()).getRepository(src.getRepository());
+                GHUser user = github.getUser(src.getRepoOwner());
+                if (user == null) {
+                    // we need to release here as we are not throwing an exception or transferring responsibility to FS
+                    Connector.release(github);
+                    return null;
+                }
+                GHRepository repo = user.getRepository(src.getRepository());
+                if (repo == null) {
+                    // we need to release here as we are not throwing an exception or transferring responsibility to FS
+                    Connector.release(github);
+                    return null;
+                }
                 if (rev == null) {
                     rev = new AbstractGitSCMSource.SCMRevisionImpl((BranchSCMHead) head, repo.getBranch(ref).getSHA1());
                 }