[FIXED JENKINS-10458] broken links to test results if test name conta…

…ins # or ?
jenkinsci · Feb 2, 2012 · b816be6 · b816be6
1 parent 8b8c62a
commit b816be6
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 2 deletions.
diff --git a/changelog.html b/changelog.html
@@ -58,6 +58,9 @@
   <li class=bug>
     Infinite loop or invalid next execution with crontab DoW=7
     (a href="https://issues.jenkins-ci.org/browse/JENKINS-12357">issue 12357</a>)
+  <li class=bug>
+    Broken links to test results with '#' or '?' in the name
+    (a href="https://issues.jenkins-ci.org/browse/JENKINS-10458">issue 10458</a>)
 </ul>
 </div><!--=TRUNK-END=-->
 

diff --git a/core/src/main/java/hudson/tasks/test/TestObject.java b/core/src/main/java/hudson/tasks/test/TestObject.java
@@ -31,13 +31,18 @@
 import hudson.tasks.junit.TestAction;
 import hudson.tasks.junit.TestResultAction;
 import jenkins.model.Jenkins;
+
+import org.apache.commons.lang.StringEscapeUtils;
+import org.apache.commons.lang.StringUtils;
 import org.kohsuke.stapler.*;
 import org.kohsuke.stapler.export.ExportedBean;
 
 import com.google.common.collect.MapMaker;
 
 import javax.servlet.ServletException;
 import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.*;
 import java.util.logging.Logger;
 
@@ -348,8 +353,11 @@ protected final synchronized String uniquifyName(
      * Replaces URL-unsafe characters.
      */
     public static String safe(String s) {
-        // 3 replace calls is still 2-3x faster than a regex replaceAll
-        return s.replace('/', '_').replace('\\', '_').replace(':', '_');
+        // this still seems to be a bit faster than a single replace with regexp
+        return s.replace('/', '_').replace('\\', '_').replace(':', '_').replace('?', '_').replace('#', '_');
+
+        // Note: we probably should some helpers like Commons URIEscapeUtils here to escape all invalid URL chars, but then we
+        // still would have to escape /, ? and so on
     }
 
     /**

diff --git a/core/src/test/java/hudson/tasks/test/TestObjectTest.java b/core/src/test/java/hudson/tasks/test/TestObjectTest.java
@@ -0,0 +1,20 @@
+package hudson.tasks.test;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestObjectTest {
+
+    @Test
+    public void testSafe() {
+        String name = "Foo#approve! is called by approve_on_foo?xyz/\\:";
+        String encoded = TestObject.safe(name);
+
+        Assert.assertFalse(encoded.contains("#"));
+        Assert.assertFalse(encoded.contains("?"));
+        Assert.assertFalse(encoded.contains("\\"));
+        Assert.assertFalse(encoded.contains("/"));
+        Assert.assertFalse(encoded.contains(":"));
+    }
+
+}