diff --git a/opengrok-web/src/main/java/org/opengrok/web/GetFile.java b/opengrok-web/src/main/java/org/opengrok/web/GetFile.java index d23f5c75088..dcaf2e4413d 100644 --- a/opengrok-web/src/main/java/org/opengrok/web/GetFile.java +++ b/opengrok-web/src/main/java/org/opengrok/web/GetFile.java @@ -18,7 +18,7 @@ */ /* - * Copyright (c) 2008, 2020, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2008, 2024, Oracle and/or its affiliates. All rights reserved. * Portions Copyright (c) 2020, Chris Fraire . * Portions Copyright (c) 2011, Jens Elkner. */ @@ -29,6 +29,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.Objects; import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServletRequest; @@ -36,9 +37,12 @@ import org.opengrok.indexer.history.HistoryGuru; import org.opengrok.indexer.web.Prefix; +/** + * Used by the webapp to serve the contents of files on /raw and /download. + */ public class GetFile extends HttpServlet { - public static final long serialVersionUID = -1; + private static final long serialVersionUID = -1; @Override protected void service(final HttpServletRequest request, final HttpServletResponse response) throws IOException { @@ -46,26 +50,29 @@ protected void service(final HttpServletRequest request, final HttpServletRespon cfg.checkSourceRootExistence(); String redir = cfg.canProcess(); - if (redir == null || redir.length() > 0) { - if (redir != null) { - response.sendRedirect(redir); - } else { - response.sendError(HttpServletResponse.SC_NOT_FOUND); - } + if (redir == null) { + response.sendError(HttpServletResponse.SC_NOT_FOUND); + return; + } + if (!redir.isEmpty()) { + response.sendRedirect(redir); return; } File f = cfg.getResourceFile(); String revision = cfg.getRequestedRevision(); - if (revision.length() == 0) { + if (revision.isEmpty()) { revision = null; } InputStream in; try { if (revision != null) { - in = HistoryGuru.getInstance().getRevision(f.getParent(), - f.getName(), revision); + in = HistoryGuru.getInstance().getRevision(f.getParent(), f.getName(), revision); + if (in == null) { + response.sendError(HttpServletResponse.SC_NOT_FOUND); + return; + } } else { long flast = cfg.getLastModified(); if (request.getDateHeader("If-Modified-Since") >= flast) { @@ -86,8 +93,7 @@ protected void service(final HttpServletRequest request, final HttpServletRespon response.setContentType(mimeType); if (cfg.getPrefix() == Prefix.DOWNLOAD_P) { - response.setHeader("content-disposition", "attachment; filename=" - + f.getName()); + response.setHeader("content-disposition", "attachment; filename=" + f.getName()); } else { response.setHeader("content-type", "text/plain"); } @@ -100,7 +106,7 @@ protected void service(final HttpServletRequest request, final HttpServletRespon o.flush(); o.close(); } finally { - in.close(); + Objects.requireNonNull(in).close(); } } } \ No newline at end of file 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 dcdb1be1f3e..d2be6e97f44 100644 --- a/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java +++ b/opengrok-web/src/main/java/org/opengrok/web/PageConfig.java @@ -1381,19 +1381,17 @@ && getUriEncodedPath().isEmpty() return req.getContextPath() + Prefix.XREF_P + '/'; } - if (getPath().length() == 0) { + if (getPath().isEmpty()) { // => / return null; } - if (prefix != Prefix.XREF_P && prefix != Prefix.HIST_L - && prefix != Prefix.RSS_P) { + if (prefix != Prefix.XREF_P && prefix != Prefix.HIST_L && prefix != Prefix.RSS_P) { // if it is an existing dir perhaps people wanted dir xref - return req.getContextPath() + Prefix.XREF_P - + getUriEncodedPath() + trailingSlash(getPath()); + return req.getContextPath() + Prefix.XREF_P + getUriEncodedPath() + trailingSlash(getPath()); } String ts = trailingSlash(getPath()); - if (ts.length() != 0) { + if (!ts.isEmpty()) { return req.getContextPath() + prefix + getUriEncodedPath() + ts; } } diff --git a/opengrok-web/src/test/java/org/opengrok/web/GetFileTest.java b/opengrok-web/src/test/java/org/opengrok/web/GetFileTest.java new file mode 100644 index 00000000000..3579d662657 --- /dev/null +++ b/opengrok-web/src/test/java/org/opengrok/web/GetFileTest.java @@ -0,0 +1,194 @@ +/* + * CDDL HEADER START + * + * The contents of this file are subject to the terms of the + * Common Development and Distribution License (the "License"). + * You may not use this file except in compliance with the License. + * + * See LICENSE.txt included in this distribution for the specific + * language governing permissions and limitations under the License. + * + * When distributing Covered Code, include this CDDL HEADER in each + * file and include the License file at LICENSE.txt. + * If applicable, add the following below this CDDL HEADER, with the + * fields enclosed by brackets "[]" replaced with your own identifying + * information: Portions Copyright [yyyy] [name of copyright owner] + * + * CDDL HEADER END + */ + +/* + * Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved. + */ +package org.opengrok.web; + +import jakarta.servlet.ServletConfig; +import jakarta.servlet.ServletContext; +import jakarta.servlet.ServletOutputStream; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.ArgumentMatchers; +import org.opengrok.indexer.configuration.RuntimeEnvironment; +import org.opengrok.indexer.util.IOUtils; +import org.opengrok.indexer.web.DummyHttpServletRequest; +import org.opengrok.indexer.web.Prefix; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Provides set of functional tests for the {@link GetFile} class. + */ +class GetFileTest { + private static Path sourceRoot; + + private static Path sourceFile; + private static final String fileContent = "int main();"; + + private static final RuntimeEnvironment env = RuntimeEnvironment.getInstance(); + + @BeforeAll + public static void setUpClass() throws IOException { + sourceRoot = Files.createTempDirectory("tmpDirPrefix"); + sourceFile = Files.createFile(Path.of(sourceRoot.toString(), "foo.c")); + Files.writeString(sourceFile, fileContent, StandardCharsets.UTF_8); + env.setSourceRoot(sourceRoot.toString()); + } + + @AfterAll + public static void tearDownClass() throws IOException { + IOUtils.removeRecursive(sourceRoot); + } + + @Test + void testGetFileNotFound() throws IOException { + GetFile getFile = new GetFile(); + final String relativePath = "/project/nonexistent.c"; + HttpServletRequest request = new DummyHttpServletRequest() { + @Override + public String getPathInfo() { + return relativePath; + } + }; + assertFalse(Path.of(env.getSourceRootPath(), relativePath).toFile().exists()); + HttpServletResponse response = mock(HttpServletResponse.class); + getFile.service(request, response); + verify(response).sendError(HttpServletResponse.SC_NOT_FOUND); + } + + @Test + void testGetFileNoRevision() throws Exception { + GetFile getFile = new GetFile(); + final String relativePath = env.getPathRelativeToSourceRoot(sourceFile.toFile()); + HttpServletRequest request = new DummyHttpServletRequest() { + @Override + public String getPathInfo() { + return relativePath; + } + + @Override + public String getServletPath() { + return Prefix.DOWNLOAD_P.toString(); + } + + public String getParameter(String s) { + return "NA"; + } + }; + assertTrue(Path.of(env.getSourceRootPath(), relativePath).toFile().exists()); + HttpServletResponse response = mock(HttpServletResponse.class); + getFile.service(request, response); + verify(response).sendError(HttpServletResponse.SC_NOT_FOUND); + } + + @Test + void testGetFileRedirect() throws Exception { + GetFile getFile = new GetFile(); + final String relativePath = "/project"; + Path dir = Path.of(env.getSourceRootPath(), relativePath); + Files.createDirectory(dir); + final String contextPath = "ctx"; + final String prefix = Prefix.XREF_P.toString(); + HttpServletRequest request = new DummyHttpServletRequest() { + @Override + public String getPathInfo() { + return ""; + } + + @Override + public String getRequestURI() { + return "foo"; + } + + @Override + public String getServletPath() { + return prefix; + } + + @Override + public String getContextPath() { + return contextPath; + } + }; + assertTrue(Path.of(env.getSourceRootPath(), relativePath).toFile().isDirectory()); + HttpServletResponse response = mock(HttpServletResponse.class); + getFile.service(request, response); + verify(response).sendRedirect(contextPath + prefix + "/"); + } + + private static Stream getFileWritePrefixParams() { + return Stream.of(Prefix.DOWNLOAD_P, Prefix.RAW_P); + } + + @ParameterizedTest + @MethodSource("getFileWritePrefixParams") + void testGetFileWrite(Prefix prefix) throws Exception { + GetFile getFileOrig = new GetFile(); + ServletConfig config = mock(ServletConfig.class); + getFileOrig.init(config); + GetFile getFile = spy(getFileOrig); + when(config.getServletContext()).thenReturn(mock(ServletContext.class)); + when(getFile.getServletContext().getMimeType(anyString())).thenReturn("text/plain"); + final String relativePath = env.getPathRelativeToSourceRoot(sourceFile.toFile()); + HttpServletRequest request = new DummyHttpServletRequest() { + @Override + public String getPathInfo() { + return relativePath; + } + + @Override + public String getServletPath() { + return prefix.toString(); + } + + @Override + public long getDateHeader(String s) { + return 1; + } + }; + assertTrue(Path.of(env.getSourceRootPath(), relativePath).toFile().exists()); + HttpServletResponse response = mock(HttpServletResponse.class); + ServletOutputStream outputStream = mock(ServletOutputStream.class); + when(response.getOutputStream()).thenReturn(outputStream); + getFile.service(request, response); + verify(outputStream).write(ArgumentMatchers.isNotNull(), eq(0), eq(fileContent.length())); + verify(outputStream).close(); + } +}