From b86d481b0089effd1dc609e6e8268315aa2e7f55 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Tue, 18 Feb 2025 16:39:21 +0100 Subject: [PATCH] HTML/URI encode path/content where possible --- .../java/org/opengrok/indexer/web/Util.java | 34 ++++++------- .../org/opengrok/indexer/web/UtilTest.java | 11 ++-- opengrok-web/src/main/webapp/history.jsp | 51 ++++++++++--------- opengrok-web/src/main/webapp/httpheader.jspf | 7 +-- opengrok-web/src/main/webapp/mast.jsp | 10 ++-- opengrok-web/src/main/webapp/more.jsp | 4 +- 6 files changed, 60 insertions(+), 57 deletions(-) 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 index b82f5f61bd9..fecbb71c28b 100644 --- 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 '.' and '..' 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 - * path + * @param path the full path from which the breadcrumb path is built + * @param sep the character that separates the path components in path * @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 sep to its name * @return path 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 - * names and optionally all redundant information like "." and - * "..". + * names and optionally all redundant information like "." and "..". * * @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 index 71e676b5bc1..a5885537845 100644 --- 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 . */ package org.opengrok.indexer.web; @@ -112,8 +112,7 @@ void breadcrumbPath() { // parent directories have a trailing slash in href assertEquals("a/b", 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/b/", 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("/xx", 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/" + "c/" + "d", Util.breadcrumbPath("/r/", "../a/b/../c//d", '/', "", true)); + // path components should be URI encoded and htmlized + assertEquals("foo/" + + "bar>", + 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 index de58bdd577b..9f80cff831a 100644 --- 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 . --%> @@ -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)); %>
History log of - <%= Util.breadcrumbPath(context + Prefix.XREF_P, path,'/',"",true,cfg.isDir()) %> + <%= Util.breadcrumbPath(context + Prefix.XREF_P, path, '/', "", true, cfg.isDir()) %> (Results <%= totalHits != 0 ? startIndex + 1 : 0 %> – <%= startIndex + thisPageIndex %> of <%= totalHits %>)
@@ -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();}); <% if (cfg.isDir()) { %> - <%= dispRev %><% + <%= htmlEncodedDisplayRevision %><% } else { if (entry.isActive()) { StringBuffer urlBuffer = request.getRequestURL(); @@ -297,7 +298,7 @@ document.domReady.push(function() {domReadyHistory();}); # "><%= dispRev %> + QueryParameters.REVISION_PARAM_EQ + Util.uriEncode(rev) %>"><%= htmlEncodedDisplayRevision %> <% %> - <%= dispRev %> + <%= htmlEncodedDisplayRevision %> <% } } @@ -354,7 +355,7 @@ 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); %><%= Util.htmlize(author)%><% @@ -362,15 +363,15 @@ document.domReady.push(function() {domReadyHistory();}); %><%= Util.htmlize(author) %><% } %> - <% + <% // 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, "")) { %> -<%= jfile %>
<% +<%= Util.htmlize(jfile) %>
<% } else { %> -<%= jfile %>
<% +<%= Util.htmlize(jfile) %>
<% } } %><% diff --git a/opengrok-web/src/main/webapp/httpheader.jspf b/opengrok-web/src/main/webapp/httpheader.jspf index 4e96c7f95a3..df152ff42a6 100644 --- 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 . Portions Copyright (c) 2020, Aleksandr Kirillov . @@ -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(""); + "title=\"RSS feed for " + Util.htmlize(cfg.getPath()) + "\" " + + "href=\"" + ctxPath + Prefix.RSS_P + Util.uriEncodePath(cfg.getPath()) + "\" />"); } %> } // 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); } %> xref: - <%= 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) %>) <% } %> <% String dtag = cfg.getDefineTagsIndex(); - if (dtag.length() > 0) { + if (!dtag.isEmpty()) { %> (<%= dtag %>)<% } %> diff --git a/opengrok-web/src/main/webapp/more.jsp b/opengrok-web/src/main/webapp/more.jsp index 69816074ae7..cf452e31acd 100644 --- 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 . @@ -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())) {