Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions src/main/java/org/codelibs/fess/filter/StaticThemeFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.List;
import java.util.Locale;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.stream.Collectors;
Expand All @@ -29,6 +30,7 @@
import org.codelibs.core.lang.StringUtil;
import org.codelibs.fess.helper.VirtualHostHelper;
import org.codelibs.fess.mylasta.direction.FessConfig;
import org.codelibs.fess.theme.StaticThemeInstaller;
import org.codelibs.fess.theme.StaticThemeResponder;
import org.codelibs.fess.theme.Theme;
import org.codelibs.fess.theme.ThemeRegistry;
Expand Down Expand Up @@ -62,6 +64,8 @@
*
* <p>Behavior summary:
* <ul>
* <li>A request for a JSP (or similar server-side page) under {@code /themes/} is answered
* with 404, whatever the method or active theme: a static theme contains plain files only.</li>
* <li>Requests other than GET and HEAD pass through unchanged.</li>
* <li>Requests that are neither a {@code /themes/...} asset nor an allowlisted UI path
* pass through unchanged (without even resolving the active theme).</li>
Expand Down Expand Up @@ -169,6 +173,14 @@ public void doFilter(final ServletRequest request, final ServletResponse respons
}
final HttpServletResponse res = (HttpServletResponse) response;

// A static theme is plain files. A JSP (or similar) page under /themes/ is never
// part of one, so it is not handed to the servlet container, whatever the method or
// active theme. Checked on the container-decoded path, which is what it maps to a servlet.
if (isServerSideThemePage(req)) {
res.sendError(HttpServletResponse.SC_NOT_FOUND);
return;
}

// GET and HEAD are the SPA's read paths. A HEAD response is built exactly like the GET
// one — the container drops the body and keeps the headers, which is what a HEAD client
// asks for. Everything else is a Fess route.
Expand Down Expand Up @@ -358,6 +370,21 @@ private static String stripContextPath(final HttpServletRequest req) {
return uri;
}

/**
* Returns whether the request targets a server-side page (JSP and the like) under
* {@code /themes/}. Uses the servlet path and path info, which the container has already
* decoded and normalized, rather than the raw request URI.
*
* @param req the request
* @return true when the request should not reach a servlet
*/
static boolean isServerSideThemePage(final HttpServletRequest req) {
final String servletPath = req.getServletPath() == null ? "" : req.getServletPath();
final String pathInfo = req.getPathInfo() == null ? "" : req.getPathInfo();
final String path = servletPath + pathInfo;
return path.toLowerCase(Locale.ROOT).startsWith("/themes/") && StaticThemeInstaller.isServerSidePage(path);
}

