HTML/URI encode path/content where possible

vladak · vladak · commit b86d481b0089 · 2025-02-21T10:17:08.000+01:00
diff --git a/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java b/opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java
@@ -350,7 +350,7 @@ private static boolean needsHtmlize(CharSequence q, boolean pre) {
      * Convenience method for {@code breadcrumbPath(urlPrefix, path, PATH_SEPARATOR)}.
      *
      * @param urlPrefix prefix to add to each url
-     * @param path path to crack
+     * @param path the full path from which the breadcrumb path is built
      * @return HTML markup for the breadcrumb or the path itself.
      *
      * @see #breadcrumbPath(String, String, char)
@@ -364,7 +364,7 @@ public static String breadcrumbPath(String urlPrefix, String path) {
      * {@code breadcrumbPath(urlPrefix, path, sep, "", false)}.
      *
      * @param urlPrefix prefix to add to each url
-     * @param path path to crack
+     * @param path the full path from which the breadcrumb path is built
      * @param sep separator to use to crack the given path
      *
      * @return HTML markup fro the breadcrumb or the path itself.
@@ -379,13 +379,13 @@ public static String breadcrumbPath(String urlPrefix, String path, char sep) {
      * {@code breadcrumbPath(urlPrefix, path, sep, "", false, path.endsWith(sep)}.
      *
      * @param urlPrefix prefix to add to each url
-     * @param path path to crack
+     * @param path the full path from which the breadcrumb path is built
      * @param sep separator to use to crack the given path
      * @param urlPostfix suffix to add to each url
      * @param compact if {@code true} the given path gets transformed into its
-     * canonical form (.i.e. all '.' and '..' and double separators removed, but
+     * canonical form (.i.e. all <code>'.'</code> and <code>'..'</code> and double separators removed, but
      * not always resolves to an absolute path) before processing starts.
-     * @return HTML markup fro the breadcrumb or the path itself.
+     * @return HTML markup for the breadcrumb or the path itself
      * @see #breadcrumbPath(String, String, char, String, boolean, boolean)
      * @see #getCanonicalPath(String, char)
      */
@@ -408,12 +408,10 @@ public static String breadcrumbPath(String urlPrefix, String path,
      * neither whether the path [component] exists nor which type it is).
      *
      * @param urlPrefix what should be prepended to the constructed URL
-     * @param path the full path from which the breadcrumb path is built.
-     * @param sep the character that separates the path components in
-     * <var>path</var>
+     * @param path the full path from which the breadcrumb path is built
+     * @param sep the character that separates the path components in <var>path</var>
      * @param urlPostfix what should be appended to the constructed URL
-     * @param compact if {@code true}, a canonical path gets constructed before
-     * processing.
+     * @param compact if {@code true}, a canonical path gets constructed before processing.
      * @param isDir if {@code true} a "/" gets append to the last path
      * component's link and <var>sep</var> to its name
      * @return <var>path</var> if it resolves to an empty or "/" or {@code null}
@@ -431,8 +429,7 @@ public static String breadcrumbPath(String urlPrefix, String path,
         String prefix = urlPrefix == null ? "" : urlPrefix;
         String postfix = urlPostfix == null ? "" : urlPostfix;
         StringBuilder pwd = new StringBuilder(path.length() + pnames.length);
-        StringBuilder markup
-                = new StringBuilder((pnames.length + 3 >> 1) * path.length()
+        StringBuilder markup = new StringBuilder((pnames.length + 3 >> 1) * path.length()
                         + pnames.length
                         * (17 + prefix.length() + postfix.length()));
         int k = path.indexOf(pnames[0]);
@@ -445,9 +442,13 @@ public static String breadcrumbPath(String urlPrefix, String path,
             if (isDir || i < pnames.length - 1) {
                 pwd.append(PATH_SEPARATOR);
             }
-            markup.append(ANCHOR_LINK_START).append(prefix).append(pwd)
-                    .append(postfix).append(CLOSE_QUOTED_TAG).append(pnames[i])
-                    .append(ANCHOR_END);
+            markup.append(ANCHOR_LINK_START).
+                    append(prefix).
+                    append(pwd).
+                    append(postfix).
+                    append(CLOSE_QUOTED_TAG).
+                    append(Util.htmlize(pnames[i])).
+                    append(ANCHOR_END);
             if (isDir || i < pnames.length - 1) {
                 markup.append(sep);
             }
@@ -520,8 +521,7 @@ public static String getEmail(String author) {
 
     /**
      * Remove all empty and {@code null} string elements from the given
-     * <var>names</var> and optionally all redundant information like "." and
-     * "..".
+     * <var>names</var> and optionally all redundant information like <code>"."</code> and <code>".."</code>.
      *
      * @param names names to check
      * @param canonical if {@code true}, remove redundant elements as well.
diff --git a/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java b/opengrok-indexer/src/test/java/org/opengrok/indexer/web/UtilTest.java
@@ -18,7 +18,7 @@
  */
 
 /*
- * Copyright (c) 2007, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2007, 2025, Oracle and/or its affiliates. All rights reserved.
  * Portions Copyright (c) 2017, 2019, Chris Fraire <cfraire@me.com>.
  */
 package org.opengrok.indexer.web;
@@ -112,8 +112,7 @@ void breadcrumbPath() {
         // parent directories have a trailing slash in href
         assertEquals("<a href=\"/r/a/\">a</a>/<a href=\"/r/a/b\">b</a>",
                 Util.breadcrumbPath("/r/", "a/b"));
-        // if basename is a dir (ends with file seperator), href link also
-        // ends with a '/'
+        // if basename is a dir (ends with file separator), href link also ends with a '/'
         assertEquals("<a href=\"/r/a/\">a</a>/<a href=\"/r/a/b/\">b</a>/",
                 Util.breadcrumbPath("/r/", "a/b/"));
         // should work the same way with a '.' as file separator
@@ -129,11 +128,15 @@ void breadcrumbPath() {
         // Prefix gets just prefixed as is and not mangled wrt. path -> "//"
         assertEquals("/<a href=\"/root//xx&project=y\">xx</a>",
                 Util.breadcrumbPath("/root/", "../xx", '/', "&project=y", true));
-        // relative pathes are resolved wrt. / , so path resolves to /a/c/d
+        // relative paths are resolved wrt. / , so path resolves to /a/c/d
         assertEquals("/<a href=\"/r//a/\">a</a>/"
                         + "<a href=\"/r//a/c/\">c</a>/"
                         + "<a href=\"/r//a/c/d\">d</a>",
                 Util.breadcrumbPath("/r/", "../a/b/../c//d", '/', "", true));
+        // path components should be URI encoded and htmlized
+        assertEquals("<a href=\"/root/foo/&project=y\">foo</a>/"
+                + "<a href=\"/root/foo/bar%3E&project=y\">bar&gt;</a>",
+                Util.breadcrumbPath("/root/", "foo/bar>", '/', "&project=y", true));
     }
 
     @Test
diff --git a/opengrok-web/src/main/webapp/history.jsp b/opengrok-web/src/main/webapp/history.jsp
@@ -18,7 +18,7 @@ information: Portions Copyright [yyyy] [name of copyright owner]
 
 CDDL HEADER END
 
-Copyright (c) 2005, 2022, Oracle and/or its affiliates. All rights reserved.
+Copyright (c) 2005, 2025, Oracle and/or its affiliates. All rights reserved.
 Portions Copyright 2011 Jens Elkner.
 Portions Copyright (c) 2018-2020, Chris Fraire <cfraire@me.com>.
 --%>
@@ -47,6 +47,7 @@ org.opengrok.indexer.web.Util"
 %>
 <%@ page import="jakarta.servlet.http.HttpServletResponse" %>
 <%@ page import="org.opengrok.indexer.web.SortOrder" %>
+<%@ page import="java.util.Optional" %>
 <%/* ---------------------- history.jsp start --------------------- */
 {
     final Logger LOGGER = LoggerFactory.getLogger(getClass());
@@ -59,7 +60,7 @@ org.opengrok.indexer.web.Util"
 
     String path = cfg.getPath();
 
-    if (path.length() > 0) {
+    if (!path.isEmpty()) {
         String primePath = path;
         Project project = cfg.getProject();
         if (project != null) {
@@ -75,8 +76,7 @@ org.opengrok.indexer.web.Util"
             try {
                 primePath = searchHelper.getPrimeRelativePath(project.getName(), path);
             } catch (IOException | ForbiddenSymlinkException ex) {
-                LOGGER.log(Level.WARNING, String.format(
-                        "Error getting prime relative for %s", path), ex);
+                LOGGER.log(Level.WARNING, String.format("Error getting prime relative for '%s'", path), ex);
             }
         }
 
@@ -148,7 +148,7 @@ include file="/httpheader.jspf"
         request.setAttribute("history.jsp-slider", Util.createSlider(startIndex, max, totalHits, request));
 %>
         <div id="Masthead">History log of 
-        <%= Util.breadcrumbPath(context + Prefix.XREF_P, path,'/',"",true,cfg.isDir()) %>
+        <%= Util.breadcrumbPath(context + Prefix.XREF_P, path, '/', "", true, cfg.isDir()) %>
         (Results <span class="bold"> <%= totalHits != 0 ? startIndex + 1 : 0 %> – <%= startIndex + thisPageIndex
             %></span> of <span class="bold"><%= totalHits %></span>)
         </div>
@@ -258,16 +258,17 @@ document.domReady.push(function() {domReadyHistory();});
             <%
             int count=0;
             for (HistoryEntry entry : hist.getHistoryEntries(maxItems, startIndex)) {
-                String dispRev = entry.getDisplayRevision();
-                if (dispRev == null || dispRev.length() == 0) {
-                    dispRev = "";
+                if (Objects.isNull(entry)) {
+                    continue;
                 }
-                String rev = entry.getRevision();
-                if (rev == null || rev.length() == 0) {
-                    rev = "";
-                }
-                String tags = hist.getTags().get(rev);
 
+                final String htmlEncodedDisplayRevision = Optional.ofNullable(entry.getDisplayRevision()).
+                        map(Util::htmlize).
+                        orElse("");
+                final String rev = Optional.ofNullable(entry.getRevision()).
+                        orElse("");
+
+                String tags = hist.getTags().get(rev);
                 if (tags != null) {
                     int colspan;
                     if (cfg.isDir())
@@ -285,7 +286,7 @@ document.domReady.push(function() {domReadyHistory();});
         <tr><%
                 if (cfg.isDir()) {
             %>
-            <td><%= dispRev %></td><%
+            <td><%= htmlEncodedDisplayRevision %></td><%
                 } else {
                     if (entry.isActive()) {
                         StringBuffer urlBuffer = request.getRequestURL();
@@ -297,7 +298,7 @@ document.domReady.push(function() {domReadyHistory();});
             <td><a href="<%= urlBuffer %>"
                 title="link to revision line">#</a>
                 <a href="<%= context + Prefix.XREF_P + uriEncodedName + "?" +
-                        QueryParameters.REVISION_PARAM_EQ + Util.uriEncode(rev) %>"><%= dispRev %>
+                        QueryParameters.REVISION_PARAM_EQ + Util.uriEncode(rev) %>"><%= htmlEncodedDisplayRevision %>
                 </a></td>
             <td><%
                 %><input type="radio"
@@ -339,7 +340,7 @@ document.domReady.push(function() {domReadyHistory();});
                     } else {
                         striked = true;
                 %>
-            <td><del><%= dispRev %></del></td>
+            <td><del><%= htmlEncodedDisplayRevision %></del></td>
             <td></td><%
                     }
                 }
@@ -354,23 +355,23 @@ document.domReady.push(function() {domReadyHistory();});
                 String author = entry.getAuthor();
                 if (author == null) {
                 %>(no author)<%
-                } else if (userPage != null && userPage.length() > 0) {
+                } else if (userPage != null && !userPage.isEmpty()) {
                     String alink = Util.getEmail(author);
                 %><a href="<%= userPage + Util.htmlize(alink) + userPageSuffix
                 %>"><%= Util.htmlize(author)%></a><%
                 } else {
                 %><%= Util.htmlize(author) %><%
                 }
                 %></td>
-            <td><a id="<%= dispRev %>"></a><%
+            <td><a id="<%= htmlEncodedDisplayRevision %>"></a><%
                 // revision message collapse threshold minimum of 10
                 int summaryLength = Math.max(10, cfg.getRevisionMessageCollapseThreshold());
                 String cout = Util.htmlize(entry.getMessage());
 
-                if (bugPage != null && bugPage.length() > 0 && bugPattern != null) {
+                if (bugPage != null && !bugPage.isEmpty() && bugPattern != null) {
                     cout = Util.linkifyPattern(cout, bugPattern, "$1", Util.completeUrl(bugPage + "$1", request));
                 }
-                if (reviewPage != null && reviewPage.length() > 0 && reviewPattern != null) {
+                if (reviewPage != null && !reviewPage.isEmpty() && reviewPattern != null) {
                     cout = Util.linkifyPattern(cout, reviewPattern, "$1", Util.completeUrl(reviewPage + "$1", request));
                 }
                 
@@ -380,10 +381,10 @@ document.domReady.push(function() {domReadyHistory();});
                     showSummary = true;
                     coutSummary = coutSummary.substring(0, summaryLength - 1);
                     coutSummary = Util.htmlize(coutSummary);
-                    if (bugPage != null && bugPage.length() > 0 && bugPattern != null) {
+                    if (bugPage != null && !bugPage.isEmpty() && bugPattern != null) {
                         coutSummary = Util.linkifyPattern(coutSummary, bugPattern, "$1", Util.completeUrl(bugPage + "$1", request));
                     }
-                    if (reviewPage != null && reviewPage.length() > 0 && reviewPattern != null) {
+                    if (reviewPage != null && !reviewPage.isEmpty() && reviewPattern != null) {
                         coutSummary = Util.linkifyPattern(coutSummary, reviewPattern, "$1", Util.completeUrl(reviewPage + "$1", request));
                     }
                 }
@@ -406,11 +407,11 @@ document.domReady.push(function() {domReadyHistory();});
                         String jfile = Util.stripPathPrefix(path, ifile);
                         if (Objects.equals(rev, "")) {
                 %>
-<a class="h" href="<%= context + Prefix.XREF_P + ifile %>"><%= jfile %></a><br/><%
+<a class="h" href="<%= context + Prefix.XREF_P + Util.uriEncodePath(ifile) %>"><%= Util.htmlize(jfile) %></a><br/><%
                         } else {
                 %>
-<a class="h" href="<%= context + Prefix.XREF_P + ifile %>?<%= QueryParameters.REVISION_PARAM_EQ %>
-<%= rev %>"><%= jfile %></a><br/><%
+<a class="h" href="<%= context + Prefix.XREF_P + Util.uriEncodePath(ifile) %>?<%= QueryParameters.REVISION_PARAM_EQ %>
+<%= Util.uriEncode(rev) %>"><%= Util.htmlize(jfile) %></a><br/><%
                         }
                     }
                 %></div><%
diff --git a/opengrok-web/src/main/webapp/httpheader.jspf b/opengrok-web/src/main/webapp/httpheader.jspf
@@ -18,7 +18,7 @@ information: Portions Copyright [yyyy] [name of copyright owner]
 
 CDDL HEADER END
 
-Copyright (c) 2007, 2021, Oracle and/or its affiliates. All rights reserved.
+Copyright (c) 2007, 2025, Oracle and/or its affiliates. All rights reserved.
 Portions Copyright 2011 Jens Elkner.
 Portions Copyright (c) 2017-2018, 2020, Chris Fraire <cfraire@me.com>.
 Portions Copyright (c) 2020, Aleksandr Kirillov <alexkirillovsamara@gmail.com>.
@@ -38,6 +38,7 @@ to set the title of the document before the include directive for this fragment:
 org.opengrok.indexer.Info,
 org.opengrok.web.PageConfig,
 org.opengrok.indexer.web.Prefix,
+org.opengrok.indexer.web.Util,
 org.opengrok.web.Scripts"
 %><%
 /* ---------------------- httpheader.jsp start --------------------- */
@@ -92,8 +93,8 @@ org.opengrok.web.Scripts"
 
 if (cfg.getPrefix().equals(Prefix.HIST_L)) {
     out.write("<link rel=\"alternate\" type=\"application/rss+xml\" " +
-        "title=\"RSS feed for " + cfg.getPath() + "\" " +
-        "href=\"" + ctxPath + Prefix.RSS_P + cfg.getPath() + "\" />");
+        "title=\"RSS feed for " + Util.htmlize(cfg.getPath()) + "\" " +
+        "href=\"" + ctxPath + Prefix.RSS_P + Util.uriEncodePath(cfg.getPath()) + "\" />");
 }
 %>
 <link rel="search" href="<%=ctxPath%>/opensearch"
diff --git a/opengrok-web/src/main/webapp/mast.jsp b/opengrok-web/src/main/webapp/mast.jsp
@@ -66,7 +66,6 @@ org.opengrok.indexer.web.Util"%>
     }
 
     // set the default page title
-    String path = cfg.getPath();
     cfg.setTitle(cfg.getPathTitle());
 }
 %>
@@ -94,19 +93,18 @@ include file="/httpheader.jspf"
 
     String messages = "";
     if (cfg.getProject() != null) {
-        messages = MessagesUtils.messagesToJson(cfg.getProject(),
-                    MessagesContainer.MESSAGES_MAIN_PAGE_TAG);
+        messages = MessagesUtils.messagesToJson(cfg.getProject(), MessagesContainer.MESSAGES_MAIN_PAGE_TAG);
     }
     %>
     <a href="<%= context + Prefix.XREF_P %>/">xref</a>:
-    <%= Util.breadcrumbPath(context + Prefix.XREF_P, path,'/',"",true,cfg.isDir()) %>
-    <% if (rev.length() != 0) { %>
+    <%= Util.breadcrumbPath(context + Prefix.XREF_P, path, '/', "", true, cfg.isDir()) %>
+    <% if (!rev.isEmpty()) { %>
         (revision <%= Util.htmlize(rev) %>)
     <% } %>
     <span id="dtag">
     <%
     String dtag = cfg.getDefineTagsIndex();
-    if (dtag.length() > 0) {
+    if (!dtag.isEmpty()) {
         %> (<%= dtag %>)<%
     }
     %></span>
diff --git a/opengrok-web/src/main/webapp/more.jsp b/opengrok-web/src/main/webapp/more.jsp
@@ -18,7 +18,7 @@ information: Portions Copyright [yyyy] [name of copyright owner]
 
 CDDL HEADER END
 
-Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
+Copyright (c) 2010, 2025, Oracle and/or its affiliates. All rights reserved.
 Portions Copyright 2011 Jens Elkner.
 Portions Copyright (c) 2018, 2020, Chris Fraire <cfraire@me.com>.
 
@@ -99,7 +99,7 @@ org.opengrok.indexer.web.SearchHelper"
                  */
                 Context sourceContext = new Context(tquery, qbuilder);
                 sourceContext.toggleAlt();
-                // SRCROOT is read with UTF-8 as a default.
+                // Files under source root are read with UTF-8 as a default.
                 try (Reader r = IOUtils.createBOMStrippedReader(
                         new FileInputStream(resourceFile),
                         StandardCharsets.UTF_8.name())) {
