Url encoding evaluated values when they are used in URL invocations #76

Author: tomasbjerre

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 Changelog of Pull Request Notifier for Bitbucket.
 
+## 2.9
+* Url encoding evaluated values when they are used in URL invocations.
+
 ## 2.8
 * New variables
  * ${PULL_REQUEST_MERGE_COMMIT} Hash of merged commit (only available for merged-event).

src/main/java/se/bjurr/prnfb/listener/PrnfbPullRequestEventListener.java

@@ -159,9 +159,9 @@ public void notify(final PrnfbNotification notification, PrnfbPullRequestAction
 
   Optional<String> postContent = absent();
   if (notification.getPostContent().isPresent()) {
-   postContent = of(renderer.render(notification.getPostContent().get()));
+   postContent = of(renderer.render(notification.getPostContent().get(), FALSE));
   }
-  String renderedUrl = renderer.render(notification.getUrl());
+  String renderedUrl = renderer.render(notification.getUrl(), TRUE);
   logger.info(notification.getName() + " > " //
     + pullRequest.getFromRef().getId() + "(" + pullRequest.getFromRef().getLatestCommit() + ") -> " //
     + pullRequest.getToRef().getId() + "(" + pullRequest.getToRef().getLatestCommit() + ")" + " " //
@@ -173,7 +173,7 @@ public void notify(final PrnfbNotification notification, PrnfbPullRequestAction
     .appendBasicAuth(notification);
   for (Header header : notification.getHeaders()) {
    urlInvoker//
-     .withHeader(header.getName(), renderer.render(header.getValue()));
+     .withHeader(header.getName(), renderer.render(header.getValue(), FALSE));
   }
   invoker.invoke(urlInvoker//
     .withProxyServer(notification.getProxyServer()) //
@@ -189,8 +189,8 @@ public boolean notificationTriggeredByAction(PrnfbNotification notification, Prn
   }
   if (notification.getFilterRegexp().isPresent()
     && notification.getFilterString().isPresent()
-    && !compile(notification.getFilterRegexp().get()).matcher(renderer.render(notification.getFilterString().get()))
-      .find()) {
+    && !compile(notification.getFilterRegexp().get()).matcher(
+      renderer.render(notification.getFilterString().get(), FALSE)).find()) {
    return FALSE;
   }
 

src/main/java/se/bjurr/prnfb/listener/PrnfbRenderer.java

@@ -1,14 +1,18 @@
 package se.bjurr.prnfb.listener;
 
+import static com.google.common.base.Charsets.UTF_8;
+import static com.google.common.base.Throwables.propagate;
 import static com.google.common.collect.Iterables.filter;
 import static com.google.common.collect.Lists.newArrayList;
+import static java.net.URLEncoder.encode;
 import static java.util.logging.Logger.getLogger;
 import static java.util.regex.Pattern.compile;
 import static se.bjurr.prnfb.listener.PrnfbRenderer.REPO_PROTOCOL.http;
 import static se.bjurr.prnfb.listener.PrnfbRenderer.REPO_PROTOCOL.ssh;
 import static se.bjurr.prnfb.listener.UrlInvoker.urlInvoker;
 import static se.bjurr.prnfb.listener.UrlInvoker.HTTP_METHOD.GET;
 
+import java.io.UnsupportedEncodingException;
 import java.util.Map;
 import java.util.Set;
 import java.util.logging.Logger;
@@ -476,12 +480,17 @@ public PrnfbRenderer(PullRequest pullRequest, PrnfbPullRequestAction pullRequest
   this.variables = variables;
  }
 
- public String render(String string) {
+ public String render(String string, Boolean forUrl) {
   for (final PrnfbVariable variable : PrnfbVariable.values()) {
    final String regExpStr = "\\$\\{" + variable.name() + "\\}";
    if (string.contains(regExpStr.replaceAll("\\\\", ""))) {
-    string = string.replaceAll(regExpStr, variable.resolve(pullRequest, pullRequestAction, ApplicationUser,
-      repositoryService, propertiesService, prnfbNotification, variables));
+    try {
+     String resolved = variable.resolve(pullRequest, pullRequestAction, ApplicationUser, repositoryService,
+       propertiesService, prnfbNotification, variables);
+     string = string.replaceAll(regExpStr, forUrl ? encode(resolved, UTF_8.name()) : resolved);
+    } catch (UnsupportedEncodingException e) {
+     propagate(e);
+    }
    }
   }
   return string;

src/test/java/se/bjurr/prnfb/admin/PrnfbPullRequestEventListenerTest.java

@@ -289,7 +289,7 @@ public void testThatAUrlWithVariablesFromAndToCanBeInvoked() throws Exception {
          .getPullRequest() //
      ) //
      .triggerButton("The Button") //
-     .invokedUrl(1, "http://bjurr.se/10The%20Button%20Text");
+     .invokedUrl(1, "http://bjurr.se/10The+Button+Text");
   }
  }
 
@@ -333,7 +333,7 @@ public void testThatAUrlWithCommentVariableHasSpacesReplaced() throws Exception
         .withCommentText("a text with\nnewline") //
         .build() //
     ) //
-    .invokedUrl(0, "http://bjurr.se/a%20text%20with%20newline");
+    .invokedUrl(0, "http://bjurr.se/a+text+with%0Anewline");
  }
 
  @Test
@@ -358,7 +358,7 @@ public void testThatAUrlWithVariableFromBranchCanBeInvokedWhenBranchIdContainsSl
         .withPullRequestAction(OPENED) //
         .build() //
     ) //
-    .invokedUrl(0, "http://bjurr.se/feature/branchmodmerge");
+    .invokedUrl(0, "http://bjurr.se/feature%2Fbranchmodmerge");
  }
 
  @Test