private static boolean isThemeUiPath(final String uri) {
if ("/".equals(uri)) {
return true;
Expand Down
33 changes: 33 additions & 0 deletions src/main/java/org/codelibs/fess/theme/StaticThemeInstaller.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.Comparator;
import java.util.List;
import java.util.Locale;
import java.util.Set;
import java.util.UUID;
import java.util.function.Supplier;
Expand Down Expand Up @@ -158,6 +160,33 @@ public void init() {
*/
private static final Set<String> DENIED_SEGMENTS = Set.of(".git", ".svn", ".hg", "__MACOSX", ".DS_Store");

/**
* File extensions of server-side pages the servlet container would evaluate rather than
* serve as files. A static theme is plain files only, so these are refused at install time
* and never served from {@code /themes/}.
*/
private static final List<String> SERVER_SIDE_EXTENSIONS = List.of(".jsp", ".jspx", ".jspf");

/**
* Returns whether the path names a server-side page (JSP and the like) rather than a static
* file. Compared case-insensitively.
*
* @param path a ZIP entry name or request path
* @return true when the path ends with one of the server-side page extensions
*/
public static boolean isServerSidePage(final String path) {
if (path == null) {
return false;
}
final String lower = path.toLowerCase(Locale.ROOT);
for (final String ext : SERVER_SIDE_EXTENSIONS) {
if (lower.endsWith(ext)) {
return true;
}
}
return false;
}

/** Retention duration in days for attic dirs (used when fessConfig is absent). */
private static final int DEFAULT_ATTIC_RETENTION_DAYS = 7;

Expand Down Expand Up @@ -461,6 +490,10 @@ private void extract(final InputStream in, final Path target) throws IOException
zis.closeEntry();
continue;
}
if (isServerSidePage(name)) {
throw new InstallException(InstallException.Code.EXTRACT_FAILED,
"Server-side page not allowed in a static theme: " + name);
}
final Path parent = resolved.getParent();
if (parent != null) {
Files.createDirectories(parent);
Expand Down
68 changes: 66 additions & 2 deletions src/test/java/org/codelibs/fess/filter/StaticThemeFilterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,62 @@ public void test_passesThroughForApi() throws Exception {
assertFalse(stub.servedAsset);
}

@Test
public void test_doFilter_answersServerSidePageUnderThemesWith404() throws Exception {
// A static theme is plain files, so a JSP under /themes/ must never reach the
// container's JSP servlet: not for the active theme, not for another theme, not for
// any method, and not when no static theme is active.
final Theme staticTheme = new Theme("t", Paths.get("/tmp/t"), null);
for (final Theme active : new Theme[] { staticTheme, null }) {
for (final String method : new String[] { "GET", "HEAD", "POST" }) {
for (final String path : new String[] { "/themes/t/x.jsp", "/themes/other/x.jsp", "/themes/other/sub/x.JSPX",
"/themes/other/x.jspf" }) {
final StaticThemeFilter f = new StaticThemeFilter();
f.setThemeRegistry(new StubRegistry(active));
final StubResponder stub = new StubResponder();
f.setStaticThemeResponder(stub);
final StubResponse res = new StubResponse();
final StubChain chain = new StubChain();
f.doFilter(new StubRequest(method, path), res, chain);
final String label = method + " " + path + " active=" + (active == null ? null : active.getName());
assertFalse(chain.called, label + " must not pass through to the container");
assertFalse(stub.servedAsset, label + " must not be served as an asset");
assertEquals(HttpServletResponse.SC_NOT_FOUND, res.errorStatus, label);
}
}
}
}

@Test
public void test_doFilter_matchesServerSidePageOnTheDecodedPath() throws Exception {
// The container maps the decoded path to a servlet, so the check must use it rather
// than the raw request URI (an encoded or ;param-suffixed URI decodes to x.jsp).
final StaticThemeFilter f = new StaticThemeFilter();
f.setThemeRegistry(new StubRegistry(null));
f.setStaticThemeResponder(new StubResponder());
final StubResponse res = new StubResponse();
final StubChain chain = new StubChain();
f.doFilter(new StubRequest("GET", "/%74hemes/other/x.jsp;a=b").withServletPath("/themes/other/x.jsp"), res, chain);
assertFalse(chain.called);
assertEquals(HttpServletResponse.SC_NOT_FOUND, res.errorStatus);
}

@Test
public void test_doFilter_passesThroughStaticFileOfInactiveTheme() throws Exception {
// Only server-side pages are stopped; a plain file of a theme that is not active
// still passes through to the container as before.
final Theme staticTheme = new Theme("t", Paths.get("/tmp/t"), null);
final StaticThemeFilter f = new StaticThemeFilter();
f.setThemeRegistry(new StubRegistry(staticTheme));
final StubResponder stub = new StubResponder();
f.setStaticThemeResponder(stub);
final StubResponse res = new StubResponse();
final StubChain chain = new StubChain();
f.doFilter(new StubRequest("GET", "/themes/other/app.js"), res, chain);
assertTrue(chain.called);
assertEquals(0, res.errorStatus);
}

@Test
public void test_doFilter_servesHeadLikeGet() throws Exception {
// HEAD must be served exactly like GET: same allowlist match, same serveIndex call.
Expand Down Expand Up @@ -679,6 +735,7 @@ public void doFilter(final ServletRequest req, final ServletResponse res) {
static class StubResponse implements HttpServletResponse {
// Implement the bare minimum; methods we don't call throw UnsupportedOperationException.
String redirectLocation;
int errorStatus;

@Override
public String getCharacterEncoding() {
Expand Down Expand Up @@ -787,7 +844,7 @@ public void sendError(final int sc, final String msg) {

@Override
public void sendError(final int sc) {
throw new UnsupportedOperationException();
this.errorStatus = sc;
}

@Override
Expand Down Expand Up @@ -874,12 +931,19 @@ static class StubRequest implements HttpServletRequest {
private String contextPath = "";
private String queryString;
private final Map<String, String[]> params = new HashMap<>();
private String servletPath;

StubRequest(final String method, final String uri) {
this.method = method;
this.uri = uri;
}

/** Sets the container-decoded servlet path when it differs from the raw request URI. */
StubRequest withServletPath(final String path) {
this.servletPath = path;
return this;
}

StubRequest withContextPath(final String ctx) {
this.contextPath = ctx;
return this;
Expand Down Expand Up @@ -1036,7 +1100,7 @@ public StringBuffer getRequestURL() {

@Override
public String getServletPath() {
return uri;
return servletPath != null ? servletPath : uri;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,48 @@ public void test_install_rejectsDenylistedDotfileEntries() throws Exception {
}
}

@Test
public void test_install_rejectsServerSidePages() throws Exception {
// A static theme is plain files; a JSP (or similar) entry would be evaluated by the
// servlet container instead of served, so the whole archive is refused and nothing is
// left behind in the themes directory.
for (final String entryName : new String[] { "x.jsp", "sub/x.jspx", "inc/x.jspf", "X.JSP" }) {
final Path themesDir = Files.createTempDirectory("themes-installer-");
try {
final StaticThemeInstaller installer = newInstaller(themesDir);
final ByteArrayOutputStream bao = new ByteArrayOutputStream();
try (ZipOutputStream zos = new ZipOutputStream(bao)) {
final String yml = String.join("\n", "apiVersion: fess.codelibs.org/v1", "kind: StaticTheme", "name: withpage",
"displayName: \"withpage\"", "version: 1.0.0");
putEntry(zos, "theme.yml", yml.getBytes(StandardCharsets.UTF_8));
putEntry(zos, "index.html", "<html></html>".getBytes(StandardCharsets.UTF_8));
putEntry(zos, entryName, "<%= 1 %>".getBytes(StandardCharsets.UTF_8));
}
final StaticThemeInstaller.InstallException ex = assertThrows(StaticThemeInstaller.InstallException.class,
() -> installer.installZip(new ByteArrayInputStream(bao.toByteArray())), entryName);
assertEquals(entryName, StaticThemeInstaller.InstallException.Code.EXTRACT_FAILED, ex.code());
assertTrue(ex.getMessage().contains(entryName), ex.getMessage());
assertFalse(Files.exists(themesDir.resolve("withpage")), entryName);
try (java.util.stream.Stream<Path> left = Files.list(themesDir)) {
assertEquals(entryName, 0L, left.filter(p -> p.getFileName().toString().startsWith(".staging-")).count());
}
} finally {
deleteRecursively(themesDir);
}
}
}

@Test
public void test_isServerSidePage() {
assertTrue(StaticThemeInstaller.isServerSidePage("x.jsp"));
assertTrue(StaticThemeInstaller.isServerSidePage("/themes/t/a/b.JSPX"));
assertTrue(StaticThemeInstaller.isServerSidePage("frag.jspf"));
assertFalse(StaticThemeInstaller.isServerSidePage("index.html"));
assertFalse(StaticThemeInstaller.isServerSidePage("app.js"));
assertFalse(StaticThemeInstaller.isServerSidePage("notes.jsp.txt"));
assertFalse(StaticThemeInstaller.isServerSidePage(null));
}

@Test
public void test_install_rejectsDenylistedDirectories() throws Exception {
// Denylist policy: .git, .svn, .hg, __MACOSX, .DS_Store path segments are rejected
Expand Down
Loading