Merge pull request #5 from jzoldak/zoldak/JENKINS-19337

Check that an SNS message is surrounded by double quotes [JENKINS-19337]
jenkinsci · Nov 19, 2015 · bcd61e8 · bcd61e8
2 parents 78cc303 + 6b38220
commit bcd61e8
Show file tree

Hide file tree

Showing 3 changed files with 97 additions and 2 deletions.
diff --git a/pom.xml b/pom.xml
@@ -6,7 +6,7 @@
     <parent>
         <groupId>org.jenkins-ci.plugins</groupId>
         <artifactId>plugin</artifactId>
-        <version>1.451</version>
+        <version>1.580</version>
     </parent>
 
     <groupId>com.base2services.jenkins</groupId>
@@ -64,6 +64,12 @@
             <artifactId>aws-java-sdk</artifactId>
             <version>1.3.26</version>
         </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-all</artifactId>
+            <version>1.9.5</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
 

diff --git a/src/main/java/com/base2services/jenkins/github/GitHubTriggerProcessor.java b/src/main/java/com/base2services/jenkins/github/GitHubTriggerProcessor.java
@@ -77,7 +77,10 @@ private JSONObject extractJsonFromPayload(String payload) {
         if(json.has("Type")) {
             String msg = json.getString("Message");
             if(msg != null) {
-                msg = msg.substring(1,msg.length()-1); //remove the leading and trailing double quotes
+                char ch[] = msg.toCharArray();
+                if((ch[0] == '"') && (ch[msg.length()-1]) == '"') {
+                   msg = msg.substring(1,msg.length()-1); //remove the leading and trailing double quotes
+                }
                 return JSONObject.fromObject(msg);
             }
         } else if (json.has("repository")){

diff --git a/src/test/java/com/base2services/jenkins/GitHubTriggerProcessorTest.java b/src/test/java/com/base2services/jenkins/GitHubTriggerProcessorTest.java
@@ -0,0 +1,86 @@
+package com.base2services.jenkins;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import com.base2services.jenkins.github.GitHubTriggerProcessor;
+import com.cloudbees.jenkins.GitHubRepositoryName;
+import hudson.model.FreeStyleProject;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.never;
+import org.mockito.runners.MockitoJUnitRunner;
+
+/**
+ * Created by benpatterson on 11/16/15.
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class GitHubTriggerProcessorTest {
+
+    private FreeStyleProject project;
+    private String payload;
+
+    @Rule
+    public JenkinsRule jenkinsRule = new JenkinsRule();
+
+    @Mock
+    private SqsBuildTrigger sbt;
+
+    @Before
+    public void setUp() throws Exception {
+        MockitoAnnotations.initMocks(this);
+        project = jenkinsRule.createFreeStyleProject("testProject");
+        project.addTrigger(sbt);
+        GitHubRepositoryName repoName = new GitHubRepositoryName("github.com", "foo", "bar");
+        Set<GitHubRepositoryName> repoSet = new HashSet<GitHubRepositoryName>();
+        repoSet.add(repoName);
+        doReturn(repoSet).when(sbt).getGitHubRepositories();
+    }
+
+    @Test
+    public void shouldTriggerWhenRepoInPayload() throws Exception {
+        payload = "{'repository': {'url': 'https://github.com/foo/bar', 'owner': {'name': 'foo'}, 'name': 'bar'}}".replace("'", "\"");
+        GitHubTriggerProcessor gtp = new GitHubTriggerProcessor();
+        gtp.processGitHubPayload(payload, sbt.getClass());
+        verify(sbt).onPost();
+    }
+
+    @Test
+    public void shouldNotTriggerWithWrongRepo() throws Exception {
+        payload = "{'repository': {'url': 'https://github.com/foo/baz', 'owner': {'name': 'foo'}, 'name': 'baz'}}".replace("'", "\"");
+        GitHubTriggerProcessor gtp = new GitHubTriggerProcessor();
+        gtp.processGitHubPayload(payload, sbt.getClass());
+        verify(sbt, never()).onPost();
+    }
+
+    @Test
+    public void shouldNotTriggerWithAFork() throws Exception {
+        payload = "{'repository': {'url': 'https://github.com/fob/bar', 'owner': {'name': 'fob'}, 'name': 'bar'}}".replace("'", "\"");
+        GitHubTriggerProcessor gtp = new GitHubTriggerProcessor();
+        gtp.processGitHubPayload(payload, sbt.getClass());
+        verify(sbt, never()).onPost();
+    }
+
+    @Test
+    public void shouldNotTriggerWithBadPayload() throws Exception {
+        payload = "{'noRepoInfo': {'empty': 'https://github.com/foo/bar'}}".replace("'", "\"");
+        GitHubTriggerProcessor gtp = new GitHubTriggerProcessor();
+        gtp.processGitHubPayload(payload, sbt.getClass());
+        verify(sbt, never()).onPost();
+    }
+
+    @Test
+    public void shouldTriggerUnquotedSnsMsg() throws Exception {
+        payload = "{\"Type\" : \"Notification\", \"Message\" : \"{'repository': {'owner': {'name': 'foo'}, 'url': 'https://github.com/foo/bar', 'name': 'bar'}}\", }";
+        GitHubTriggerProcessor gtp = new GitHubTriggerProcessor();
+        gtp.processGitHubPayload(payload, sbt.getClass());
+        verify(sbt).onPost();
+    }
+}