Merge pull request #1752 from vladak/diff_link_encoding

add HTML encoding of links generated by history search

Author: tarzanek

src/org/opensolaris/opengrok/search/context/HistoryContext.java

@@ -18,7 +18,7 @@
  */
 
 /*
- * Copyright (c) 2005, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2017, Oracle and/or its affiliates. All rights reserved.
  */
 
 package org.opensolaris.opengrok.search.context;
@@ -43,10 +43,11 @@
 import org.opensolaris.opengrok.search.Hit;
 import org.opensolaris.opengrok.search.QueryBuilder;
 import org.opensolaris.opengrok.web.Prefix;
+import org.opensolaris.opengrok.web.Util;
 
 /**
  * it is supposed to get the matching lines from history log files.
- * since lucene does not easily give the match context.
+ * since Lucene does not easily give the match context.
  */
 public class HistoryContext {
 
@@ -199,21 +200,28 @@ private boolean getHistoryContext(
      * @param end position of the first char after the match
      * @param flatten should multi-line log entries be flattened to a single
      * @param path path to the file
-     * @param wcontext web context (begin of url)
+     * @param wcontext web context (begin of URL)
      * @param nrev old revision
      * @param rev current revision
      * line? If {@code true}, replace newline with space.
      */
-    private void writeMatch(Appendable out, String line,
-                            int start, int end, boolean flatten, String path, String wcontext, String nrev, String rev)
+    protected static void writeMatch(Appendable out, String line,
+                            int start, int end, boolean flatten, String path,
+                            String wcontext, String nrev, String rev)
             throws IOException {
+
         String prefix = line.substring(0, start);
         String match = line.substring(start, end);
         String suffix = line.substring(end);
 
-        if (wcontext!=null && nrev!=null && !wcontext.isEmpty() ) {
-            //does below need to be encoded? see bug 16985
-            out.append("<a href="+wcontext+Prefix.DIFF_P+path+"?r2="+path+"@"+rev+"&r1="+path+"@"+nrev+" title=\"diff to previous version\">diff</a> ");
+        if (wcontext != null && nrev != null && !wcontext.isEmpty()) {
+            out.append("<a href=\"");
+            printHTML(out, wcontext + Prefix.DIFF_P +
+                    Util.URIEncodePath(path) +
+                    "?r2=" + Util.URIEncodePath(path) + "@" + rev +
+                    "&r1=" + Util.URIEncodePath(path) + "@" + nrev +
+                    "\" title=\"diff to previous version\"", flatten);
+            out.append(">diff</a> ");
         }
 
         printHTML(out, prefix, flatten);
@@ -231,7 +239,7 @@ private void writeMatch(Appendable out, String line,
      * @param flatten should multi-line strings be flattened to a single
      * line? If {@code true}, replace newline with space.
      */
-    private void printHTML(Appendable out, String str, boolean flatten)
+    private static void printHTML(Appendable out, String str, boolean flatten)
             throws IOException {
         for (int i = 0; i < str.length(); i++) {
             char ch = str.charAt(i);

src/org/opensolaris/opengrok/web/PageConfig.java

@@ -185,7 +185,7 @@ public String getHeaderData() {
     }
 
     /**
-     * Get all data required to create a diff view wrt. to this request in one
+     * Get all data required to create a diff view w.r.t. to this request in one
      * go.
      *
      * @return an instance with just enough information to render a sufficient

test/org/opensolaris/opengrok/search/context/HistoryContextTest.java

@@ -24,6 +24,7 @@
 package org.opensolaris.opengrok.search.context;
 
 import java.io.File;
+import java.io.IOException;
 import java.io.StringWriter;
 import java.util.ArrayList;
 import org.apache.lucene.index.Term;
@@ -32,6 +33,7 @@
 import org.apache.lucene.search.PhraseQuery;
 import org.apache.lucene.search.TermQuery;
 import org.junit.AfterClass;
+import org.junit.Assert;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.opensolaris.opengrok.history.HistoryGuru;
@@ -167,4 +169,17 @@ public void testGetContext_4args() throws Exception {
                 "Created a <b>small</b> dummy program"));
     }
 
+    /**
+     * Test URI and HTML encoding of {@code writeMatch()}.
+     * @throws IOException
+     */
+    @Test
+    public void testWriteMatch() throws IOException {
+        StringBuilder sb = new StringBuilder();
+        HistoryContext.writeMatch(sb, "foo", 0, 3, true, "/foo bar/haf+haf",
+                "ctx", "1", "2");
+        Assert.assertEquals("<a href=\"ctx/diff/foo%20bar/haf%2Bhaf?r2=/foo%20bar/haf%2Bhaf@2&amp;" +
+                "r1=/foo%20bar/haf%2Bhaf@1\" title=\"diff to previous version\">diff</a> <b>foo</b>",
+                sb.toString());
+    }
 }