HTML/URI encode content/path where possible

opengrok-indexer/src/main/java/org/opengrok/indexer/configuration/IncludeFiles.java

@@ -18,7 +18,7 @@
  */
 
 /*
- * Copyright (c) 2008, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2008, 2025, Oracle and/or its affiliates. All rights reserved.
  * Portions Copyright (c) 2020, Aleksandr Kirillov <[email protected]>.
  */
 package org.opengrok.indexer.configuration;
@@ -93,7 +93,7 @@ public String getBodyIncludeFileContent(boolean force) {
         return body;
     }
 
-    private transient String eforbidden_content = null;
+    private transient String eforbiddenContent = null;
 
     /**
      * Get the contents of the page for forbidden error page (403 Forbidden)
@@ -105,14 +105,14 @@ public String getBodyIncludeFileContent(boolean force) {
      * @see Configuration#E_FORBIDDEN_INCLUDE_FILE
      */
     public String getForbiddenIncludeFileContent(boolean force) {
-        if (eforbidden_content == null || force) {
-            eforbidden_content = getFileContent(new File(RuntimeEnvironment.getInstance().getIncludeRootPath(),
+        if (eforbiddenContent == null || force) {
+            eforbiddenContent = getFileContent(new File(RuntimeEnvironment.getInstance().getIncludeRootPath(),
                     Configuration.E_FORBIDDEN_INCLUDE_FILE));
         }
-        return eforbidden_content;
+        return eforbiddenContent;
     }
 
-    private transient String http_header = null;
+    private transient String httpHeader = null;
 
     /**
      * Get the contents of the HTTP header include file.
@@ -123,10 +123,10 @@ public String getForbiddenIncludeFileContent(boolean force) {
      * @see Configuration#HTTP_HEADER_INCLUDE_FILE
      */
     public String getHttpHeaderIncludeFileContent(boolean force) {
-        if (http_header == null || force) {
-            http_header = getFileContent(new File(RuntimeEnvironment.getInstance().getIncludeRootPath(),
+        if (httpHeader == null || force) {
+            httpHeader = getFileContent(new File(RuntimeEnvironment.getInstance().getIncludeRootPath(),
                     Configuration.HTTP_HEADER_INCLUDE_FILE));
         }
-        return http_header;
+        return httpHeader;
     }
 }

opengrok-indexer/src/main/java/org/opengrok/indexer/web/Util.java

@@ -18,7 +18,7 @@
  */
 
 /*
- * Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2025, Oracle and/or its affiliates. All rights reserved.
  * Portions Copyright (c) 2011, Jens Elkner.
  * Portions Copyright (c) 2017, 2020, Chris Fraire <[email protected]>.
  * Portions Copyright (c) 2019, Krystof Tulinger <[email protected]>.
@@ -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)
      */
@@ -401,19 +401,17 @@ public static String breadcrumbPath(String urlPrefix, String path,
     /**
      * Create a breadcrumb path to allow navigation to each element of a path.
      * Consecutive separators (<var>sep</var>) in the given <var>path</var> are
-     * always collapsed into a single separator automatically. If
-     * <var>compact</var> is {@code true} path gets translated into a canonical
+     * always collapsed into a single separator automatically.
+     * If <var>compact</var> is {@code true} path gets translated into a canonical
      * path similar to {@link File#getCanonicalPath()}, however the current
      * working directory is assumed to be "/" and no checks are done (e.g.
      * neither whether the path [component] exists nor which type it is).
      *
-     * @param urlPrefix what should be prepend 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 urlPostfix what should be append to the constructed URL
-     * @param compact if {@code true}, a canonical path gets constructed before
-     * processing.
+     * @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 urlPostfix what should be appended to the constructed URL
+     * @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.

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 <[email protected]>.
  */
 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

opengrok-web/src/main/java/org/opengrok/web/PageConfig.java

@@ -18,7 +18,7 @@
  */
 
 /*
- * Copyright (c) 2011, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2025, Oracle and/or its affiliates. All rights reserved.
  * Portions Copyright (c) 2011, Jens Elkner.
  * Portions Copyright (c) 2017, 2020, Chris Fraire <[email protected]>.
  * Portions Copyright (c) 2023, Gino Augustine <[email protected]>.
@@ -1101,7 +1101,10 @@ public Prefix getPrefix() {
      * Get the canonical path of the related resource relative to the source
      * root directory (used file separators are all {@link org.opengrok.indexer.index.Indexer#PATH_SEPARATOR}).
      * No check is made, whether the obtained path is really an accessible resource on disk.
-     *
+     * <p>
+     * Also, care needs to be taken when embedding the result in a page. Consider using {@link Util#htmlize(String)}
+     * or {@link Util#uriEncodePath(String)}.
+     * </p>
      * @return a possible empty String (denotes the source root directory) but not {@code null}.
      * @see HttpServletRequest#getPathInfo()
      */
@@ -1738,6 +1741,9 @@ public String getHistoryTitle() {
         return Util.htmlize(getShortPath(strPath) + " - OpenGrok history log for " + strPath);
     }
 
+    /**
+     * @return page title string with base name of the path and optionally revision ID, HTML encoded
+     */
     public String getPathTitle() {
         String strPath = getPath();
         String title = getShortPath(strPath);

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 <[email protected]>.
 --%>
@@ -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><%