diff --git a/cas/src/main/java/org/springframework/security/cas/web/authentication/DefaultServiceAuthenticationDetails.java b/cas/src/main/java/org/springframework/security/cas/web/authentication/DefaultServiceAuthenticationDetails.java index bb5fe0d7932..682a37e2b6d 100644 --- a/cas/src/main/java/org/springframework/security/cas/web/authentication/DefaultServiceAuthenticationDetails.java +++ b/cas/src/main/java/org/springframework/security/cas/web/authentication/DefaultServiceAuthenticationDetails.java @@ -33,6 +33,7 @@ * and using the current URL minus the artifact and the corresponding value. * * @author Rob Winch + * @author Ngoc Nhan */ final class DefaultServiceAuthenticationDetails extends WebAuthenticationDetails implements ServiceAuthenticationDetails { @@ -70,14 +71,13 @@ public String getServiceUrl() { @Override public boolean equals(Object obj) { - if (this == obj) { + if (super.equals(obj)) { return true; } - if (!super.equals(obj) || !(obj instanceof DefaultServiceAuthenticationDetails)) { - return false; + if (obj instanceof DefaultServiceAuthenticationDetails that) { + return this.serviceUrl.equals(that.getServiceUrl()); } - ServiceAuthenticationDetails that = (ServiceAuthenticationDetails) obj; - return this.serviceUrl.equals(that.getServiceUrl()); + return false; } @Override @@ -100,17 +100,18 @@ public String toString() { /** * If present, removes the artifactParameterName and the corresponding value from the * query String. - * @param request + * @param request the current {@link HttpServletRequest} to obtain the + * {@link #getServiceUrl()} from. + * @param artifactPattern the {@link Pattern} that will be used to clean up the query + * string from containing the artifact name and value. This can be created using + * {@link #createArtifactPattern(String)}. * @return the query String minus the artifactParameterName and the corresponding * value. */ private String getQueryString(final HttpServletRequest request, final Pattern artifactPattern) { final String query = request.getQueryString(); - if (query == null) { - return null; - } - String result = artifactPattern.matcher(query).replaceFirst(""); - if (result.length() == 0) { + String result = (query != null) ? artifactPattern.matcher(query).replaceFirst("") : ""; + if (result.isEmpty()) { return null; } // strip off the trailing & only if the artifact was the first query param @@ -121,8 +122,9 @@ private String getQueryString(final HttpServletRequest request, final Pattern ar * Creates a {@link Pattern} that can be passed into the constructor. This allows the * {@link Pattern} to be reused for every instance of * {@link DefaultServiceAuthenticationDetails}. - * @param artifactParameterName - * @return + * @param artifactParameterName the artifactParameterName that is removed from the + * current URL. The result becomes the service url. Cannot be null or an empty String. + * @return a {@link Pattern} */ static Pattern createArtifactPattern(String artifactParameterName) { Assert.hasLength(artifactParameterName, "artifactParameterName is expected to have a length"); diff --git a/core/src/main/java/org/springframework/security/access/annotation/Jsr250MethodSecurityMetadataSource.java b/core/src/main/java/org/springframework/security/access/annotation/Jsr250MethodSecurityMetadataSource.java index fc19138609a..2e946e664d8 100644 --- a/core/src/main/java/org/springframework/security/access/annotation/Jsr250MethodSecurityMetadataSource.java +++ b/core/src/main/java/org/springframework/security/access/annotation/Jsr250MethodSecurityMetadataSource.java @@ -31,6 +31,7 @@ import org.springframework.core.annotation.AnnotationUtils; import org.springframework.security.access.ConfigAttribute; import org.springframework.security.access.method.AbstractFallbackMethodSecurityMetadataSource; +import org.springframework.util.StringUtils; /** * Sources method security metadata from major JSR 250 security annotations. @@ -108,7 +109,7 @@ private String getRoleWithDefaultPrefix(String role) { if (role == null) { return role; } - if (this.defaultRolePrefix == null || this.defaultRolePrefix.length() == 0) { + if (!StringUtils.hasLength(this.defaultRolePrefix)) { return role; } if (role.startsWith(this.defaultRolePrefix)) { diff --git a/core/src/main/java/org/springframework/security/access/expression/SecurityExpressionRoot.java b/core/src/main/java/org/springframework/security/access/expression/SecurityExpressionRoot.java index 483cdbee074..6de460382b6 100644 --- a/core/src/main/java/org/springframework/security/access/expression/SecurityExpressionRoot.java +++ b/core/src/main/java/org/springframework/security/access/expression/SecurityExpressionRoot.java @@ -31,6 +31,7 @@ import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.util.Assert; +import org.springframework.util.StringUtils; import org.springframework.util.function.SingletonSupplier; /** @@ -38,6 +39,7 @@ * * @author Luke Taylor * @author Evgeniy Cheban + * @author Ngoc Nhan * @since 3.0 */ public abstract class SecurityExpressionRoot implements SecurityExpressionOperations { @@ -167,7 +169,8 @@ public final boolean isFullyAuthenticated() { /** * Convenience method to access {@link Authentication#getPrincipal()} from * {@link #getAuthentication()} - * @return + * @return the Principal being authenticated or the authenticated + * principal after authentication. */ public @Nullable Object getPrincipal() { return getAuthentication().getPrincipal(); @@ -228,15 +231,15 @@ public void setPermissionEvaluator(PermissionEvaluator permissionEvaluator) { /** * Prefixes role with defaultRolePrefix if defaultRolePrefix is non-null and if role * does not already start with defaultRolePrefix. - * @param defaultRolePrefix - * @param role - * @return + * @param defaultRolePrefix the default prefix to add to roles. + * @param role the role that should be required. + * @return a {@code String} role */ private static String getRoleWithDefaultPrefix(@Nullable String defaultRolePrefix, String role) { if (role == null) { return role; } - if (defaultRolePrefix == null || defaultRolePrefix.length() == 0) { + if (!StringUtils.hasLength(defaultRolePrefix)) { return role; } if (role.startsWith(defaultRolePrefix)) { diff --git a/crypto/src/main/java/org/springframework/security/crypto/password/AbstractValidatingPasswordEncoder.java b/crypto/src/main/java/org/springframework/security/crypto/password/AbstractValidatingPasswordEncoder.java index 930770e4b26..09e95f68f65 100644 --- a/crypto/src/main/java/org/springframework/security/crypto/password/AbstractValidatingPasswordEncoder.java +++ b/crypto/src/main/java/org/springframework/security/crypto/password/AbstractValidatingPasswordEncoder.java @@ -18,6 +18,14 @@ import org.jspecify.annotations.Nullable; +import org.springframework.util.StringUtils; + +/** + * Implementation of PasswordEncoder. + * + * @author Rob Winch + * @since 7.0 + */ public abstract class AbstractValidatingPasswordEncoder implements PasswordEncoder { @Override @@ -32,21 +40,20 @@ public abstract class AbstractValidatingPasswordEncoder implements PasswordEncod @Override public final boolean matches(@Nullable CharSequence rawPassword, @Nullable String encodedPassword) { - if (rawPassword == null || rawPassword.length() == 0 || encodedPassword == null - || encodedPassword.length() == 0) { - return false; + if (StringUtils.hasLength(rawPassword) && StringUtils.hasLength(encodedPassword)) { + return matchesNonNull(rawPassword.toString(), encodedPassword); } - return matchesNonNull(rawPassword.toString(), encodedPassword); + return false; } protected abstract boolean matchesNonNull(String rawPassword, String encodedPassword); @Override public final boolean upgradeEncoding(@Nullable String encodedPassword) { - if (encodedPassword == null || encodedPassword.length() == 0) { - return false; + if (StringUtils.hasLength(encodedPassword)) { + return upgradeEncodingNonNull(encodedPassword); } - return upgradeEncodingNonNull(encodedPassword); + return false; } protected boolean upgradeEncodingNonNull(String encodedPassword) { diff --git a/web/src/main/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServices.java b/web/src/main/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServices.java index cefe4c08a66..387fabbb4f0 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServices.java +++ b/web/src/main/java/org/springframework/security/web/authentication/rememberme/AbstractRememberMeServices.java @@ -16,7 +16,6 @@ package org.springframework.security.web.authentication.rememberme; -import java.io.UnsupportedEncodingException; import java.net.URLDecoder; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; @@ -59,6 +58,7 @@ * @author Rob Winch * @author Eddú Meléndez * @author Onur Kagan Ozcan + * @author Ngoc Nhan * @since 2.0 */ public abstract class AbstractRememberMeServices @@ -129,7 +129,7 @@ public Authentication autoLogin(HttpServletRequest request, HttpServletResponse return null; } this.logger.debug("Remember-me cookie detected"); - if (rememberMeCookie.length() == 0) { + if (rememberMeCookie.isEmpty()) { this.logger.debug("Cookie was empty"); cancelCookie(request, response); return null; @@ -170,7 +170,7 @@ public Authentication autoLogin(HttpServletRequest request, HttpServletResponse */ protected String extractRememberMeCookie(HttpServletRequest request) { Cookie[] cookies = request.getCookies(); - if ((cookies == null) || (cookies.length == 0)) { + if (cookies == null) { return null; } for (Cookie cookie : cookies) { @@ -220,12 +220,7 @@ protected String[] decodeCookie(String cookieValue) throws InvalidCookieExceptio } String[] tokens = StringUtils.delimitedListToStringArray(cookieAsPlainText, DELIMITER); for (int i = 0; i < tokens.length; i++) { - try { - tokens[i] = URLDecoder.decode(tokens[i], StandardCharsets.UTF_8.toString()); - } - catch (UnsupportedEncodingException ex) { - this.logger.error(ex.getMessage(), ex); - } + tokens[i] = URLDecoder.decode(tokens[i], StandardCharsets.UTF_8); } return tokens; } @@ -238,12 +233,7 @@ protected String[] decodeCookie(String cookieValue) throws InvalidCookieExceptio protected String encodeCookie(String[] cookieTokens) { StringBuilder sb = new StringBuilder(); for (int i = 0; i < cookieTokens.length; i++) { - try { - sb.append(URLEncoder.encode(cookieTokens[i], StandardCharsets.UTF_8.toString())); - } - catch (UnsupportedEncodingException ex) { - this.logger.error(ex.getMessage(), ex); - } + sb.append(URLEncoder.encode(cookieTokens[i], StandardCharsets.UTF_8)); if (i < cookieTokens.length - 1) { sb.append(DELIMITER); } @@ -382,7 +372,7 @@ protected void setCookie(String[] tokens, int maxAge, HttpServletRequest request private String getCookiePath(HttpServletRequest request) { String contextPath = request.getContextPath(); - return (contextPath.length() > 0) ? contextPath : "/"; + return contextPath.isEmpty() ? "/" : contextPath; } /** diff --git a/web/src/main/java/org/springframework/security/web/firewall/RequestWrapper.java b/web/src/main/java/org/springframework/security/web/firewall/RequestWrapper.java index 1ae04a804b1..d650b8e3615 100644 --- a/web/src/main/java/org/springframework/security/web/firewall/RequestWrapper.java +++ b/web/src/main/java/org/springframework/security/web/firewall/RequestWrapper.java @@ -43,6 +43,7 @@ * bypassed by the malicious addition of parameters to the path component. * * @author Luke Taylor + * @author Ngoc Nhan */ final class RequestWrapper extends FirewalledRequest { @@ -56,7 +57,7 @@ final class RequestWrapper extends FirewalledRequest { super(request); this.strippedServletPath = strip(request.getServletPath()); String pathInfo = strip(request.getPathInfo()); - if (pathInfo != null && pathInfo.length() == 0) { + if (pathInfo != null && pathInfo.isEmpty()) { pathInfo = null; } this.strippedPathInfo = pathInfo; diff --git a/web/src/main/java/org/springframework/security/web/savedrequest/DefaultSavedRequest.java b/web/src/main/java/org/springframework/security/web/savedrequest/DefaultSavedRequest.java index 2a9c5e52d57..470a0d305fc 100644 --- a/web/src/main/java/org/springframework/security/web/savedrequest/DefaultSavedRequest.java +++ b/web/src/main/java/org/springframework/security/web/savedrequest/DefaultSavedRequest.java @@ -36,6 +36,7 @@ import org.springframework.security.web.util.UrlUtils; import org.springframework.util.Assert; import org.springframework.util.ObjectUtils; +import org.springframework.util.StringUtils; import org.springframework.web.util.UriComponentsBuilder; /** @@ -58,6 +59,7 @@ * @author Andrey Grebnev * @author Ben Alex * @author Luke Taylor + * @author Ngoc Nhan */ public class DefaultSavedRequest implements SavedRequest { @@ -206,21 +208,17 @@ private void addLocale(Locale locale) { * @since 4.2 */ private void addParameters(Map parameters) { - if (!ObjectUtils.isEmpty(parameters)) { - for (String paramName : parameters.keySet()) { - Object paramValues = parameters.get(paramName); - if (paramValues instanceof String[]) { - this.addParameter(paramName, (String[]) paramValues); - } - else { - logger.warn("ServletRequest.getParameterMap() returned non-String array"); - } - } + if (ObjectUtils.isEmpty(parameters)) { + return; } - } - private void addParameter(String name, String[] values) { - this.parameters.put(name, values); + for (Map.Entry entry : parameters.entrySet()) { + String name = entry.getKey(); + String[] values = entry.getValue(); + if (values != null) { + this.parameters.put(name, values); + } + } } /** @@ -378,7 +376,7 @@ private static String createQueryString(String queryString, String matchingReque if (matchingRequestParameterName == null) { return queryString; } - if (queryString == null || queryString.length() == 0) { + if (!StringUtils.hasLength(queryString)) { return matchingRequestParameterName; } return UriComponentsBuilder.newInstance() diff --git a/web/src/main/java/org/springframework/security/web/util/TextEscapeUtils.java b/web/src/main/java/org/springframework/security/web/util/TextEscapeUtils.java index cdca910549e..242c409e965 100644 --- a/web/src/main/java/org/springframework/security/web/util/TextEscapeUtils.java +++ b/web/src/main/java/org/springframework/security/web/util/TextEscapeUtils.java @@ -16,6 +16,8 @@ package org.springframework.security.web.util; +import org.springframework.util.StringUtils; + /** * Internal utility for escaping characters in HTML strings. * @@ -25,7 +27,7 @@ public abstract class TextEscapeUtils { public static String escapeEntities(String s) { - if (s == null || s.length() == 0) { + if (!StringUtils.hasLength(s)) { return s; } StringBuilder sb = new StringBuilder(); diff --git a/web/src/test/java/org/springframework/security/web/firewall/RequestWrapperTests.java b/web/src/test/java/org/springframework/security/web/firewall/RequestWrapperTests.java index c9294e4187e..df14f37c1c7 100644 --- a/web/src/test/java/org/springframework/security/web/firewall/RequestWrapperTests.java +++ b/web/src/test/java/org/springframework/security/web/firewall/RequestWrapperTests.java @@ -73,7 +73,7 @@ public void pathParametersAreRemovedFromPathInfo() { String path = entry.getKey(); String expectedResult = entry.getValue(); // Should be null when stripped value is empty - if (expectedResult.length() == 0) { + if (expectedResult.isEmpty()) { expectedResult = null; } request.setPathInfo(path);