@@ -426,7 +426,7 @@ public void testThatAUrlWithVariablesExceptFromAndToCanBeInvoked() throws Except
     ) //
     .invokedUrl(
       0,
-      "http://bjurr.se/id=10&action=OPENED&displayName=authorDisplayName&authorEmail=authorEmail&authorId=100&authorName=authorName&authorSlug=authorSlug&pullRequestUrl=http://bitbucket.server/projects/theProject/repos/theRepoName/pull-requests/10" //
+      "http://bjurr.se/id=10&action=OPENED&displayName=authorDisplayName&authorEmail=authorEmail&authorId=100&authorName=authorName&authorSlug=authorSlug&pullRequestUrl=http%3A%2F%2Fbitbucket.server%2Fprojects%2FtheProject%2Frepos%2FtheRepoName%2Fpull-requests%2F10" //
     );
  }
 
@@ -889,7 +889,7 @@ public void testThatFilterCanBeUsedWithComments() throws Exception {
         .withPullRequestAction(COMMENTED) //
         .build() //
     ) //
-    .invokedUrl(0, "http://bjurr.se/?comment=keyword%20A%20nice%20comment&version=0"); //
+    .invokedUrl(0, "http://bjurr.se/?comment=keyword+A+nice+comment&version=0"); //
  }
 
  @Test
@@ -922,7 +922,7 @@ public void testThatFilterCanBeUsedWithCommentsAndSpecialEscapedChars() throws E
         .withPullRequestAction(COMMENTED) //
         .build() //
     ) //
-    .invokedUrl(0, "http://bjurr.se/?comment=%20keyword%20&version=0");
+    .invokedOnlyUrl("http://bjurr.se/?comment=+keyword+&version=0");
  }
 
  @Test
@@ -1430,7 +1430,7 @@ public void testThatButtonCanBeUsedForTriggeringEvent() throws Exception {
     ) //
     .store() //
     .triggerButton("Button Form") //
-    .invokedOnlyUrl("http://bjurr.se/Trigger%20notification") //
+    .invokedOnlyUrl("http://bjurr.se/Trigger+notification") //
     .hasButtonsEnabled("Button Form");
  }
 
@@ -1512,7 +1512,7 @@ public void testThatEventTriggeredByButtonCanBeFiltered() throws Exception {
     ) //
     .store() //
     .triggerButton("Button Form") //
-    .invokedOnlyUrl("http://bjurr.se/?123=button%20text%20123");
+    .invokedOnlyUrl("http://bjurr.se/?123=button+text+123");
  }
 
  @Test
@@ -1547,7 +1547,7 @@ public void testThatEventTriggeredByButtonCanBeIgnoredByFilterWhileAnotherIsNotI
     ) //
     .store() //
     .triggerButton("Button Form") //
-    .invokedOnlyUrl("http://bjurr.se/?123=button%20text%20123");
+    .invokedOnlyUrl("http://bjurr.se/?123=button+text+123");
  }
 
  @Test
@@ -1578,8 +1578,8 @@ public void testThatEventTriggeredByButtonCanResultInSeveralNotifications() thro
         .withFieldValue(button_visibility, EVERYONE.name()).build()) //
     .store() //
     .triggerButton("Button Form") //
-    .invokedUrl(0, "http://bjurr.se/?123=button%20text%20123") //
-    .invokedUrl(1, "http://bjurr.se/?456=button%20text%20123");
+    .invokedUrl(0, "http://bjurr.se/?123=button+text+123") //
+    .invokedUrl(1, "http://bjurr.se/?456=button+text+123");
  }
 
  @Test
@@ -1613,9 +1613,9 @@ public void testThatThereCanBeSeveralButtons() throws Exception {
     ) //
     .store() //
     .triggerButton("Button Form 1") //
-    .invokedUrl(0, "http://bjurr.se/?button%20text%201") //
+    .invokedUrl(0, "http://bjurr.se/?button+text+1") //
     .triggerButton("Button Form 2") //
-    .invokedUrl(1, "http://bjurr.se/?button%20text%202");
+    .invokedUrl(1, "http://bjurr.se/?button+text+2");
  }
 
  @Test
@@ -1637,7 +1637,7 @@ public void testThatValueFromUrlCanBeUsedInInvocation() throws Exception {
         .withPullRequestAction(OPENED) //
         .build() //
     ) //
-    .invokedOnlyUrl("http://bjurr.se/?some%20content");
+    .invokedOnlyUrl("http://bjurr.se/?some+content");
  }
 
  @Test