Skip to content

Commit 0e0e613

Browse files
authored
add test coverage for GetFile (#4635)
1 parent b2518da commit 0e0e613

File tree

3 files changed

+218
-20
lines changed

3 files changed

+218
-20
lines changed

opengrok-web/src/main/java/org/opengrok/web/GetFile.java

+20-14
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919

2020
/*
21-
* Copyright (c) 2008, 2020, Oracle and/or its affiliates. All rights reserved.
21+
* Copyright (c) 2008, 2024, Oracle and/or its affiliates. All rights reserved.
2222
* Portions Copyright (c) 2020, Chris Fraire <[email protected]>.
2323
* Portions Copyright (c) 2011, Jens Elkner.
2424
*/
@@ -29,43 +29,50 @@
2929
import java.io.IOException;
3030
import java.io.InputStream;
3131
import java.io.OutputStream;
32+
import java.util.Objects;
3233

3334
import jakarta.servlet.http.HttpServlet;
3435
import jakarta.servlet.http.HttpServletRequest;
3536
import jakarta.servlet.http.HttpServletResponse;
3637
import org.opengrok.indexer.history.HistoryGuru;
3738
import org.opengrok.indexer.web.Prefix;
3839

40+
/**
41+
* Used by the webapp to serve the contents of files on /raw and /download.
42+
*/
3943
public class GetFile extends HttpServlet {
4044

41-
public static final long serialVersionUID = -1;
45+
private static final long serialVersionUID = -1;
4246

4347
@Override
4448
protected void service(final HttpServletRequest request, final HttpServletResponse response) throws IOException {
4549
PageConfig cfg = PageConfig.get(request);
4650
cfg.checkSourceRootExistence();
4751

4852
String redir = cfg.canProcess();
49-
if (redir == null || redir.length() > 0) {
50-
if (redir != null) {
51-
response.sendRedirect(redir);
52-
} else {
53-
response.sendError(HttpServletResponse.SC_NOT_FOUND);
54-
}
53+
if (redir == null) {
54+
response.sendError(HttpServletResponse.SC_NOT_FOUND);
55+
return;
56+
}
57+
if (!redir.isEmpty()) {
58+
response.sendRedirect(redir);
5559
return;
5660
}
5761

5862
File f = cfg.getResourceFile();
5963
String revision = cfg.getRequestedRevision();
60-
if (revision.length() == 0) {
64+
if (revision.isEmpty()) {
6165
revision = null;
6266
}
6367

6468
InputStream in;
6569
try {
6670
if (revision != null) {
67-
in = HistoryGuru.getInstance().getRevision(f.getParent(),
68-
f.getName(), revision);
71+
in = HistoryGuru.getInstance().getRevision(f.getParent(), f.getName(), revision);
72+
if (in == null) {
73+
response.sendError(HttpServletResponse.SC_NOT_FOUND);
74+
return;
75+
}
6976
} else {
7077
long flast = cfg.getLastModified();
7178
if (request.getDateHeader("If-Modified-Since") >= flast) {
@@ -86,8 +93,7 @@ protected void service(final HttpServletRequest request, final HttpServletRespon
8693
response.setContentType(mimeType);
8794

8895
if (cfg.getPrefix() == Prefix.DOWNLOAD_P) {
89-
response.setHeader("content-disposition", "attachment; filename="
90-
+ f.getName());
96+
response.setHeader("content-disposition", "attachment; filename=" + f.getName());
9197
} else {
9298
response.setHeader("content-type", "text/plain");
9399
}
@@ -100,7 +106,7 @@ protected void service(final HttpServletRequest request, final HttpServletRespon
100106
o.flush();
101107
o.close();
102108
} finally {
103-
in.close();
109+
Objects.requireNonNull(in).close();
104110
}
105111
}
106112
}

opengrok-web/src/main/java/org/opengrok/web/PageConfig.java

+4-6
Original file line numberDiff line numberDiff line change
@@ -1381,19 +1381,17 @@ && getUriEncodedPath().isEmpty()
13811381
return req.getContextPath() + Prefix.XREF_P + '/';
13821382
}
13831383

1384-
if (getPath().length() == 0) {
1384+
if (getPath().isEmpty()) {
13851385
// => /
13861386
return null;
13871387
}
13881388

1389-
if (prefix != Prefix.XREF_P && prefix != Prefix.HIST_L
1390-
&& prefix != Prefix.RSS_P) {
1389+
if (prefix != Prefix.XREF_P && prefix != Prefix.HIST_L && prefix != Prefix.RSS_P) {
13911390
// if it is an existing dir perhaps people wanted dir xref
1392-
return req.getContextPath() + Prefix.XREF_P
1393-
+ getUriEncodedPath() + trailingSlash(getPath());
1391+
return req.getContextPath() + Prefix.XREF_P + getUriEncodedPath() + trailingSlash(getPath());
13941392
}
13951393
String ts = trailingSlash(getPath());
1396-
if (ts.length() != 0) {
1394+
if (!ts.isEmpty()) {
13971395
return req.getContextPath() + prefix + getUriEncodedPath() + ts;
13981396
}
13991397
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
/*
2+
* CDDL HEADER START
3+
*
4+
* The contents of this file are subject to the terms of the
5+
* Common Development and Distribution License (the "License").
6+
* You may not use this file except in compliance with the License.
7+
*
8+
* See LICENSE.txt included in this distribution for the specific
9+
* language governing permissions and limitations under the License.
10+
*
11+
* When distributing Covered Code, include this CDDL HEADER in each
12+
* file and include the License file at LICENSE.txt.
13+
* If applicable, add the following below this CDDL HEADER, with the
14+
* fields enclosed by brackets "[]" replaced with your own identifying
15+
* information: Portions Copyright [yyyy] [name of copyright owner]
16+
*
17+
* CDDL HEADER END
18+
*/
19+
20+
/*
21+
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
22+
*/
23+
package org.opengrok.web;
24+
25+
import jakarta.servlet.ServletConfig;
26+
import jakarta.servlet.ServletContext;
27+
import jakarta.servlet.ServletOutputStream;
28+
import jakarta.servlet.http.HttpServletRequest;
29+
import jakarta.servlet.http.HttpServletResponse;
30+
import org.junit.jupiter.api.AfterAll;
31+
import org.junit.jupiter.api.BeforeAll;
32+
import org.junit.jupiter.api.Test;
33+
import org.junit.jupiter.params.ParameterizedTest;
34+
import org.junit.jupiter.params.provider.MethodSource;
35+
import org.mockito.ArgumentMatchers;
36+
import org.opengrok.indexer.configuration.RuntimeEnvironment;
37+
import org.opengrok.indexer.util.IOUtils;
38+
import org.opengrok.indexer.web.DummyHttpServletRequest;
39+
import org.opengrok.indexer.web.Prefix;
40+
41+
import java.io.IOException;
42+
import java.nio.charset.StandardCharsets;
43+
import java.nio.file.Files;
44+
import java.nio.file.Path;
45+
import java.util.stream.Stream;
46+
47+
import static org.junit.jupiter.api.Assertions.assertFalse;
48+
import static org.junit.jupiter.api.Assertions.assertTrue;
49+
import static org.mockito.ArgumentMatchers.anyString;
50+
import static org.mockito.ArgumentMatchers.eq;
51+
import static org.mockito.Mockito.mock;
52+
import static org.mockito.Mockito.spy;
53+
import static org.mockito.Mockito.verify;
54+
import static org.mockito.Mockito.when;
55+
56+
/**
57+
* Provides set of functional tests for the {@link GetFile} class.
58+
*/
59+
class GetFileTest {
60+
private static Path sourceRoot;
61+
62+
private static Path sourceFile;
63+
private static final String fileContent = "int main();";
64+
65+
private static final RuntimeEnvironment env = RuntimeEnvironment.getInstance();
66+
67+
@BeforeAll
68+
public static void setUpClass() throws IOException {
69+
sourceRoot = Files.createTempDirectory("tmpDirPrefix");
70+
sourceFile = Files.createFile(Path.of(sourceRoot.toString(), "foo.c"));
71+
Files.writeString(sourceFile, fileContent, StandardCharsets.UTF_8);
72+
env.setSourceRoot(sourceRoot.toString());
73+
}
74+
75+
@AfterAll
76+
public static void tearDownClass() throws IOException {
77+
IOUtils.removeRecursive(sourceRoot);
78+
}
79+
80+
@Test
81+
void testGetFileNotFound() throws IOException {
82+
GetFile getFile = new GetFile();
83+
final String relativePath = "/project/nonexistent.c";
84+
HttpServletRequest request = new DummyHttpServletRequest() {
85+
@Override
86+
public String getPathInfo() {
87+
return relativePath;
88+
}
89+
};
90+
assertFalse(Path.of(env.getSourceRootPath(), relativePath).toFile().exists());
91+
HttpServletResponse response = mock(HttpServletResponse.class);
92+
getFile.service(request, response);
93+
verify(response).sendError(HttpServletResponse.SC_NOT_FOUND);
94+
}
95+
96+
@Test
97+
void testGetFileNoRevision() throws Exception {
98+
GetFile getFile = new GetFile();
99+
final String relativePath = env.getPathRelativeToSourceRoot(sourceFile.toFile());
100+
HttpServletRequest request = new DummyHttpServletRequest() {
101+
@Override
102+
public String getPathInfo() {
103+
return relativePath;
104+
}
105+
106+
@Override
107+
public String getServletPath() {
108+
return Prefix.DOWNLOAD_P.toString();
109+
}
110+
111+
public String getParameter(String s) {
112+
return "NA";
113+
}
114+
};
115+
assertTrue(Path.of(env.getSourceRootPath(), relativePath).toFile().exists());
116+
HttpServletResponse response = mock(HttpServletResponse.class);
117+
getFile.service(request, response);
118+
verify(response).sendError(HttpServletResponse.SC_NOT_FOUND);
119+
}
120+
121+
@Test
122+
void testGetFileRedirect() throws Exception {
123+
GetFile getFile = new GetFile();
124+
final String relativePath = "/project";
125+
Path dir = Path.of(env.getSourceRootPath(), relativePath);
126+
Files.createDirectory(dir);
127+
final String contextPath = "ctx";
128+
final String prefix = Prefix.XREF_P.toString();
129+
HttpServletRequest request = new DummyHttpServletRequest() {
130+
@Override
131+
public String getPathInfo() {
132+
return "";
133+
}
134+
135+
@Override
136+
public String getRequestURI() {
137+
return "foo";
138+
}
139+
140+
@Override
141+
public String getServletPath() {
142+
return prefix;
143+
}
144+
145+
@Override
146+
public String getContextPath() {
147+
return contextPath;
148+
}
149+
};
150+
assertTrue(Path.of(env.getSourceRootPath(), relativePath).toFile().isDirectory());
151+
HttpServletResponse response = mock(HttpServletResponse.class);
152+
getFile.service(request, response);
153+
verify(response).sendRedirect(contextPath + prefix + "/");
154+
}
155+
156+
private static Stream<Prefix> getFileWritePrefixParams() {
157+
return Stream.of(Prefix.DOWNLOAD_P, Prefix.RAW_P);
158+
}
159+
160+
@ParameterizedTest
161+
@MethodSource("getFileWritePrefixParams")
162+
void testGetFileWrite(Prefix prefix) throws Exception {
163+
GetFile getFileOrig = new GetFile();
164+
ServletConfig config = mock(ServletConfig.class);
165+
getFileOrig.init(config);
166+
GetFile getFile = spy(getFileOrig);
167+
when(config.getServletContext()).thenReturn(mock(ServletContext.class));
168+
when(getFile.getServletContext().getMimeType(anyString())).thenReturn("text/plain");
169+
final String relativePath = env.getPathRelativeToSourceRoot(sourceFile.toFile());
170+
HttpServletRequest request = new DummyHttpServletRequest() {
171+
@Override
172+
public String getPathInfo() {
173+
return relativePath;
174+
}
175+
176+
@Override
177+
public String getServletPath() {
178+
return prefix.toString();
179+
}
180+
181+
@Override
182+
public long getDateHeader(String s) {
183+
return 1;
184+
}
185+
};
186+
assertTrue(Path.of(env.getSourceRootPath(), relativePath).toFile().exists());
187+
HttpServletResponse response = mock(HttpServletResponse.class);
188+
ServletOutputStream outputStream = mock(ServletOutputStream.class);
189+
when(response.getOutputStream()).thenReturn(outputStream);
190+
getFile.service(request, response);
191+
verify(outputStream).write(ArgumentMatchers.isNotNull(), eq(0), eq(fileContent.length()));
192+
verify(outputStream).close();
193+
}
194+
}

0 commit comments

Comments
 (0)