From ec6a4cb3f0c64c7880fe1b92860bc5620c573227 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 29 Jul 2020 20:03:19 -0700 Subject: [PATCH] Use consistent equals/hashCode/toString order Ensure that `equals` `hashCode` and `toString` methods always appear in the same order. This aligns with the style used in Spring Framework. Issue gh-8945 --- .../acls/domain/AbstractPermission.java | 27 +++++++--------- .../DefaultServiceAuthenticationDetails.java | 16 +++++----- .../jaas/JaasGrantedAuthority.java | 16 +++++----- .../security/util/InMemoryResource.java | 11 +++---- etc/checkstyle/checkstyle-suppressions.xml | 1 - ...icationPrincipalArgumentResolverTests.java | 16 +++++----- .../setup/SecurityMockMvcConfigurer.java | 8 ++--- .../web/access/intercept/RequestKey.java | 18 +++++------ .../SwitchUserGrantedAuthority.java | 16 +++++----- .../web/csrf/LazyCsrfTokenRepository.java | 31 +++++++++---------- .../web/util/OnCommittedResponseWrapper.java | 28 ++++++++--------- ...icationPrincipalArgumentResolverTests.java | 16 +++++----- 12 files changed, 94 insertions(+), 110 deletions(-) diff --git a/acl/src/main/java/org/springframework/security/acls/domain/AbstractPermission.java b/acl/src/main/java/org/springframework/security/acls/domain/AbstractPermission.java index e878de16e6..147f643966 100644 --- a/acl/src/main/java/org/springframework/security/acls/domain/AbstractPermission.java +++ b/acl/src/main/java/org/springframework/security/acls/domain/AbstractPermission.java @@ -52,38 +52,35 @@ public abstract class AbstractPermission implements Permission { } @Override - public final boolean equals(Object arg0) { - if (arg0 == null) { + public final boolean equals(Object obj) { + if (obj == null) { return false; } - - if (!(arg0 instanceof Permission)) { + if (!(obj instanceof Permission)) { return false; } - - Permission rhs = (Permission) arg0; - - return (this.mask == rhs.getMask()); + Permission other = (Permission) obj; + return (this.mask == other.getMask()); } @Override - public final int getMask() { + public final int hashCode() { return this.mask; } - @Override - public String getPattern() { - return AclFormattingUtils.printBinary(this.mask, this.code); - } - @Override public final String toString() { return this.getClass().getSimpleName() + "[" + getPattern() + "=" + this.mask + "]"; } @Override - public final int hashCode() { + public final int getMask() { return this.mask; } + @Override + public String getPattern() { + return AclFormattingUtils.printBinary(this.mask, this.code); + } + } 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 d16c723826..370402c26a 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 @@ -67,14 +67,6 @@ final class DefaultServiceAuthenticationDetails extends WebAuthenticationDetails return this.serviceUrl; } - @Override - public int hashCode() { - final int prime = 31; - int result = super.hashCode(); - result = prime * result + this.serviceUrl.hashCode(); - return result; - } - @Override public boolean equals(Object obj) { if (this == obj) { @@ -87,6 +79,14 @@ final class DefaultServiceAuthenticationDetails extends WebAuthenticationDetails return this.serviceUrl.equals(that.getServiceUrl()); } + @Override + public int hashCode() { + final int prime = 31; + int result = super.hashCode(); + result = prime * result + this.serviceUrl.hashCode(); + return result; + } + @Override public String toString() { StringBuilder result = new StringBuilder(); diff --git a/core/src/main/java/org/springframework/security/authentication/jaas/JaasGrantedAuthority.java b/core/src/main/java/org/springframework/security/authentication/jaas/JaasGrantedAuthority.java index 4bfadc6454..d0d76b89d1 100644 --- a/core/src/main/java/org/springframework/security/authentication/jaas/JaasGrantedAuthority.java +++ b/core/src/main/java/org/springframework/security/authentication/jaas/JaasGrantedAuthority.java @@ -53,27 +53,25 @@ public final class JaasGrantedAuthority implements GrantedAuthority { return this.role; } - @Override - public int hashCode() { - int result = this.principal.hashCode(); - result = 31 * result + this.role.hashCode(); - return result; - } - @Override public boolean equals(Object obj) { if (this == obj) { return true; } - if (obj instanceof JaasGrantedAuthority) { JaasGrantedAuthority jga = (JaasGrantedAuthority) obj; return this.role.equals(jga.role) && this.principal.equals(jga.principal); } - return false; } + @Override + public int hashCode() { + int result = this.principal.hashCode(); + result = 31 * result + this.role.hashCode(); + return result; + } + @Override public String toString() { return "Jaas Authority [" + this.role + "," + this.principal + "]"; diff --git a/core/src/main/java/org/springframework/security/util/InMemoryResource.java b/core/src/main/java/org/springframework/security/util/InMemoryResource.java index c027324f90..30b3158b6a 100644 --- a/core/src/main/java/org/springframework/security/util/InMemoryResource.java +++ b/core/src/main/java/org/springframework/security/util/InMemoryResource.java @@ -62,18 +62,17 @@ public class InMemoryResource extends AbstractResource { return new ByteArrayInputStream(this.source); } - @Override - public int hashCode() { - return 1; - } - @Override public boolean equals(Object res) { if (!(res instanceof InMemoryResource)) { return false; } - return Arrays.equals(this.source, ((InMemoryResource) res).source); } + @Override + public int hashCode() { + return 1; + } + } diff --git a/etc/checkstyle/checkstyle-suppressions.xml b/etc/checkstyle/checkstyle-suppressions.xml index ff6fb32e1b..d36fa62d5a 100644 --- a/etc/checkstyle/checkstyle-suppressions.xml +++ b/etc/checkstyle/checkstyle-suppressions.xml @@ -3,7 +3,6 @@ "-//Checkstyle//DTD SuppressionFilter Configuration 1.2//EN" "https://checkstyle.org/dtds/suppressions_1_2.dtd"> - diff --git a/messaging/src/test/java/org/springframework/security/messaging/context/AuthenticationPrincipalArgumentResolverTests.java b/messaging/src/test/java/org/springframework/security/messaging/context/AuthenticationPrincipalArgumentResolverTests.java index 565e85ccea..fe3e6c2323 100644 --- a/messaging/src/test/java/org/springframework/security/messaging/context/AuthenticationPrincipalArgumentResolverTests.java +++ b/messaging/src/test/java/org/springframework/security/messaging/context/AuthenticationPrincipalArgumentResolverTests.java @@ -275,14 +275,6 @@ public class AuthenticationPrincipalArgumentResolverTests { this.property = toCopy.property; } - @Override - public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((this.property == null) ? 0 : this.property.hashCode()); - return result; - } - @Override public boolean equals(Object obj) { if (this == obj) { @@ -306,6 +298,14 @@ public class AuthenticationPrincipalArgumentResolverTests { return true; } + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((this.property == null) ? 0 : this.property.hashCode()); + return result; + } + } } diff --git a/test/src/main/java/org/springframework/security/test/web/servlet/setup/SecurityMockMvcConfigurer.java b/test/src/main/java/org/springframework/security/test/web/servlet/setup/SecurityMockMvcConfigurer.java index d6438e5973..0a0df842c3 100644 --- a/test/src/main/java/org/springframework/security/test/web/servlet/setup/SecurityMockMvcConfigurer.java +++ b/test/src/main/java/org/springframework/security/test/web/servlet/setup/SecurityMockMvcConfigurer.java @@ -143,13 +143,13 @@ final class SecurityMockMvcConfigurer extends MockMvcConfigurerAdapter { } @Override - public int hashCode() { - return getDelegate().hashCode(); + public boolean equals(Object obj) { + return getDelegate().equals(obj); } @Override - public boolean equals(Object obj) { - return getDelegate().equals(obj); + public int hashCode() { + return getDelegate().hashCode(); } @Override diff --git a/web/src/main/java/org/springframework/security/web/access/intercept/RequestKey.java b/web/src/main/java/org/springframework/security/web/access/intercept/RequestKey.java index e7a37eb180..6779564117 100644 --- a/web/src/main/java/org/springframework/security/web/access/intercept/RequestKey.java +++ b/web/src/main/java/org/springframework/security/web/access/intercept/RequestKey.java @@ -46,32 +46,28 @@ public class RequestKey { return this.method; } - @Override - public int hashCode() { - int result = this.url.hashCode(); - result = 31 * result + (this.method != null ? this.method.hashCode() : 0); - return result; - } - @Override public boolean equals(Object obj) { if (!(obj instanceof RequestKey)) { return false; } - RequestKey key = (RequestKey) obj; - if (!this.url.equals(key.url)) { return false; } - if (this.method == null) { return key.method == null; } - return this.method.equals(key.method); } + @Override + public int hashCode() { + int result = this.url.hashCode(); + result = 31 * result + (this.method != null ? this.method.hashCode() : 0); + return result; + } + @Override public String toString() { StringBuilder sb = new StringBuilder(this.url.length() + 7); diff --git a/web/src/main/java/org/springframework/security/web/authentication/switchuser/SwitchUserGrantedAuthority.java b/web/src/main/java/org/springframework/security/web/authentication/switchuser/SwitchUserGrantedAuthority.java index 5764b1ca3b..a05155e9e0 100644 --- a/web/src/main/java/org/springframework/security/web/authentication/switchuser/SwitchUserGrantedAuthority.java +++ b/web/src/main/java/org/springframework/security/web/authentication/switchuser/SwitchUserGrantedAuthority.java @@ -59,27 +59,25 @@ public final class SwitchUserGrantedAuthority implements GrantedAuthority { return this.role; } - @Override - public int hashCode() { - int result = this.role.hashCode(); - result = 31 * result + this.source.hashCode(); - return result; - } - @Override public boolean equals(Object obj) { if (this == obj) { return true; } - if (obj instanceof SwitchUserGrantedAuthority) { SwitchUserGrantedAuthority swa = (SwitchUserGrantedAuthority) obj; return this.role.equals(swa.role) && this.source.equals(swa.source); } - return false; } + @Override + public int hashCode() { + int result = this.role.hashCode(); + result = 31 * result + this.source.hashCode(); + return result; + } + @Override public String toString() { return "Switch User Authority [" + this.role + "," + this.source + "]"; diff --git a/web/src/main/java/org/springframework/security/web/csrf/LazyCsrfTokenRepository.java b/web/src/main/java/org/springframework/security/web/csrf/LazyCsrfTokenRepository.java index c64724a8bc..fcb044a327 100644 --- a/web/src/main/java/org/springframework/security/web/csrf/LazyCsrfTokenRepository.java +++ b/web/src/main/java/org/springframework/security/web/csrf/LazyCsrfTokenRepository.java @@ -129,28 +129,12 @@ public final class LazyCsrfTokenRepository implements CsrfTokenRepository { return this.delegate.getToken(); } - @Override - public String toString() { - return "SaveOnAccessCsrfToken [delegate=" + this.delegate + "]"; - } - - @Override - public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((this.delegate == null) ? 0 : this.delegate.hashCode()); - return result; - } - @Override public boolean equals(Object obj) { if (this == obj) { return true; } - if (obj == null) { - return false; - } - if (getClass() != obj.getClass()) { + if (obj == null || getClass() != obj.getClass()) { return false; } SaveOnAccessCsrfToken other = (SaveOnAccessCsrfToken) obj; @@ -165,6 +149,19 @@ public final class LazyCsrfTokenRepository implements CsrfTokenRepository { return true; } + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((this.delegate == null) ? 0 : this.delegate.hashCode()); + return result; + } + + @Override + public String toString() { + return "SaveOnAccessCsrfToken [delegate=" + this.delegate + "]"; + } + private void saveTokenIfNecessary() { if (this.tokenRepository == null) { return; diff --git a/web/src/main/java/org/springframework/security/web/util/OnCommittedResponseWrapper.java b/web/src/main/java/org/springframework/security/web/util/OnCommittedResponseWrapper.java index aac699f03e..d35d6336a0 100644 --- a/web/src/main/java/org/springframework/security/web/util/OnCommittedResponseWrapper.java +++ b/web/src/main/java/org/springframework/security/web/util/OnCommittedResponseWrapper.java @@ -283,13 +283,13 @@ public abstract class OnCommittedResponseWrapper extends HttpServletResponseWrap } @Override - public int hashCode() { - return this.delegate.hashCode(); + public boolean equals(Object obj) { + return this.delegate.equals(obj); } @Override - public boolean equals(Object obj) { - return this.delegate.equals(obj); + public int hashCode() { + return this.delegate.hashCode(); } @Override @@ -529,16 +529,6 @@ public abstract class OnCommittedResponseWrapper extends HttpServletResponseWrap this.delegate.close(); } - @Override - public int hashCode() { - return this.delegate.hashCode(); - } - - @Override - public boolean equals(Object obj) { - return this.delegate.equals(obj); - } - @Override public void print(boolean b) throws IOException { trackContentLength(b); @@ -658,6 +648,16 @@ public abstract class OnCommittedResponseWrapper extends HttpServletResponseWrap this.delegate.setWriteListener(writeListener); } + @Override + public boolean equals(Object obj) { + return this.delegate.equals(obj); + } + + @Override + public int hashCode() { + return this.delegate.hashCode(); + } + @Override public String toString() { return getClass().getName() + "[delegate=" + this.delegate.toString() + "]"; diff --git a/web/src/test/java/org/springframework/security/web/method/annotation/AuthenticationPrincipalArgumentResolverTests.java b/web/src/test/java/org/springframework/security/web/method/annotation/AuthenticationPrincipalArgumentResolverTests.java index 630627d31c..31b97dc145 100644 --- a/web/src/test/java/org/springframework/security/web/method/annotation/AuthenticationPrincipalArgumentResolverTests.java +++ b/web/src/test/java/org/springframework/security/web/method/annotation/AuthenticationPrincipalArgumentResolverTests.java @@ -279,14 +279,6 @@ public class AuthenticationPrincipalArgumentResolverTests { this.property = toCopy.property; } - @Override - public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((this.property == null) ? 0 : this.property.hashCode()); - return result; - } - @Override public boolean equals(Object obj) { if (this == obj) { @@ -310,6 +302,14 @@ public class AuthenticationPrincipalArgumentResolverTests { return true; } + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((this.property == null) ? 0 : this.property.hashCode()); + return result; + } + } }