Skip to content

Propagate HTTP headers in ExceptionUtil #7352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
labkey-jeckels wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Propagate HTTP headers in ExceptionUtil #7352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a2b9fe
Propagate HTTP headers in ExceptionUtil
labkey-jeckels Jan 22, 2026
e30e4b2
Fix test case
labkey-jeckels Jan 22, 2026
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
17 changes: 12 additions & 5 deletions api/src/org/labkey/api/security/AuthFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@
public class AuthFilter implements Filter
{
private static final Object FIRST_REQUEST_LOCK = new Object();

public static final String STRICT_TRANSPORT_SECURITY_HEADER_NAME = "Strict-Transport-Security";
public static final String X_FRAME_OPTIONS_HEADER_NAME = "X-Frame-Options";
public static final String X_CONTENT_TYPE_OPTIONS_HEADER_NAME = "X-Content-Type-Options";
public static final String REFERRER_POLICY_HEADER_NAME = "Referrer-Policy";
public static final String SERVER_HEADER_NAME = "Server";

private static boolean _firstRequestHandled = false;
private static volatile boolean _sslChecked = false;
private static SecurityPointcutService _securityPointcut = null;
Expand Down Expand Up @@ -81,17 +88,17 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
if (ModuleLoader.getInstance().isStartupComplete())
{
if (!"ALLOW".equals(AppProps.getInstance().getXFrameOption()))
resp.setHeader("X-Frame-Options", AppProps.getInstance().getXFrameOption());
resp.setHeader("X-Content-Type-Options", "nosniff");
resp.setHeader("Referrer-Policy", "origin-when-cross-origin" );
resp.setHeader(X_FRAME_OPTIONS_HEADER_NAME, AppProps.getInstance().getXFrameOption());
resp.setHeader(X_CONTENT_TYPE_OPTIONS_HEADER_NAME, "nosniff");
resp.setHeader(REFERRER_POLICY_HEADER_NAME, "origin-when-cross-origin" );

if (AppProps.getInstance().isIncludeServerHttpHeader())
{
if (_serverHeader == null)
{
_serverHeader = "LabKey/" + AppProps.getInstance().getReleaseVersion();
}
resp.setHeader("Server", _serverHeader);
resp.setHeader(SERVER_HEADER_NAME, _serverHeader);
}
}

Expand Down Expand Up @@ -168,7 +175,7 @@ else if (!AppProps.getInstance().isDevMode())
{
// Issue 51904: Strict-Transport-Security header when HTTPS is required
// Avoid setting when in dev mode to make it easier to toggle HTTPS on and off again for local deployments
resp.setHeader("Strict-Transport-Security", "max-age=31536000;includeSubdomains");
resp.setHeader(STRICT_TRANSPORT_SECURITY_HEADER_NAME, "max-age=31536000;includeSubdomains");
}
}

Expand Down
79 changes: 58 additions & 21 deletions api/src/org/labkey/api/util/ExceptionUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.labkey.api.miniprofiler.MiniProfiler;
import org.labkey.api.module.ModuleLoader;
import org.labkey.api.search.SearchService;
import org.labkey.api.security.AuthFilter;
import org.labkey.api.security.LoginUrls;
import org.labkey.api.security.User;
import org.labkey.api.security.UserManager;
Expand All @@ -54,6 +55,7 @@
import org.labkey.api.view.template.PageConfig;
import org.labkey.api.webdav.DavException;
import org.labkey.api.writer.PrintWriters;
import org.labkey.filters.ContentSecurityPolicyFilter;
import org.springframework.dao.DataAccessResourceFailureException;

