diff --git a/acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java b/acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java index 01b0303d55..b314b389a9 100644 --- a/acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java +++ b/acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java @@ -31,6 +31,7 @@ import org.springframework.security.acls.model.PermissionGrantingStrategy; import org.springframework.security.acls.model.Sid; import org.springframework.security.acls.model.UnloadedSidException; import org.springframework.util.Assert; +import org.springframework.util.ObjectUtils; /** * Base implementation of Acl. @@ -285,36 +286,22 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl { @Override public boolean equals(Object obj) { - if (obj instanceof AclImpl) { - AclImpl rhs = (AclImpl) obj; - if (this.aces.equals(rhs.aces)) { - if ((this.parentAcl == null && rhs.parentAcl == null) - || (this.parentAcl != null && this.parentAcl.equals(rhs.parentAcl))) { - if ((this.objectIdentity == null && rhs.objectIdentity == null) - || (this.objectIdentity != null && this.objectIdentity.equals(rhs.objectIdentity))) { - if ((this.id == null && rhs.id == null) || (this.id != null && this.id.equals(rhs.id))) { - if ((this.owner == null && rhs.owner == null) - || (this.owner != null && this.owner.equals(rhs.owner))) { - if (this.entriesInheriting == rhs.entriesInheriting) { - if ((this.loadedSids == null && rhs.loadedSids == null)) { - return true; - } - if (this.loadedSids != null && (this.loadedSids.size() == rhs.loadedSids.size())) { - for (int i = 0; i < this.loadedSids.size(); i++) { - if (!this.loadedSids.get(i).equals(rhs.loadedSids.get(i))) { - return false; - } - } - return true; - } - } - } - } - } - } - } + if (obj == this) { + return true; } - return false; + if (obj == null || !(obj instanceof AclImpl)) { + return false; + } + AclImpl other = (AclImpl) obj; + boolean result = true; + result = result && this.aces.equals(other.aces); + result = result && ObjectUtils.nullSafeEquals(this.parentAcl, other.parentAcl); + result = result && ObjectUtils.nullSafeEquals(this.objectIdentity, other.objectIdentity); + result = result && ObjectUtils.nullSafeEquals(this.id, other.id); + result = result && ObjectUtils.nullSafeEquals(this.owner, other.owner); + result = result && this.entriesInheriting == other.entriesInheriting; + result = result && ObjectUtils.nullSafeEquals(this.loadedSids, other.loadedSids); + return result; } @Override diff --git a/config/src/main/java/org/springframework/security/config/http/HeadersBeanDefinitionParser.java b/config/src/main/java/org/springframework/security/config/http/HeadersBeanDefinitionParser.java index d3ff5e4ab1..0d24b203cb 100644 --- a/config/src/main/java/org/springframework/security/config/http/HeadersBeanDefinitionParser.java +++ b/config/src/main/java/org/springframework/security/config/http/HeadersBeanDefinitionParser.java @@ -426,80 +426,83 @@ public class HeadersBeanDefinitionParser implements BeanDefinitionParser { private void parseFrameOptionsElement(boolean addIfNotPresent, Element element, ParserContext parserContext) { BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(XFrameOptionsHeaderWriter.class); - - Element frameElt = element == null ? null : DomUtils.getChildElementByTagName(element, FRAME_OPTIONS_ELEMENT); - if (frameElt != null) { - String header = getAttribute(frameElt, ATT_POLICY, null); - boolean disabled = "true".equals(getAttribute(frameElt, ATT_DISABLED, "false")); - - if (disabled && header != null) { - this.attrNotAllowed(parserContext, ATT_DISABLED, ATT_POLICY, frameElt); - } - if (!StringUtils.hasText(header)) { - header = "DENY"; - } - - if (ALLOW_FROM.equals(header)) { - String strategyRef = getAttribute(frameElt, ATT_REF, null); - String strategy = getAttribute(frameElt, ATT_STRATEGY, null); - - if (StringUtils.hasText(strategy) && StringUtils.hasText(strategyRef)) { - parserContext.getReaderContext().error("Only one of 'strategy' or 'strategy-ref' can be set.", - frameElt); - } - else if (strategyRef != null) { - builder.addConstructorArgReference(strategyRef); - } - else if (strategy != null) { - String value = getAttribute(frameElt, ATT_VALUE, null); - if (!StringUtils.hasText(value)) { - parserContext.getReaderContext().error("Strategy requires a 'value' to be set.", frameElt); - } - // static, whitelist, regexp - if ("static".equals(strategy)) { - try { - builder.addConstructorArgValue(new StaticAllowFromStrategy(new URI(value))); - } - catch (URISyntaxException e) { - parserContext.getReaderContext().error("'value' attribute doesn't represent a valid URI.", - frameElt, e); - } - } - else { - BeanDefinitionBuilder allowFromStrategy; - if ("whitelist".equals(strategy)) { - allowFromStrategy = BeanDefinitionBuilder - .rootBeanDefinition(WhiteListedAllowFromStrategy.class); - allowFromStrategy.addConstructorArgValue(StringUtils.commaDelimitedListToSet(value)); - } - else { - allowFromStrategy = BeanDefinitionBuilder.rootBeanDefinition(RegExpAllowFromStrategy.class); - allowFromStrategy.addConstructorArgValue(value); - } - String fromParameter = getAttribute(frameElt, ATT_FROM_PARAMETER, "from"); - allowFromStrategy.addPropertyValue("allowFromParameterName", fromParameter); - builder.addConstructorArgValue(allowFromStrategy.getBeanDefinition()); - } - } - else { - parserContext.getReaderContext().error("One of 'strategy' and 'strategy-ref' must be set.", - frameElt); - } - } - else { - builder.addConstructorArgValue(header); - } - - if (disabled) { - return; + Element frameElement = (element != null) ? DomUtils.getChildElementByTagName(element, FRAME_OPTIONS_ELEMENT) + : null; + if (frameElement == null) { + if (addIfNotPresent) { + this.headerWriters.add(builder.getBeanDefinition()); } + return; } - - if (addIfNotPresent || frameElt != null) { + String header = getAttribute(frameElement, ATT_POLICY, null); + boolean disabled = "true".equals(getAttribute(frameElement, ATT_DISABLED, "false")); + if (disabled && header != null) { + this.attrNotAllowed(parserContext, ATT_DISABLED, ATT_POLICY, frameElement); + } + header = StringUtils.hasText(header) ? header : "DENY"; + if (ALLOW_FROM.equals(header)) { + parseAllowFromFrameOptionsElement(parserContext, builder, frameElement); + } + else { + builder.addConstructorArgValue(header); + } + if (!disabled) { this.headerWriters.add(builder.getBeanDefinition()); } } + private void parseAllowFromFrameOptionsElement(ParserContext parserContext, BeanDefinitionBuilder builder, + Element frameElement) { + String strategyRef = getAttribute(frameElement, ATT_REF, null); + String strategy = getAttribute(frameElement, ATT_STRATEGY, null); + if (StringUtils.hasText(strategy) && StringUtils.hasText(strategyRef)) { + parserContext.getReaderContext().error("Only one of 'strategy' or 'strategy-ref' can be set.", + frameElement); + return; + } + if (strategyRef != null) { + builder.addConstructorArgReference(strategyRef); + return; + } + if (strategy == null) { + parserContext.getReaderContext().error("One of 'strategy' and 'strategy-ref' must be set.", frameElement); + return; + } + String value = getAttribute(frameElement, ATT_VALUE, null); + if (!StringUtils.hasText(value)) { + parserContext.getReaderContext().error("Strategy requires a 'value' to be set.", frameElement); + return; + } + // static, whitelist, regexp + if ("static".equals(strategy)) { + try { + builder.addConstructorArgValue(new StaticAllowFromStrategy(new URI(value))); + } + catch (URISyntaxException e) { + parserContext.getReaderContext().error("'value' attribute doesn't represent a valid URI.", frameElement, + e); + } + return; + } + BeanDefinitionBuilder allowFromStrategy = getAllowFromStrategy(strategy, value); + String fromParameter = getAttribute(frameElement, ATT_FROM_PARAMETER, "from"); + allowFromStrategy.addPropertyValue("allowFromParameterName", fromParameter); + builder.addConstructorArgValue(allowFromStrategy.getBeanDefinition()); + } + + private BeanDefinitionBuilder getAllowFromStrategy(String strategy, String value) { + if ("whitelist".equals(strategy)) { + BeanDefinitionBuilder allowFromStrategy = BeanDefinitionBuilder + .rootBeanDefinition(WhiteListedAllowFromStrategy.class); + allowFromStrategy.addConstructorArgValue(StringUtils.commaDelimitedListToSet(value)); + return allowFromStrategy; + } + BeanDefinitionBuilder allowFromStrategy; + allowFromStrategy = BeanDefinitionBuilder.rootBeanDefinition(RegExpAllowFromStrategy.class); + allowFromStrategy.addConstructorArgValue(value); + return allowFromStrategy; + } + private void parseXssElement(boolean addIfNotPresent, Element element, ParserContext parserContext) { Element xssElt = element == null ? null : DomUtils.getChildElementByTagName(element, XSS_ELEMENT); BeanDefinitionBuilder builder = BeanDefinitionBuilder.genericBeanDefinition(XXssProtectionHeaderWriter.class); diff --git a/etc/checkstyle/checkstyle-suppressions.xml b/etc/checkstyle/checkstyle-suppressions.xml index 9235077c8b..667855b99e 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"> -