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..f527e95ee1a 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,13 +1841,19 @@ public boolean isNotModified(HttpServletRequest request, HttpServletResponse res return false; } - /** - * @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(); + @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() + ) + ); } /** 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; } 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)); + } }