From ad33a262a0326d359a2e54d713541c67998dc417 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 22 Sep 2025 09:38:42 +0200 Subject: [PATCH 1/3] add tests for PageConfig#isNotModified() --- .../java/org/opengrok/web/PageConfig.java | 30 ++++++------- .../java/org/opengrok/web/PageConfigTest.java | 42 +++++++++++++++++++ 2 files changed, 58 insertions(+), 14 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 9f93e7e4474..5b57eb169b4 100644 --- a/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java +++ b/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java @@ -38,7 +38,6 @@ import java.net.URISyntaxException; import java.net.URLDecoder; import java.nio.charset.StandardCharsets; -import java.nio.file.Paths; import java.security.InvalidParameterException; import java.util.ArrayList; import java.util.Arrays; @@ -1828,21 +1827,9 @@ private SortedSet getProjectMessages() { * @see HTTP Caching */ public boolean isNotModified(HttpServletRequest request, HttpServletResponse response) { - String currentEtag = String.format("W/\"%s\"", - Objects.hash( - // last modified time as UTC timestamp in millis - getLastModified(), - // all project related messages which changes the view - getProjectMessages(), - // last timestamp value - getEnv().getDateForLastIndexRun() != null ? getEnv().getDateForLastIndexRun().getTime() : 0, - // OpenGrok version has changed since the last time - Info.getVersion() - ) - ); + String currentEtag = getEtag(); String headerEtag = request.getHeader(HttpHeaders.IF_NONE_MATCH); - if (headerEtag != null && headerEtag.equals(currentEtag)) { // weak ETag has not changed, return 304 NOT MODIFIED response.setStatus(HttpServletResponse.SC_NOT_MODIFIED); @@ -1854,6 +1841,21 @@ public boolean isNotModified(HttpServletRequest request, HttpServletResponse res return false; } + @VisibleForTesting @NotNull String getEtag() { + return String.format("W/\"%s\"", + Objects.hash( + // last modified time as UTC timestamp in millis + getLastModified(), + // all project related messages which changes the view + getProjectMessages(), + // last timestamp value + getEnv().getDateForLastIndexRun() != null ? getEnv().getDateForLastIndexRun().getTime() : 0, + // OpenGrok version + Info.getVersion() + ) + ); + } + /** * @param root root path * @param path path diff --git a/opengrok-web/src/test/java/org/opengrok/web/PageConfigTest.java b/opengrok-web/src/test/java/org/opengrok/web/PageConfigTest.java index a8e70108f6d..e0dd781ac01 100644 --- a/opengrok-web/src/test/java/org/opengrok/web/PageConfigTest.java +++ b/opengrok-web/src/test/java/org/opengrok/web/PageConfigTest.java @@ -39,6 +39,8 @@ import java.util.stream.Stream; import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import jakarta.ws.rs.core.HttpHeaders; import org.apache.commons.lang3.tuple.Pair; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -68,6 +70,11 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.startsWith; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import static org.opengrok.indexer.condition.RepositoryInstalled.Type.MERCURIAL; import static org.opengrok.indexer.history.LatestRevisionUtil.getLatestRevision; @@ -647,4 +654,39 @@ public String getPathInfo() { } }; } + + @Test + void testIsNotModifiedEtag() { + HttpServletRequest req = new DummyHttpServletRequest() { + @Override + public String getHeader(String name) { + if (name.equals(HttpHeaders.IF_NONE_MATCH)) { + return "foo"; // will not match the hash computed in + } + return null; + } + + @Override + public String getPathInfo() { + return "path"; + } + }; + + PageConfig cfg = PageConfig.get(req); + HttpServletResponse resp = mock(HttpServletResponse.class); + assertFalse(cfg.isNotModified(req, resp)); + verify(resp).setHeader(eq(HttpHeaders.ETAG), startsWith("W/")); + } + + @Test + void testIsNotModifiedNotModified() { + DummyHttpServletRequest req = mock(DummyHttpServletRequest.class); + when(req.getPathInfo()).thenReturn("/"); + PageConfig cfg = PageConfig.get(req); + final String etag = cfg.getEtag(); + when(req.getHeader(HttpHeaders.IF_NONE_MATCH)).thenReturn(etag); + HttpServletResponse resp = mock(HttpServletResponse.class); + assertTrue(cfg.isNotModified(req, resp)); + verify(resp).setStatus(eq(HttpServletResponse.SC_NOT_MODIFIED)); + } } From bbbef58d98619bf3b689a77075962d3135403f5f Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 22 Sep 2025 14:29:52 +0200 Subject: [PATCH 2/3] remove unused method --- .../src/main/java/org/opengrok/web/PageConfig.java | 9 --------- 1 file changed, 9 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 5b57eb169b4..f527e95ee1a 100644 --- a/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java +++ b/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java @@ -1856,15 +1856,6 @@ public boolean isNotModified(HttpServletRequest request, HttpServletResponse res ); } - /** - * @param root root path - * @param path path - * @return path relative to root - */ - public static String getRelativePath(String root, String path) { - return Paths.get(root).relativize(Paths.get(path)).toString(); - } - /** * Determines whether a match offset from a search result has been indicated, * and if so tries to calculate a translated xref fragment identifier. From 07ebd4a2eec89ad1abff61e53e909110d57be025 Mon Sep 17 00:00:00 2001 From: Vladimir Kotal Date: Mon, 22 Sep 2025 14:31:03 +0200 Subject: [PATCH 3/3] rename for better readability --- opengrok-web/src/main/webapp/mast.jsp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/opengrok-web/src/main/webapp/mast.jsp b/opengrok-web/src/main/webapp/mast.jsp index 55213134fbd..e3c7516c992 100644 --- a/opengrok-web/src/main/webapp/mast.jsp +++ b/opengrok-web/src/main/webapp/mast.jsp @@ -44,12 +44,12 @@ org.opengrok.indexer.web.Util"%> return; } - String redir = cfg.canProcess(); - if (redir == null || !redir.isEmpty()) { - if (redir == null) { + String redirectLocation = cfg.canProcess(); + if (redirectLocation == null || !redirectLocation.isEmpty()) { + if (redirectLocation == null) { response.sendError(HttpServletResponse.SC_NOT_FOUND); } else { - response.sendRedirect(redir); + response.sendRedirect(redirectLocation); } return; }