From a4b0cad8f71f6b0dda8bb40a22825383fd2dc9f6 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 1 Sep 2025 15:59:33 +0200 Subject: [PATCH 1/4] diff.jsp and DiffData cleanup --- .../java/org/opengrok/web/PageConfig.java | 6 +- opengrok-web/src/main/webapp/diff.jsp | 126 ++++++++++-------- 2 files changed, 71 insertions(+), 61 deletions(-) diff --git a/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java b/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java index d79f7d52cd4..bc6d0aeac18 100644 --- a/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java +++ b/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java @@ -276,7 +276,7 @@ private boolean getFileRevision(DiffData data, String context, String[] filepath */ public DiffData getDiffData() { DiffData data = new DiffData(getPath().substring(0, getPath().lastIndexOf(PATH_SEPARATOR)), - Util.htmlize(getResourceFile().getName())); + getResourceFile().getName()); String context = req.getContextPath(); String[] filepath = new String[2]; @@ -372,7 +372,7 @@ private void populateRevisionData(DiffData data) { try { data.revision = Diff.diff(data.file[0], data.file[1]); } catch (DifferentiationFailedException e) { - data.errorMsg = "Unable to get diffs: " + Util.htmlize(e.getMessage()); + data.errorMsg = "Unable to get diffs: " + e.getMessage(); } } @@ -384,7 +384,7 @@ private void populateRevisionURLDetails(DiffData data, String[] filePath) { filePath[i] + "@" + data.rev[i], null); data.param[i] = u.getRawQuery(); } catch (URISyntaxException e) { - LOGGER.log(Level.WARNING, "Failed to create URI: ", e); + LOGGER.log(Level.WARNING, "Failed to create URI: ", Laundromat.launderLog(e.toString())); } }); } diff --git a/opengrok-web/src/main/webapp/diff.jsp b/opengrok-web/src/main/webapp/diff.jsp index 75a296a97bb..42924e31732 100644 --- a/opengrok-web/src/main/webapp/diff.jsp +++ b/opengrok-web/src/main/webapp/diff.jsp @@ -16,7 +16,7 @@ information: Portions Copyright [yyyy] [name of copyright owner] CDDL HEADER END -Copyright (c) 2006, 2023, Oracle and/or its affiliates. All rights reserved. +Copyright (c) 2006, 2025, Oracle and/or its affiliates. All rights reserved. Portions Copyright 2011 Jens Elkner. Portions Copyright (c) 2020, Chris Fraire . --%> @@ -48,10 +48,8 @@ private String getAnnotateRevision(DiffData data) { PageConfig cfg = PageConfig.get(request); cfg.addScript("diff"); cfg.checkSourceRootExistence(); - /** - * This block must be the first block before any other output in the - * response. - * + /* + * This block must be the first block before any other output in the response. * If there is already any output written into the response, and we * use the same response and reset the content and the headers then we have * a collision with the response streams and the "getOutputStream() has @@ -64,8 +62,8 @@ private String getAnnotateRevision(DiffData data) { && request.getParameter("action").equals("download")) { try (OutputStream o = response.getOutputStream()) { for (int i = 0; i < data.getRevision().size(); i++) { - Delta d = data.getRevision().getDelta(i); - try (InputStream in = new ByteArrayInputStream(d.toString().getBytes(StandardCharsets.UTF_8))) { + Delta delta = data.getRevision().getDelta(i); + try (InputStream in = new ByteArrayInputStream(delta.toString().getBytes(StandardCharsets.UTF_8))) { response.setHeader("content-disposition", "attachment; filename=" + cfg.getResourceFile().getName() + "@" + data.getRev(0) + "-" + data.getRev(1) + ".diff"); @@ -98,24 +96,27 @@ include file="/mast.jsp" %>

Error:

-

<%= data.getErrorMsg() %>

+

<%= Util.htmlize(data.getErrorMsg()) %>

<% } else if (data.getGenre() == AbstractAnalyzer.Genre.IMAGE) { - String link = request.getContextPath() + Prefix.DOWNLOAD_P - + Util.htmlize(cfg.getPath()); + String link = request.getContextPath() + Prefix.DOWNLOAD_P + Util.htmlize(cfg.getPath()); %>
- - + + + - + - @@ -124,14 +125,15 @@ include file="/mast.jsp" } else if (data.getGenre() != AbstractAnalyzer.Genre.PLAIN && data.getGenre() != AbstractAnalyzer.Genre.HTML) { - String link = request.getContextPath() + Prefix.DOWNLOAD_P - + Util.htmlize(cfg.getPath()); + String link = request.getContextPath() + Prefix.DOWNLOAD_P + Util.uriEncodePath(cfg.getPath()); %> -
Diffs for binary files cannot be displayed! Files are <%= - data.getFilename() %>(revision <%= data.getRev(0) %>) and <%= - data.getFilename() %>(revision <%= data.getRev(1) %>). +<% } else if (data.getRevision().size() == 0) { @@ -163,9 +165,9 @@ include file="/mast.jsp" if (type == t) { %> <%= t.toString() %><% if (t == DiffType.OLD) { - %> (<%= data.getRev(0) %>)<% + %> (<%= Util.htmlize(data.getRev(0)) %>)<% } else if (t == DiffType.NEW) { - %> (<%= data.getRev(1) %>)<% + %> (<%= Util.htmlize(data.getRev(1)) %>)<% } %><% } else { @@ -175,9 +177,9 @@ include file="/mast.jsp" <%= QueryParameters.DIFF_LEVEL_PARAM_EQ %><%= full ? '1' : '0'%>"><%= t.toString() %> <% if (t == DiffType.OLD) { - %> (<%= data.getShortRev(0) %>)<% + %> (<%= Util.htmlize(data.getShortRev(0)) %>)<% } else if (t == DiffType.NEW) { - %> (<%= data.getShortRev(1) %>)<% + %> (<%= Util.htmlize(data.getShortRev(1)) %>)<% } %><% } @@ -186,22 +188,22 @@ include file="/mast.jsp"
<% if (!full) { %> - ?<%= QueryParameters.REVISION_1_PARAM_EQ %><%= Util.uriEncode(rp1) %>& +<%= QueryParameters.REVISION_2_PARAM_EQ %><%= Util.uriEncode(rp2) %>& <%= QueryParameters.FORMAT_PARAM_EQ %><%= type.getAbbrev() %>& <%= QueryParameters.DIFF_LEVEL_PARAM_EQ %>1">full compact<% } else { %> full - ?<%= QueryParameters.REVISION_1_PARAM_EQ %><%= Util.uriEncode(rp1) %>& +<%= QueryParameters.REVISION_2_PARAM_EQ %><%= Util.uriEncode(rp2) %>& <%= QueryParameters.FORMAT_PARAM_EQ %><%= type.getAbbrev() %>& <%= QueryParameters.DIFF_LEVEL_PARAM_EQ %>0">compact<% } %>jumper - ?<%= QueryParameters.REVISION_1_PARAM_EQ %><%= Util.uriEncode(rp1) %>& +<%= QueryParameters.REVISION_2_PARAM_EQ %><%= Util.uriEncode(rp2) %>& <%= QueryParameters.FORMAT_PARAM_EQ %><%= DiffType.TEXT %>& action=download">download diff<% %>
@@ -212,25 +214,33 @@ action=download">download diff<% if (type == DiffType.SIDEBYSIDE || type == DiffType.UNIFIED) { %>
<%= data.getFilename() %> (revision <%= data.getRev(0) %>)<%= data.getFilename() %> (revision <%= data.getRev(1) %>)
<%= Util.htmlize(data.getFilename()) %> (revision <%= Util.htmlize(data.getRev(0)) %>)<%= Util.htmlize(data.getFilename()) %> (revision <%= Util.htmlize(data.getRev(1)) %>)
previous image +
+ previous image new image + + new image
<% if (type == DiffType.SIDEBYSIDE) { - String linkPrefix = request.getContextPath() + Prefix.XREF_P + Util.htmlize(cfg.getPath()) + + String linkPrefix = request.getContextPath() + Prefix.XREF_P + Util.uriEncodePath(cfg.getPath()) + "?" + QueryParameters.REVISION_PARAM_EQ; %> - - + + <% } %> <% } - for (int i=0; i < data.getRevision().size(); i++) { - Delta d = data.getRevision().getDelta(i); + for (int i = 0; i < data.getRevision().size(); i++) { + Delta delta = data.getRevision().getDelta(i); if (type == DiffType.TEXT) { - %><%= Util.htmlize(d.toString()) %><% + %><%= Util.htmlize(delta.toString()) %><% } else { - Chunk c1 = d.getOriginal(); - Chunk c2 = d.getRevised(); + Chunk c1 = delta.getOriginal(); + Chunk c2 = delta.getRevised(); int cn1 = c1.first(); int cl1 = c1.last(); int cn2 = c2.first(); @@ -295,7 +305,7 @@ action=download">download diff<% if (cn1 <= cl1) { %> @@ -331,7 +341,7 @@ action=download">download diff<% Util.htmlize(file1[j]) %>
<% } %> @@ -473,12 +483,12 @@ action=download">download diff<% } else { %> @@ -501,7 +511,7 @@ action=download">download diff<% } else { %> @@ -484,12 +484,12 @@ action=download">download diff<% %> @@ -502,7 +502,7 @@ action=download">download diff<% %> @@ -512,7 +512,7 @@ action=download">download diff<% %> @@ -523,11 +523,11 @@ action=download">download diff<% } else if (type == DiffType.OLD) { if (full || file1.length - ln1 < 20) { for (int j = ln1; j < file1.length; j++) { - %><%= j+1 %><%= Util.htmlize(file1[j]) %>
<% + %><%= j + 1 %><%= Util.htmlize(file1[j]) %>
<% } } else { for (int j = ln1; j < ln1 + 8; j++) { - %><%= j+1 %><%= Util.htmlize(file1[j]) %>
<% + %><%= j + 1 %><%= Util.htmlize(file1[j]) %>
<% } %>
--- <%= file1.length - ln1 - 8 %> unchanged lines hidden ---
<% @@ -535,11 +535,11 @@ action=download">download diff<% } else if (type == DiffType.NEW) { if (full || file2.length - ln2 < 20) { for (int j = ln2; j < file2.length; j++) { - %><%= j+1 %><%=Util.htmlize(file2[j])%>
<% + %><%= j + 1 %><%=Util.htmlize(file2[j])%>
<% } } else { for (int j = ln2; j < ln2 + 8; j++) { - %><%= j+1 %><%= Util.htmlize(file2[j]) %>
<% + %><%= j + 1 %><%= Util.htmlize(file2[j]) %>
<% } %>
--- <%= file2.length - ln2 - 8 %> unchanged lines hidden ---
<% From 6f2abe07bca3916446f9705988abd37011d7bb3f Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 1 Sep 2025 16:54:25 +0200 Subject: [PATCH 3/4] avoid white space in links --- opengrok-web/src/main/webapp/diff.jsp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/opengrok-web/src/main/webapp/diff.jsp b/opengrok-web/src/main/webapp/diff.jsp index 5df376aea15..021cbf482e7 100644 --- a/opengrok-web/src/main/webapp/diff.jsp +++ b/opengrok-web/src/main/webapp/diff.jsp @@ -219,14 +219,10 @@ action=download">download diff<% %> <% } From 6a978c40462a9d0f015c68d25aa2631e031db958 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 1 Sep 2025 16:58:30 +0200 Subject: [PATCH 4/4] further reduce white space --- opengrok-web/src/main/webapp/diff.jsp | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/opengrok-web/src/main/webapp/diff.jsp b/opengrok-web/src/main/webapp/diff.jsp index 021cbf482e7..19b232a9b3f 100644 --- a/opengrok-web/src/main/webapp/diff.jsp +++ b/opengrok-web/src/main/webapp/diff.jsp @@ -112,11 +112,9 @@ include file="/mast.jsp" - - @@ -218,12 +216,8 @@ action=download">download diff<% "?" + QueryParameters.REVISION_PARAM_EQ; %> - - + + <% } %>
<%= data.getFilename() %> (<%= data.getRev(0) %>)<%= data.getFilename() %> (<%= data.getRev(1) %>) + + <%= Util.htmlize(data.getFilename()) %> (<%= Util.htmlize(data.getRev(0)) %>) + + + + <%= Util.htmlize(data.getFilename()) %> (<%= Util.htmlize(data.getRev(1)) %>) + +
<% - for (int j = cn1; j <= cl1 ; j++) { + for (int j = cn1; j <= cl1 ; j++) { %><%= ++ln1 %><%= file1[j] %>
<% } @@ -309,12 +319,12 @@ action=download">download diff<% %> chunk<% } %>">		<% - for (int j = cn2; j < cl2; j++) { + for (int j = cn2; j < cl2; j++) { %><%= ++ln2 %><%= file2[j] %>
<% } %><%= ++ln2 %><%= file2[cl2] %><% - if(full) { + if (full) { %><% } %>		<% - for (int j = ln2; j < cn2 ; j++) { + for (int j = ln2; j < cn2 ; j++) { %><%= ++ln2 %><%= Util.htmlize(file2[j]) %>
<% } @@ -375,11 +385,11 @@ action=download">download diff<% } %>
<% - for (int j = cn1; j <= cl1; j++) { + for (int j = cn1; j <= cl1; j++) { %><%= ++ln1 %><%= file1[j] %>
<% } %>		<% - for (int j = cn2; j <= cl2; j++) { + for (int j = cn2; j <= cl2; j++) { %><%= ++ln2 %><%= file2[j] %>
<% } @@ -412,7 +422,7 @@ action=download">download diff<% } } } - for (int j = cn1; j <= cl1 ; j++) { + for (int j = cn1; j <= cl1 ; j++) { %><%= ++ln1 %><%= file1[j] %>
<% } if (full) { @@ -422,7 +432,7 @@ action=download">download diff<% } else if (type == DiffType.NEW) { if (cn2 > ln2) { if (full || cn2 - ln2 < 20) { - for (int j = ln2; j < cn2 ; j++) { + for (int j = ln2; j < cn2 ; j++) { %><%= ++ln2 %><%= Util.htmlize(file2[j]) %>
<% } @@ -444,7 +454,7 @@ action=download">download diff<% } } } - for (int j = cn2; j <= cl2 ; j++) { + for (int j = cn2; j <= cl2 ; j++) { %><%= ++ln2 %><%= file2[j] %>
<% } if (full) { @@ -459,11 +469,11 @@ action=download">download diff<% if (full || file1.length - ln1 < 20) { %>
<% - for (int j = ln1; j < file1.length ; j++) { + for (int j = ln1; j < file1.length; j++) { %><%= j+1 %><%= Util.htmlize(file1[j]) %>
<% } %>		<% - for (int j = ln2; j < file2.length ; j++) { + for (int j = ln2; j < file2.length; j++) { %><%= j+1 %><%= Util.htmlize(file2[j]) %>
<% } %>
<% - for (int j = ln1; j < ln1 + 8 ; j++) { + for (int j = ln1; j < ln1 + 8; j++) { %><%= j+1 %><%= Util.htmlize(file1[j]) %>
<% } %>
--- <%= file1.length - ln1 - 8 %> unchanged lines hidden --- 		<% - for (int j = ln2; j < ln2 + 8 ; j++) { + for (int j = ln2; j < ln2 + 8; j++) { %><%= j+1 %><%= Util.htmlize(file2[j]) %>
<% } %>
--- <%= file1.length - ln1 - 8 @@ -491,7 +501,7 @@ action=download">download diff<% if (full || file2.length - ln2 < 20) { %>
<% - for (int j = ln2; j < file2.length ; j++) { + for (int j = ln2; j < file2.length; j++) { %><%= j+1 %><%= Util.htmlize(file2[j]) %>
<% } %>
<% - for (int j = ln2; j < ln2 + 8 ; j++) { + for (int j = ln2; j < ln2 + 8; j++) { %><%= j+1 %><%= Util.htmlize(file2[j]) %>
<% } %>
--- <%= file2.length - ln2 - 8 @@ -512,11 +522,11 @@ action=download">download diff<% } } else if (type == DiffType.OLD) { if (full || file1.length - ln1 < 20) { - for (int j = ln1; j < file1.length ; j++) { + for (int j = ln1; j < file1.length; j++) { %><%= j+1 %><%= Util.htmlize(file1[j]) %>
<% } } else { - for (int j = ln1; j < ln1 + 8 ; j++) { + for (int j = ln1; j < ln1 + 8; j++) { %><%= j+1 %><%= Util.htmlize(file1[j]) %>
<% } %>
--- <%= file1.length - ln1 - 8 @@ -524,11 +534,11 @@ action=download">download diff<% } } else if (type == DiffType.NEW) { if (full || file2.length - ln2 < 20) { - for (int j = ln2; j < file2.length ; j++) { + for (int j = ln2; j < file2.length; j++) { %><%= j+1 %><%=Util.htmlize(file2[j])%>
<% } } else { - for (int j = ln2; j < ln2 + 8 ; j++) { + for (int j = ln2; j < ln2 + 8; j++) { %><%= j+1 %><%= Util.htmlize(file2[j]) %>
<% } %>
--- <%= file2.length - ln2 - 8 From 1aab66ce641a4560c57415d03b5dec6b8e61f4b8 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 1 Sep 2025 16:03:58 +0200 Subject: [PATCH 2/4] binary operators should be surrounded by space --- opengrok-web/src/main/webapp/diff.jsp | 30 +++++++++++++-------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/opengrok-web/src/main/webapp/diff.jsp b/opengrok-web/src/main/webapp/diff.jsp index 42924e31732..5df376aea15 100644 --- a/opengrok-web/src/main/webapp/diff.jsp +++ b/opengrok-web/src/main/webapp/diff.jsp @@ -283,7 +283,7 @@ action=download">download diff<% } } else { for (int j = ln2; j < ln2 + 8; j++) { - %><%= j+1 %><%= + %><%= j + 1 %><%= Util.htmlize(file2[j]) %>
<% } %>
--- <%= cn2 - ln2 - 16 @@ -347,7 +347,7 @@ action=download">download diff<% } } else { for (int j = ln1; j < ln1 + 8; j++) { - %><%= j+1 %><%= + %><%= j + 1 %><%= Util.htmlize(file1[j]) %>
<% } %>
--- <%= cn1 - ln1 - 16 @@ -364,7 +364,7 @@ action=download">download diff<% } %>		<% for (int j = ln2; j < ln2 + 8; j++) { - %><%= j+1 %><%= + %><%= j + 1 %><%= Util.htmlize(file2[j]) %>
<% } %>
--- <%= cn2 - ln2 - 16 @@ -406,7 +406,7 @@ action=download">download diff<% } } else { for (int j = ln1; j < ln1 + 8; j++) { - %><%= j+1 %><%= + %><%= j + 1 %><%= Util.htmlize(file1[j]) %>
<% } %>
--- <%= cn1 - ln1 - 16 @@ -438,7 +438,7 @@ action=download">download diff<% } } else { for (int j = ln2; j < ln2 + 8; j++) { - %><%= j+1 %><%= + %><%= j + 1 %><%= Util.htmlize(file2[j]) %>
<% } %>
--- <%= cn2 - ln2 - 16 @@ -470,11 +470,11 @@ action=download">download diff<% %>
<% for (int j = ln1; j < file1.length; j++) { - %><%= j+1 %><%= Util.htmlize(file1[j]) %>
<% + %><%= j + 1 %><%= Util.htmlize(file1[j]) %>
<% } %>		<% for (int j = ln2; j < file2.length; j++) { - %><%= j+1 %><%= Util.htmlize(file2[j]) %>
<% + %><%= j + 1 %><%= Util.htmlize(file2[j]) %>
<% } %>
<% for (int j = ln1; j < ln1 + 8; j++) { - %><%= j+1 %><%= Util.htmlize(file1[j]) %>
<% + %><%= j + 1 %><%= Util.htmlize(file1[j]) %>
<% } %>
--- <%= file1.length - ln1 - 8 %> unchanged lines hidden --- 		<% for (int j = ln2; j < ln2 + 8; j++) { - %><%= j+1 %><%= Util.htmlize(file2[j]) %>
<% + %><%= j + 1 %><%= Util.htmlize(file2[j]) %>
<% } %>
--- <%= file1.length - ln1 - 8 %> unchanged lines hidden ---
<% for (int j = ln2; j < file2.length; j++) { - %><%= j+1 %><%= Util.htmlize(file2[j]) %>
<% + %><%= j + 1 %><%= Util.htmlize(file2[j]) %>
<% } %>
<% for (int j = ln2; j < ln2 + 8; j++) { - %><%= j+1 %><%= Util.htmlize(file2[j]) %>
<% + %><%= j + 1 %><%= Util.htmlize(file2[j]) %>
<% } %>
--- <%= file2.length - ln2 - 8 %> unchanged lines hidden ---
- - <%= Util.htmlize(data.getFilename()) %> (<%= Util.htmlize(data.getRev(0)) %>) - + <%= Util.htmlize(data.getFilename()) %> (<%= Util.htmlize(data.getRev(0)) %>) - - <%= Util.htmlize(data.getFilename()) %> (<%= Util.htmlize(data.getRev(1)) %>) - + <%= Util.htmlize(data.getFilename()) %> (<%= Util.htmlize(data.getRev(1)) %>)
- previous image + previous image - new image + new image
- <%= Util.htmlize(data.getFilename()) %> (<%= Util.htmlize(data.getRev(0)) %>) - - <%= Util.htmlize(data.getFilename()) %> (<%= Util.htmlize(data.getRev(1)) %>) - <%= Util.htmlize(data.getFilename()) %> (<%= Util.htmlize(data.getRev(0)) %>)<%= Util.htmlize(data.getFilename()) %> (<%= Util.htmlize(data.getRev(1)) %>)