import jakarta.servlet.ServletException;
Expand Down Expand Up @@ -594,7 +596,7 @@ else if (!local && AppProps.getInstance().isDevMode() && MothershipReport.isShow
else if (!local)
{
String hash = hashStackTrace(stackTrace);
ExceptionTally tally = EXCEPTION_TALLIES.computeIfAbsent(hash, k -> new ExceptionTally());
ExceptionTally tally = EXCEPTION_TALLIES.computeIfAbsent(hash, _ -> new ExceptionTally());
// Once we've reported an exception 5 times within this server session, don't report it more than every 5 minutes
if (tally._count.incrementAndGet() > 5 && System.currentTimeMillis() - tally._lastReported < 1000 * 60 * 5)
{
Expand Down Expand Up @@ -822,18 +824,29 @@ static ActionURL handleException(@NotNull HttpServletRequest request, @NotNull H
{
try
{
response.reset();
// mostly to make security scanners happy
if (ModuleLoader.getInstance().isStartupComplete())
// Capture security-related headers to re-apply after reset()
Map<String, String> securityHeaders = new HashMap<>();
for (String headerName : Arrays.asList(
ContentSecurityPolicyFilter.ContentSecurityPolicyType.Enforce.getHeaderName(),
ContentSecurityPolicyFilter.ContentSecurityPolicyType.Report.getHeaderName(),
AuthFilter.STRICT_TRANSPORT_SECURITY_HEADER_NAME,
AuthFilter.X_FRAME_OPTIONS_HEADER_NAME,
AuthFilter.X_CONTENT_TYPE_OPTIONS_HEADER_NAME,
AuthFilter.REFERRER_POLICY_HEADER_NAME,
AuthFilter.SERVER_HEADER_NAME))
{
if (!"ALLOW".equals(AppProps.getInstance().getXFrameOption()))
response.setHeader("X-Frame-Options", AppProps.getInstance().getXFrameOption());
response.setHeader("X-Content-Type-Options", "nosniff");
String value = response.getHeader(headerName);
if (value != null)
securityHeaders.put(headerName, value);
}

response.reset();

securityHeaders.forEach(response::setHeader);
Copy link
Contributor

@labkey-matthewb labkey-matthewb Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this test response.isCommitted()? Will it throw if setHeader() is called late?

Copy link
Contributor Author

@labkey-jeckels labkey-jeckels Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all inside of if (!response.isCommitted()). See line 823, just outside the lines shown in the diff.

}
catch (IllegalStateException x)
{
// This is fine, just can't clear the existing response as its
// This is fine, just can't clear the existing response as it's
// been at least partially written back to the client
}
}
Expand Down Expand Up @@ -1246,7 +1259,7 @@ public static boolean decorateException(Throwable t, Enum<?> key, String value,
t = unwrapException(t);
synchronized (_exceptionDecorations)
{
HashMap<Enum<?>, String> m = _exceptionDecorations.computeIfAbsent(t, k -> new HashMap<>());
HashMap<Enum<?>, String> m = _exceptionDecorations.computeIfAbsent(t, _ -> new HashMap<>());
if (overwrite || !m.containsKey(key))
{
LOG.debug("add decoration to " + t.getClass() + "@" + System.identityHashCode(t) + " " + key + "=" + value);
Expand Down Expand Up @@ -1305,15 +1318,13 @@ public static String getExtendedMessage(Throwable t)
@Nullable
public static Throwable getCause(Throwable t)
{
Throwable cause;
if (t instanceof RuntimeSQLException)
cause = ((RuntimeSQLException)t).getSQLException();
else if (t instanceof ServletException)
cause = ((ServletException)t).getRootCause();
else if (t instanceof BatchUpdateException)
cause = ((BatchUpdateException)t).getNextException();
else
cause = t.getCause();
Throwable cause = switch (t)
{
case RuntimeSQLException runtimeSQLException -> runtimeSQLException.getSQLException();
case ServletException servletException -> servletException.getRootCause();
case BatchUpdateException throwables -> throwables.getNextException();
case null, default -> t.getCause();
};
return cause==t ? null : cause;
}

Expand All @@ -1330,7 +1341,7 @@ public static class TestCase extends Assert
ExceptionResponse handleIt(final User user, Exception ex)
{
final MockServletResponse res = new MockServletResponse();
InvocationHandler h = (o, method, objects) -> {
InvocationHandler h = (_, method, objects) -> {
// still calls in 'headers' for validation
res.addHeader(method.getDeclaringClass().getSimpleName() + "." + method.getName(), objects.length==0 ? "" : objects.length==1 ? String.valueOf(objects[0]) : Arrays.toString(objects));
return null;
Expand Down Expand Up @@ -1485,9 +1496,35 @@ public void testServerError()
}

@Test
public void testUnwrap()
public void testSecurityHeaderRetention()
{
final User user = UserManager.getGuestUser();
final MockServletResponse res = new MockServletResponse();
res.setHeader(ContentSecurityPolicyFilter.ContentSecurityPolicyType.Report.getHeaderName(), "default-src 'self'");
res.setHeader(AuthFilter.STRICT_TRANSPORT_SECURITY_HEADER_NAME, "max-age=31536000");
final String otherHeader = "Some-Other-Header";
res.setHeader(otherHeader, "should-be-gone");

HttpServletRequestWrapper req = new HttpServletRequestWrapper(TestContext.get().getRequest())
{
@Override
public Principal getUserPrincipal()
{
return user;
}

@Override
public String getMethod()
{
return "GET";
}
};

ExceptionUtil.handleException(req, res, new RuntimeException("Test Exception"), null, false);

assertEquals("default-src 'self'", res.getHeader(ContentSecurityPolicyFilter.ContentSecurityPolicyType.Report.getHeaderName()));
assertEquals("max-age=31536000", res.getHeader(AuthFilter.STRICT_TRANSPORT_SECURITY_HEADER_NAME));
assertNull(res.getHeader(otherHeader));
}
}

Expand Down Expand Up @@ -1701,9 +1738,9 @@ public boolean isCommitted()
public void reset()
{
resetBuffer();
headers.clear();
status = 0;
message = null;
// headers.clear();
}

@Override
Expand Down