Skip to content

Commit 5370f11

Browse files
rwinchrwinch
committed
Enable Null checking in spring-security-taglibs via JSpecify
Closes gh-17828
1 parent f13d8d5 commit 5370f11

File tree

8 files changed

+77
-20
lines changed

8 files changed

+77
-20
lines changed

‎taglibs/spring-security-taglibs.gradle

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
plugins {
2+
id 'apply-nullability'
3+
}
4+
15
apply plugin: 'io.spring.convention.spring-module'
26

37
dependencies {

‎taglibs/src/main/java/org/springframework/security/taglibs/authz/AbstractAuthorizeTag.java

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424
import jakarta.servlet.ServletRequest;
2525
import jakarta.servlet.ServletResponse;
2626
import jakarta.servlet.http.HttpServletRequest;
27+
import org.jspecify.annotations.NullUnmarked;
28+
import org.jspecify.annotations.Nullable;
2729

2830
import org.springframework.context.ApplicationContext;
2931
import org.springframework.core.GenericTypeResolver;
@@ -40,6 +42,7 @@
4042
import org.springframework.security.web.WebAttributes;
4143
import org.springframework.security.web.access.WebInvocationPrivilegeEvaluator;
4244
import org.springframework.security.web.context.support.SecurityWebApplicationContextUtils;
45+
import org.springframework.util.Assert;
4346
import org.springframework.util.StringUtils;
4447

4548
/**
@@ -60,11 +63,13 @@
6063
*/
6164
public abstract class AbstractAuthorizeTag {
6265

63-
private String access;
66+
@SuppressWarnings("NullAway.Init")
67+
private @Nullable String access;
6468

65-
private String url;
69+
@SuppressWarnings("NullAway.Init")
70+
private @Nullable String url;
6671

67-
private String method = "GET";
72+
private @Nullable String method = "GET";
6873

6974
/**
7075
* This method allows subclasses to provide a way to access the ServletRequest
@@ -112,14 +117,17 @@ public boolean authorize() throws IOException {
112117
* @return the result of the authorization decision
113118
* @throws IOException
114119
*/
120+
@SuppressWarnings("NullAway") // Dataflow analysis limitation
115121
public boolean authorizeUsingAccessExpression() throws IOException {
116122
if (getContext().getAuthentication() == null) {
117123
return false;
118124
}
125+
String access = getAccess();
126+
Assert.notNull(access, "access cannot be null");
119127
SecurityExpressionHandler<FilterInvocation> handler = getExpressionHandler();
120128
Expression accessExpression;
121129
try {
122-
accessExpression = handler.getExpressionParser().parseExpression(getAccess());
130+
accessExpression = handler.getExpressionParser().parseExpression(access);
123131
}
124132
catch (ParseException ex) {
125133
throw new IOException(ex);
@@ -143,32 +151,36 @@ protected EvaluationContext createExpressionEvaluationContext(SecurityExpression
143151
* @return the result of the authorization decision
144152
* @throws IOException
145153
*/
154+
@SuppressWarnings("NullAway") // Dataflow analysis limitation
146155
public boolean authorizeUsingUrlCheck() throws IOException {
156+
String url = getUrl();
157+
Assert.notNull(url, "url cannot be null");
147158
String contextPath = ((HttpServletRequest) getRequest()).getContextPath();
148159
Authentication currentUser = getContext().getAuthentication();
149-
return getPrivilegeEvaluator().isAllowed(contextPath, getUrl(), getMethod(), currentUser);
160+
return getPrivilegeEvaluator().isAllowed(contextPath, url, getMethod(), currentUser);
150161
}
151162

152-
public String getAccess() {
163+
public @Nullable String getAccess() {
153164
return this.access;
154165
}
155166

156167
public void setAccess(String access) {
157168
this.access = access;
158169
}
159170

160-
public String getUrl() {
171+
public @Nullable String getUrl() {
161172
return this.url;
162173
}
163174

164175
public void setUrl(String url) {
165176
this.url = url;
166177
}
167178

168-
public String getMethod() {
179+
public @Nullable String getMethod() {
169180
return this.method;
170181
}
171182

183+
@NullUnmarked
172184
public void setMethod(String method) {
173185
this.method = (method != null) ? method.toUpperCase(Locale.ENGLISH) : null;
174186
}

‎taglibs/src/main/java/org/springframework/security/taglibs/authz/AccessControlListTag.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.ArrayList;
2020
import java.util.List;
2121
import java.util.Map;
22+
import java.util.Objects;
2223

2324
import jakarta.servlet.ServletContext;
2425
import jakarta.servlet.jsp.JspException;
@@ -27,6 +28,7 @@
2728
import jakarta.servlet.jsp.tagext.TagSupport;
2829
import org.apache.commons.logging.Log;
2930
import org.apache.commons.logging.LogFactory;
31+
import org.jspecify.annotations.Nullable;
3032

3133
import org.springframework.context.ApplicationContext;
3234
import org.springframework.security.access.PermissionEvaluator;
@@ -60,14 +62,18 @@ public class AccessControlListTag extends TagSupport {
6062
private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder
6163
.getContextHolderStrategy();
6264

65+
@SuppressWarnings("NullAway.Init")
6366
private ApplicationContext applicationContext;
6467

68+
@SuppressWarnings("NullAway.Init")
6569
private Object domainObject;
6670

71+
@SuppressWarnings("NullAway.Init")
6772
private PermissionEvaluator permissionEvaluator;
6873

6974
private String hasPermission = "";
7075

76+
@SuppressWarnings("NullAway.Init")
7177
private String var;
7278

7379
@Override
@@ -148,7 +154,8 @@ private void initializeIfRequired() throws JspException {
148154
return;
149155
}
150156
this.applicationContext = getContext(this.pageContext);
151-
this.permissionEvaluator = getBeanOfType(PermissionEvaluator.class);
157+
this.permissionEvaluator = Objects.requireNonNull(getBeanOfType(PermissionEvaluator.class),
158+
"PermissionEvaluator Bean is required");
152159
String[] names = this.applicationContext.getBeanNamesForType(SecurityContextHolderStrategy.class);
153160
if (names.length == 1) {
154161
SecurityContextHolderStrategy strategy = this.applicationContext
@@ -157,7 +164,7 @@ private void initializeIfRequired() throws JspException {
157164
}
158165
}
159166

160-
private <T> T getBeanOfType(Class<T> type) throws JspException {
167+
private <T> @Nullable T getBeanOfType(Class<T> type) throws JspException {
161168
Map<String, T> map = this.applicationContext.getBeansOfType(type);
162169
for (ApplicationContext context = this.applicationContext.getParent(); context != null; context = context
163170
.getParent()) {

‎taglibs/src/main/java/org/springframework/security/taglibs/authz/AuthenticationTag.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import jakarta.servlet.jsp.PageContext;
2424
import jakarta.servlet.jsp.tagext.Tag;
2525
import jakarta.servlet.jsp.tagext.TagSupport;
26+
import org.jspecify.annotations.Nullable;
2627

2728
import org.springframework.beans.BeanWrapperImpl;
2829
import org.springframework.beans.BeansException;
@@ -49,9 +50,9 @@ public class AuthenticationTag extends TagSupport {
4950
private SecurityContextHolderStrategy securityContextHolderStrategy = SecurityContextHolder
5051
.getContextHolderStrategy();
5152

52-
private String var;
53+
private @Nullable String var;
5354

54-
private String property;
55+
private @Nullable String property;
5556

5657
private int scope;
5758

‎taglibs/src/main/java/org/springframework/security/taglibs/authz/JspAuthorizeTag.java

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import jakarta.servlet.jsp.JspException;
2626
import jakarta.servlet.jsp.PageContext;
2727
import jakarta.servlet.jsp.tagext.Tag;
28+
import org.jspecify.annotations.Nullable;
2829

2930
import org.springframework.expression.BeanResolver;
3031
import org.springframework.expression.ConstructorResolver;
@@ -49,13 +50,16 @@
4950
*/
5051
public class JspAuthorizeTag extends AbstractAuthorizeTag implements Tag {
5152

52-
private Tag parent;
53+
@SuppressWarnings("NullAway.Init")
54+
private @Nullable Tag parent;
5355

56+
@SuppressWarnings("NullAway.Init")
5457
protected PageContext pageContext;
5558

56-
protected String id;
59+
protected @Nullable String id;
5760

58-
private String var;
61+
@SuppressWarnings("NullAway.Init")
62+
private @Nullable String var;
5963

6064
private boolean authorized;
6165

@@ -104,7 +108,7 @@ public int doEndTag() throws JspException {
104108
return EVAL_PAGE;
105109
}
106110

107-
public String getId() {
111+
public @Nullable String getId() {
108112
return this.id;
109113
}
110114

@@ -113,7 +117,7 @@ public void setId(String id) {
113117
}
114118

115119
@Override
116-
public Tag getParent() {
120+
public @Nullable Tag getParent() {
117121
return this.parent;
118122
}
119123

@@ -122,7 +126,7 @@ public void setParent(Tag parent) {
122126
this.parent = parent;
123127
}
124128

125-
public String getVar() {
129+
public @Nullable String getVar() {
126130
return this.var;
127131
}
128132

@@ -205,12 +209,12 @@ public OperatorOverloader getOperatorOverloader() {
205209
}
206210

207211
@Override
208-
public BeanResolver getBeanResolver() {
212+
public @Nullable BeanResolver getBeanResolver() {
209213
return this.delegate.getBeanResolver();
210214
}
211215

212216
@Override
213-
public void setVariable(String name, Object value) {
217+
public void setVariable(String name, @Nullable Object value) {
214218
this.delegate.setVariable(name, value);
215219
}
216220

‎taglibs/src/main/java/org/springframework/security/taglibs/authz/package-info.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,7 @@
1717
/**
1818
* JSP Security tag library implementation.
1919
*/
20+
@NullMarked
2021
package org.springframework.security.taglibs.authz;
22+
23+
import org.jspecify.annotations.NullMarked;

‎taglibs/src/main/java/org/springframework/security/taglibs/csrf/package-info.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/*
2+
* Copyright 2004-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
/**
18+
* JSP Security tag library integration with CSRF protection.
19+
*/
20+
@NullMarked
21+
package org.springframework.security.taglibs.csrf;
22+
23+
import org.jspecify.annotations.NullMarked;

‎taglibs/src/main/java/org/springframework/security/taglibs/package-info.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,7 @@
1717
/**
1818
* Security related tag libraries that can be used in JSPs and templates.
1919
*/
20+
@NullMarked
2021
package org.springframework.security.taglibs;
22+
23+
import org.jspecify.annotations.NullMarked;

0 commit comments

Comments
 (0)