Skip to content

Commit

Permalink
Address SecurityContextHolder memory leak
Browse files Browse the repository at this point in the history
To get current context without creating a new context.
Creating a new context may cause ThreadLocal leak.

Closes gh-9841
  • Loading branch information
shirosaki authored and jzheaux committed Nov 30, 2021
1 parent 898ba67 commit 809ff88
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContext;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.web.context.request.RequestAttributes;
import org.springframework.web.context.request.RequestContextHolder;
Expand Down Expand Up @@ -94,7 +95,12 @@ <T> CoreSubscriber<T> createSubscriberIfNecessary(CoreSubscriber<T> delegate) {
}

private static boolean contextAttributesAvailable() {
return SecurityContextHolder.getContext().getAuthentication() != null
SecurityContext context = SecurityContextHolder.peekContext();
Authentication authentication = null;
if (context != null) {
authentication = context.getAuthentication();
}
return authentication != null
|| RequestContextHolder.getRequestAttributes() instanceof ServletRequestAttributes;
}

Expand All @@ -107,7 +113,11 @@ private static Map<Object, Object> getContextAttributes() {
servletRequest = servletRequestAttributes.getRequest();
servletResponse = servletRequestAttributes.getResponse(); // possible null
}
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
SecurityContext context = SecurityContextHolder.peekContext();
Authentication authentication = null;
if (context != null) {
authentication = context.getAuthentication();
}
if (authentication == null && servletRequest == null) {
return Collections.emptyMap();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ public SecurityContext getContext() {
return contextHolder;
}

@Override
public SecurityContext peekContext() {
return contextHolder;
}

@Override
public void setContext(SecurityContext context) {
Assert.notNull(context, "Only non-null SecurityContext instances are permitted");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ public SecurityContext getContext() {
return ctx;
}

@Override
public SecurityContext peekContext() {
return contextHolder.get();
}

@Override
public void setContext(SecurityContext context) {
Assert.notNull(context, "Only non-null SecurityContext instances are permitted");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,14 @@ public static SecurityContext getContext() {
return strategy.getContext();
}

/**
* Peeks the current <code>SecurityContext</code>.
* @return the security context (may be <code>null</code>)
*/
public static SecurityContext peekContext() {
return strategy.peekContext();
}

/**
* Primarily for troubleshooting purposes, this method shows how many times the class
* has re-initialized its <code>SecurityContextHolderStrategy</code>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ public interface SecurityContextHolderStrategy {
*/
SecurityContext getContext();

/**
* Peeks the current context without creating an empty context.
* @return a context (may be <code>null</code>)
*/
SecurityContext peekContext();

/**
* Sets the current context.
* @param context to the new argument (should never be <code>null</code>, although
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ public SecurityContext getContext() {
return ctx;
}

@Override
public SecurityContext peekContext() {
return contextHolder.get();
}

@Override
public void setContext(SecurityContext context) {
Assert.notNull(context, "Only non-null SecurityContext instances are permitted");
Expand Down

0 comments on commit 809ff88

Please sign in to comment.