This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/ws-wss4j.git
The following commit(s) were added to refs/heads/master by this push: new 9b54ccf77 Updating APIs after pmd update 9b54ccf77 is described below commit 9b54ccf77e794f3b511f835a9ebeaa7c2ea90364 Author: Colm O hEigeartaigh <cohei...@apache.org> AuthorDate: Tue Jun 7 10:14:41 2022 +0100 Updating APIs after pmd update --- build-tools/wss4j-pmd-ruleset.xml | 123 ++++++++++++++++++--- .../integration/test/kerberos/KerberosTest.java | 6 +- .../wss4j/common/crypto/BouncyCastleUtils.java | 4 +- .../wss4j/common/crypto/CertificateStore.java | 18 +-- .../org/apache/wss4j/common/crypto/CryptoBase.java | 2 +- .../apache/wss4j/common/crypto/CryptoFactory.java | 4 +- .../org/apache/wss4j/common/crypto/Merlin.java | 18 +-- .../org/apache/wss4j/common/crypto/MerlinAKI.java | 6 +- .../wss4j/common/crypto/WSProviderConfig.java | 2 +- .../principal/WSUsernameTokenPrincipalImpl.java | 7 +- .../wss4j/common/saml/OpenSAMLBootstrap.java | 2 +- .../wss4j/common/saml/SamlAssertionWrapper.java | 11 +- .../common/saml/bean/AudienceRestrictionBean.java | 6 +- .../common/saml/bean/ProxyRestrictionBean.java | 6 +- .../common/saml/builder/SAML1ComponentBuilder.java | 1 + .../common/spnego/DefaultSpnegoClientAction.java | 2 +- .../common/spnego/DefaultSpnegoServiceAction.java | 2 +- .../apache/wss4j/common/token/BinarySecurity.java | 7 +- .../org/apache/wss4j/common/token/DOMX509Data.java | 5 +- .../apache/wss4j/common/token/PKIPathSecurity.java | 2 +- .../org/apache/wss4j/common/token/Reference.java | 5 +- .../wss4j/common/token/SecurityTokenReference.java | 12 +- .../apache/wss4j/common/util/AttachmentUtils.java | 14 +-- .../common/util/CommaDelimiterRfc2253Name.java | 2 +- .../org/apache/wss4j/common/util/KeyUtils.java | 2 +- .../wss4j/common/util/UsernameTokenUtil.java | 2 +- .../org/apache/wss4j/common/util/XMLUtils.java | 3 +- .../wss4j/common/crypto/NameConstraintsTest.java | 4 +- .../apache/wss4j/dom/str/SignatureSTRParser.java | 10 +- .../org/apache/wss4j/dom/saml/SamlTokenTest.java | 4 +- 30 files changed, 178 insertions(+), 114 deletions(-) diff --git a/build-tools/wss4j-pmd-ruleset.xml b/build-tools/wss4j-pmd-ruleset.xml index b94133a36..c98497fc3 100644 --- a/build-tools/wss4j-pmd-ruleset.xml +++ b/build-tools/wss4j-pmd-ruleset.xml @@ -17,29 +17,120 @@ specific language governing permissions and limitations under the License. --> -<ruleset name="wss4j-pmd" xmlns="http://pmd.sourceforge.net/ruleset/2.0.0" +<ruleset name="santuario-pmd" xmlns="http://pmd.sourceforge.net/ruleset/2.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd"> <description> A PMD ruleset for Apache WSS4J </description> - <rule ref="rulesets/java/basic.xml"/> - <rule ref="rulesets/java/unusedcode.xml"/> - <rule ref="rulesets/java/imports.xml"/> - <rule ref="rulesets/java/imports.xml/TooManyStaticImports"> - <properties> - <property name="maximumStaticImports" value="6"/> - </properties> + <rule ref="category/java/bestpractices.xml"> + <exclude name="AbstractClassWithoutAbstractMethod" /> + <exclude name="AccessorMethodGeneration" /> + <exclude name="ArrayIsStoredDirectly" /> + <exclude name="AvoidReassigningParameters" /> + <exclude name="AvoidReassigningLoopVariables" /> + <exclude name="AvoidUsingHardCodedIP" /> + <exclude name="AvoidMessageDigestField" /> + <exclude name="ConstantsInInterface" /> + <exclude name="ForLoopCanBeForeach" /> + <exclude name="ForLoopVariableCount" /> + <exclude name="GuardLogStatement" /> + <exclude name="JUnitAssertionsShouldIncludeMessage"/> + <exclude name="JUnitTestContainsTooManyAsserts" /> + <exclude name="JUnitTestsShouldIncludeAssert" /> + <exclude name="JUnitUseExpected" /> + <exclude name="JUnit5TestShouldBePackagePrivate" /> + <exclude name="LiteralsFirstInComparisons" /> + <exclude name="MethodReturnsInternalArray" /> + <exclude name="MissingOverride" /> + <exclude name="OneDeclarationPerLine" /> + <exclude name="PreserveStackTrace" /> + <exclude name="SwitchStmtsShouldHaveDefault" /> + <exclude name="SystemPrintln" /> + <exclude name="UnusedAssignment" /> + <exclude name="UnusedPrivateMethod" /> + <exclude name="UseAssertSameInsteadOfAssertTrue" /> + <exclude name="UseVarargs" /> </rule> - <rule ref="rulesets/java/braces.xml"/> - <rule ref="rulesets/java/empty.xml"/> - <rule ref="rulesets/java/migrating.xml"> - <exclude name="JUnit4TestShouldUseAfterAnnotation" /> - <exclude name="JUnit4TestShouldUseAfterAnnotation" /> - <exclude name="JUnit4TestShouldUseTestAnnotation" /> - <exclude name="JUnit4TestShouldUseBeforeAnnotation" /> + + <rule ref="category/java/design.xml" > + <exclude name="AvoidCatchingGenericException" /> + <exclude name="AvoidDeeplyNestedIfStmts" /> + <exclude name="AvoidRethrowingException" /> + <exclude name="AvoidThrowingNullPointerException" /> + <exclude name="AvoidThrowingRawExceptionTypes" /> + <exclude name="AvoidUncheckedExceptionsInSignatures" /> + <exclude name="CognitiveComplexity" /> + <exclude name="CouplingBetweenObjects" /> + <exclude name="CyclomaticComplexity" /> + <exclude name="DataClass" /> + <exclude name="ExceptionAsFlowControl" /> + <exclude name="ExcessiveClassLength" /> + <exclude name="ExcessiveImports" /> + <exclude name="ExcessiveMethodLength" /> + <exclude name="ExcessivePublicCount" /> + <exclude name="GodClass" /> + <exclude name="ImmutableField" /> + <exclude name="LawOfDemeter" /> + <exclude name="NcssCount" /> + <exclude name="NPathComplexity" /> + <exclude name="SignatureDeclareThrowsException" /> + <exclude name="SingularField" /> + <exclude name="SwitchDensity" /> + <exclude name="TooManyFields" /> + <exclude name="TooManyMethods" /> + <exclude name="UseObjectForClearerAPI" /> + <exclude name="UseUtilityClass" /> + </rule> + + <rule ref="category/java/errorprone.xml"> + <exclude name="AssignmentInOperand" /> + <exclude name="AssignmentToNonFinalStatic" /> + <exclude name="AvoidCatchingNPE" /> + <exclude name="AvoidCatchingThrowable" /> + <exclude name="AvoidDuplicateLiterals" /> + <exclude name="AvoidFieldNameMatchingMethodName" /> + <exclude name="AvoidFieldNameMatchingTypeName" /> + <exclude name="AvoidLiteralsInIfCondition" /> + <exclude name="BeanMembersShouldSerialize" /> + <exclude name="CompareObjectsWithEquals" /> + <exclude name="ConstructorCallsOverridableMethod" /> + <exclude name="DataflowAnomalyAnalysis" /> + <exclude name="DoNotCallGarbageCollectionExplicitly" /> + <exclude name="EmptyCatchBlock" /> + <exclude name="InvalidLogMessageFormat" /> + <exclude name="JUnitSpelling" /> + <exclude name="MissingSerialVersionUID" /> + <exclude name="NullAssignment" /> + <exclude name="SingletonClassReturningNewInstance" /> + <exclude name="SingleMethodSingleton" /> + <exclude name="SuspiciousEqualsMethodName" /> + <exclude name="UseLocaleWithCaseConversions" /> + <exclude name="UseProperClassLoader" /> + </rule> + + <rule ref="category/java/multithreading.xml"> + <exclude name="AvoidSynchronizedAtMethodLevel" /> + <exclude name="AvoidUsingVolatile" /> + <exclude name="DoNotUseThreads" /> + <exclude name="NonThreadSafeSingleton" /> + <exclude name="UseConcurrentHashMap" /> + </rule> + + <rule ref="category/java/performance.xml"> + <exclude name="AddEmptyString" /> + <exclude name="AvoidInstantiatingObjectsInLoops" /> + <exclude name="AvoidFileStream" /> + <exclude name="ConsecutiveAppendsShouldReuse" /> + <exclude name="InefficientEmptyStringCheck" /> + <exclude name="OptimizableToArrayCall" /> + <exclude name="RedundantFieldInitializer" /> + <exclude name="UseStringBufferForStringAppends" /> </rule> - <rule ref="rulesets/java/unnecessary.xml"/> + <rule ref="category/java/security.xml" > + <exclude name="HardCodedCryptoKey" /> + </rule> + </ruleset> diff --git a/integration/src/test/java/org/apache/wss4j/integration/test/kerberos/KerberosTest.java b/integration/src/test/java/org/apache/wss4j/integration/test/kerberos/KerberosTest.java index 536789af6..228db1f52 100644 --- a/integration/src/test/java/org/apache/wss4j/integration/test/kerberos/KerberosTest.java +++ b/integration/src/test/java/org/apache/wss4j/integration/test/kerberos/KerberosTest.java @@ -487,7 +487,6 @@ public class KerberosTest { builder.setCustomReferenceValue(WSConstants.WSS_GSS_KRB_V5_AP_REQ); builder.setEncKeyId(bst.getID()); - try { Document encryptedDoc = builder.build(null, secretKey); if (LOG.isDebugEnabled()) { @@ -516,9 +515,6 @@ public class KerberosTest { Principal principal = (Principal)actionResult.get(WSSecurityEngineResult.TAG_PRINCIPAL); assertTrue(principal instanceof KerberosPrincipal); assertTrue(principal.getName().contains("alice")); - } catch (Throwable t) { - t.printStackTrace(); - } } /** @@ -1281,4 +1277,4 @@ public class KerberosTest { } } -} \ No newline at end of file +} diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/BouncyCastleUtils.java b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/BouncyCastleUtils.java index 6475736fb..47556d9df 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/BouncyCastleUtils.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/BouncyCastleUtils.java @@ -39,7 +39,7 @@ public final class BouncyCastleUtils { AuthorityKeyIdentifier.getInstance(octets); return authorityKeyIdentifier.getKeyIdentifier(); } - return null; + return new byte[0]; } public static byte[] getSubjectKeyIdentifierBytes(X509Certificate cert) { @@ -51,7 +51,7 @@ public final class BouncyCastleUtils { SubjectKeyIdentifier.getInstance(subjectOctets); return subjectKeyIdentifier.getKeyIdentifier(); } - return null; + return new byte[0]; } } diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CertificateStore.java b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CertificateStore.java index d615f6d0d..024c9db52 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CertificateStore.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CertificateStore.java @@ -72,10 +72,10 @@ public class CertificateStore extends CryptoBase { */ public X509Certificate[] getX509Certificates(CryptoType cryptoType) throws WSSecurityException { if (cryptoType == null) { - return null; + return new X509Certificate[0]; } CryptoType.TYPE type = cryptoType.getType(); - X509Certificate[] certs = null; + X509Certificate[] certs = new X509Certificate[0]; switch (type) { case ISSUER_SERIAL: certs = getX509Certificates(cryptoType.getIssuer(), cryptoType.getSerial()); @@ -180,7 +180,7 @@ public class CertificateStore extends CryptoBase { // If a certificate has been found, the certificates must be compared // to ensure against phony DNs (compare encoded form including signature) // - if (foundCerts != null && foundCerts[0] != null && foundCerts[0].equals(certs[0])) { + if (foundCerts != null && foundCerts.length > 0 && foundCerts[0] != null && foundCerts[0].equals(certs[0])) { LOG.debug( "Direct trust for certificate with {}", certs[0].getSubjectX500Principal().getName() ); @@ -352,7 +352,7 @@ public class CertificateStore extends CryptoBase { } } - return null; + return new X509Certificate[0]; } /** @@ -366,7 +366,7 @@ public class CertificateStore extends CryptoBase { MessageDigest sha = null; if (trustedCerts == null) { - return null; + return new X509Certificate[0]; } try { @@ -390,7 +390,7 @@ public class CertificateStore extends CryptoBase { return new X509Certificate[]{trustedCert}; } } - return null; + return new X509Certificate[0]; } /** @@ -401,7 +401,7 @@ public class CertificateStore extends CryptoBase { */ private X509Certificate[] getX509CertificatesSKI(byte[] skiBytes) throws WSSecurityException { if (trustedCerts == null) { - return null; + return new X509Certificate[0]; } for (X509Certificate trustedCert : trustedCerts) { byte[] data = getSKIBytesFromCert(trustedCert); @@ -409,7 +409,7 @@ public class CertificateStore extends CryptoBase { return new X509Certificate[]{trustedCert}; } } - return null; + return new X509Certificate[0]; } /** @@ -448,7 +448,7 @@ public class CertificateStore extends CryptoBase { } } - return null; + return new X509Certificate[0]; } } diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CryptoBase.java b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CryptoBase.java index 64f280138..076fe193c 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CryptoBase.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CryptoBase.java @@ -393,7 +393,7 @@ public abstract class CryptoBase implements Crypto { protected byte[] getNameConstraints(final X509Certificate cert) throws WSSecurityException { byte[] bytes = cert.getExtensionValue(NAME_CONSTRAINTS_OID); if (bytes == null || bytes.length <= 0) { - return null; + return new byte[0]; } switch (bytes[0]) { diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CryptoFactory.java b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CryptoFactory.java index ec8974250..842794905 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CryptoFactory.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/CryptoFactory.java @@ -267,8 +267,7 @@ public abstract class CryptoFactory { ClassLoader loader ) throws WSSecurityException { Properties properties = new Properties(); - try { - InputStream is = Loader.loadInputStream(loader, propFilename); + try (InputStream is = Loader.loadInputStream(loader, propFilename)) { if (is == null) { throw new WSSecurityException( WSSecurityException.ErrorCode.FAILURE, @@ -277,7 +276,6 @@ public abstract class CryptoFactory { ); } properties.load(is); - is.close(); } catch (IOException e) { if (LOG.isDebugEnabled()) { LOG.debug("Cannot find resource: " + propFilename, e); diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java index 48d2b1707..396139994 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/Merlin.java @@ -556,10 +556,10 @@ public class Merlin extends CryptoBase { */ public X509Certificate[] getX509Certificates(CryptoType cryptoType) throws WSSecurityException { if (cryptoType == null) { - return null; + return new X509Certificate[0]; } CryptoType.TYPE type = cryptoType.getType(); - X509Certificate[] certs = null; + X509Certificate[] certs = new X509Certificate[0]; switch (type) { case ISSUER_SERIAL: certs = getX509Certificates(cryptoType.getIssuer(), cryptoType.getSerial()); @@ -772,7 +772,7 @@ public class Merlin extends CryptoBase { // If a certificate has been found, the certificates must be compared // to ensure against phony DNs (compare encoded form including signature) // - if (foundCerts != null && foundCerts[0] != null && foundCerts[0].equals(certs[0])) { + if (foundCerts != null && foundCerts.length > 0 && foundCerts[0] != null && foundCerts[0].equals(certs[0])) { try { certs[0].checkValidity(); } catch (CertificateExpiredException | CertificateNotYetValidException e) { @@ -984,7 +984,7 @@ public class Merlin extends CryptoBase { } if (certs == null || certs.length == 0) { - return null; + return new X509Certificate[0]; } return Arrays.copyOf(certs, certs.length, X509Certificate[].class); @@ -1072,7 +1072,7 @@ public class Merlin extends CryptoBase { } if (certs == null || certs.length == 0) { - return null; + return new X509Certificate[0]; } return Arrays.copyOf(certs, certs.length, X509Certificate[].class); @@ -1153,7 +1153,7 @@ public class Merlin extends CryptoBase { } if (certs == null || certs.length == 0) { - return null; + return new X509Certificate[0]; } return Arrays.copyOf(certs, certs.length, X509Certificate[].class); @@ -1228,7 +1228,7 @@ public class Merlin extends CryptoBase { } if (certs == null || certs.isEmpty()) { - return null; + return new X509Certificate[0]; } // We just choose the first entry @@ -1261,7 +1261,7 @@ public class Merlin extends CryptoBase { */ private X509Certificate[] getX509Certificates(String identifier) throws WSSecurityException { if (identifier == null) { - return null; + return new X509Certificate[0]; } Certificate[] certs = null; try { @@ -1289,7 +1289,7 @@ public class Merlin extends CryptoBase { } if (certs == null) { - return null; + return new X509Certificate[0]; } } catch (KeyStoreException e) { throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, e, "keystore"); diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/MerlinAKI.java b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/MerlinAKI.java index 9e7f395f8..5e77f8abc 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/MerlinAKI.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/MerlinAKI.java @@ -101,7 +101,7 @@ public class MerlinAKI extends Merlin { // If a certificate has been found, the certificates must be compared // to ensure against phony DNs (compare encoded form including signature) // - if (foundCerts != null && foundCerts[0] != null && foundCerts[0].equals(certs[0])) { + if (foundCerts != null && foundCerts.length > 0 && foundCerts[0] != null && foundCerts[0].equals(certs[0])) { try { certs[0].checkValidity(); } catch (CertificateExpiredException | CertificateNotYetValidException e) { @@ -210,7 +210,7 @@ public class MerlinAKI extends Merlin { byte[] keyIdentifierBytes ) throws WSSecurityException, NoSuchAlgorithmException, CertificateEncodingException { if (keyIdentifierBytes == null) { - return null; + return new X509Certificate[0]; } Certificate[] certs = null; @@ -224,7 +224,7 @@ public class MerlinAKI extends Merlin { } if (certs == null || certs.length == 0) { - return null; + return new X509Certificate[0]; } return Arrays.copyOf(certs, certs.length, X509Certificate[].class); diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/WSProviderConfig.java b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/WSProviderConfig.java index 91ce1c9de..fd5608e01 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/WSProviderConfig.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/crypto/WSProviderConfig.java @@ -174,7 +174,7 @@ public final class WSProviderConfig { AccessController.doPrivileged(new PrivilegedExceptionAction<Boolean>() { public Boolean run() throws Exception { Field f = XMLUtils.class.getDeclaredField("ignoreLineBreaks"); - f.setAccessible(true); + f.setAccessible(true); //NOPMD f.set(null, Boolean.TRUE); return false; } diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/principal/WSUsernameTokenPrincipalImpl.java b/ws-security-common/src/main/java/org/apache/wss4j/common/principal/WSUsernameTokenPrincipalImpl.java index 12e22721e..de08d413b 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/principal/WSUsernameTokenPrincipalImpl.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/principal/WSUsernameTokenPrincipalImpl.java @@ -202,11 +202,8 @@ public class WSUsernameTokenPrincipalImpl implements java.io.Serializable, Usern : !this.createdTime.equals(that.getCreatedTime())) { return false; } - if (this.passwordType == null ? that.getPasswordType() != null - : !this.passwordType.equals(that.getPasswordType())) { - return false; - } - return true; + return this.passwordType == null ? that.getPasswordType() == null + : this.passwordType.equals(that.getPasswordType()); } /** diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/OpenSAMLBootstrap.java b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/OpenSAMLBootstrap.java index 01e0d703d..f3858401c 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/OpenSAMLBootstrap.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/OpenSAMLBootstrap.java @@ -90,7 +90,7 @@ public final class OpenSAMLBootstrap { for (String config : XML_CONFIGS) { if (includeXacml || !config.contains("xacml")) { //most are found in the Configuration.class classloader - InputStream ins = Configuration.class.getResourceAsStream(config); + InputStream ins = Configuration.class.getResourceAsStream(config); //NOPMD if (ins == null) { //some are from us ins = OpenSAMLBootstrap.class.getResourceAsStream(config); diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/SamlAssertionWrapper.java b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/SamlAssertionWrapper.java index 0cea8bb9d..32e2fe635 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/SamlAssertionWrapper.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/SamlAssertionWrapper.java @@ -413,12 +413,9 @@ public class SamlAssertionWrapper { * @return the signed (type boolean) of this SamlAssertionWrapper object. */ public boolean isSigned() { - if (samlObject instanceof SignableSAMLObject + return samlObject instanceof SignableSAMLObject && (((SignableSAMLObject)samlObject).isSigned() - || ((SignableSAMLObject)samlObject).getSignature() != null)) { - return true; - } - return false; + || ((SignableSAMLObject)samlObject).getSignature() != null); } /** @@ -754,7 +751,7 @@ public class SamlAssertionWrapper { if (sig != null) { return getSignatureValue(sig); } - return null; + return new byte[0]; } private byte[] getSignatureValue(Signature signature) throws WSSecurityException { @@ -772,7 +769,7 @@ public class SamlAssertionWrapper { } } - return null; + return new byte[0]; } public Signature getSignature() throws WSSecurityException { diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/bean/AudienceRestrictionBean.java b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/bean/AudienceRestrictionBean.java index 5e259f560..0bf0e1ede 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/bean/AudienceRestrictionBean.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/bean/AudienceRestrictionBean.java @@ -83,11 +83,7 @@ public class AudienceRestrictionBean { AudienceRestrictionBean that = (AudienceRestrictionBean) o; - if (!audienceURIs.equals(that.audienceURIs)) { - return false; - } - - return true; + return audienceURIs.equals(that.audienceURIs); } /** diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/bean/ProxyRestrictionBean.java b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/bean/ProxyRestrictionBean.java index 7a27aaf4f..7546817c2 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/bean/ProxyRestrictionBean.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/bean/ProxyRestrictionBean.java @@ -89,11 +89,7 @@ public class ProxyRestrictionBean { return false; } - if (!audienceURIs.equals(that.audienceURIs)) { - return false; - } - - return true; + return audienceURIs.equals(that.audienceURIs); } /** diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/builder/SAML1ComponentBuilder.java b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/builder/SAML1ComponentBuilder.java index 6db7e7e88..44f24ff9e 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/builder/SAML1ComponentBuilder.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/builder/SAML1ComponentBuilder.java @@ -232,6 +232,7 @@ public final class SAML1ComponentBuilder { break; case X509_ISSUER_SERIAL: kiFactory.setEmitX509IssuerSerial(true); + break; } return kiFactory.newInstance().generate(keyInfoCredential); diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/spnego/DefaultSpnegoClientAction.java b/ws-security-common/src/main/java/org/apache/wss4j/common/spnego/DefaultSpnegoClientAction.java index 5d0b4707e..cfd324c09 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/spnego/DefaultSpnegoClientAction.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/spnego/DefaultSpnegoClientAction.java @@ -73,7 +73,7 @@ public class DefaultSpnegoClientAction implements SpnegoClientAction { LOG.debug("Error in obtaining a Kerberos token", e); } - return null; + return new byte[0]; } /** diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/spnego/DefaultSpnegoServiceAction.java b/ws-security-common/src/main/java/org/apache/wss4j/common/spnego/DefaultSpnegoServiceAction.java index edcd6e1ff..e2f5a2a59 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/spnego/DefaultSpnegoServiceAction.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/spnego/DefaultSpnegoServiceAction.java @@ -70,7 +70,7 @@ public class DefaultSpnegoServiceAction implements SpnegoServiceAction { LOG.debug("Error in obtaining a Kerberos token", e); } - return null; + return new byte[0]; } /** diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/token/BinarySecurity.java b/ws-security-common/src/main/java/org/apache/wss4j/common/token/BinarySecurity.java index c9956e200..bf720dd9e 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/token/BinarySecurity.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/token/BinarySecurity.java @@ -193,7 +193,7 @@ public class BinarySecurity { } String text = XMLUtils.getElementText(element); if (text == null) { - return null; + return new byte[0]; } return org.apache.xml.security.utils.XMLUtils.decode(text); @@ -328,10 +328,7 @@ public class BinarySecurity { return false; } String encodingType = binarySecurity.getEncodingType(); - if (!encodingType.equals(getEncodingType())) { - return false; - } - return true; + return encodingType.equals(getEncodingType()); } public boolean isStoreBytesInAttachment() { diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/token/DOMX509Data.java b/ws-security-common/src/main/java/org/apache/wss4j/common/token/DOMX509Data.java index c10e8dda2..d21f00c7b 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/token/DOMX509Data.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/token/DOMX509Data.java @@ -62,10 +62,7 @@ public final class DOMX509Data { * Return true if this X509Data element contains a X509IssuerSerial element */ public boolean containsIssuerSerial() { - if (x509IssuerSerial == null) { - return false; - } - return true; + return x509IssuerSerial != null; } /** diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/token/PKIPathSecurity.java b/ws-security-common/src/main/java/org/apache/wss4j/common/token/PKIPathSecurity.java index 0dc03f551..76073aec8 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/token/PKIPathSecurity.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/token/PKIPathSecurity.java @@ -68,7 +68,7 @@ public class PKIPathSecurity extends BinarySecurity { throws WSSecurityException { byte[] data = getToken(); if (data == null) { - return null; + return new X509Certificate[0]; } if (crypto == null) { throw new WSSecurityException(WSSecurityException.ErrorCode.FAILURE, "noSigCryptoFile"); diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/token/Reference.java b/ws-security-common/src/main/java/org/apache/wss4j/common/token/Reference.java index 21e7f8d45..77518e390 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/token/Reference.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/token/Reference.java @@ -158,10 +158,7 @@ public class Reference { if (!compare(getURI(), reference.getURI())) { return false; } - if (!compare(getValueType(), reference.getValueType())) { - return false; - } - return true; + return compare(getValueType(), reference.getValueType()); } private boolean compare(String item1, String item2) { diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/token/SecurityTokenReference.java b/ws-security-common/src/main/java/org/apache/wss4j/common/token/SecurityTokenReference.java index a1996e937..5eba043be 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/token/SecurityTokenReference.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/token/SecurityTokenReference.java @@ -315,7 +315,7 @@ public class SecurityTokenReference { */ public X509Certificate[] getKeyIdentifier(Crypto crypto) throws WSSecurityException { if (crypto == null) { - return null; + return new X509Certificate[0]; } Element elem = getFirstElement(); @@ -338,13 +338,13 @@ public class SecurityTokenReference { CryptoType cryptoType = new CryptoType(CryptoType.TYPE.THUMBPRINT_SHA1); cryptoType.setBytes(thumb); X509Certificate[] certs = crypto.getX509Certificates(cryptoType); - if (certs != null) { + if (certs != null && certs.length > 0) { return new X509Certificate[]{certs[0]}; } } } - return null; + return new X509Certificate[0]; } public String getKeyIdentifierValue() { @@ -384,7 +384,7 @@ public class SecurityTokenReference { CryptoType cryptoType = new CryptoType(CryptoType.TYPE.SKI_BYTES); cryptoType.setBytes(skiBytes); X509Certificate[] certs = crypto.getX509Certificates(cryptoType); - if (certs != null) { + if (certs != null && certs.length > 0) { return certs[0]; } return null; @@ -424,13 +424,13 @@ public class SecurityTokenReference { */ public X509Certificate[] getX509IssuerSerial(Crypto crypto) throws WSSecurityException { if (crypto == null) { - return null; + return new X509Certificate[0]; } if (issuerSerial == null) { issuerSerial = getIssuerSerial(); if (issuerSerial == null) { - return null; + return new X509Certificate[0]; } } CryptoType cryptoType = new CryptoType(CryptoType.TYPE.ISSUER_SERIAL); diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/util/AttachmentUtils.java b/ws-security-common/src/main/java/org/apache/wss4j/common/util/AttachmentUtils.java index dfa25f4d7..50febaca5 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/util/AttachmentUtils.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/util/AttachmentUtils.java @@ -563,11 +563,11 @@ public final class AttachmentUtils { Cipher cipher, boolean complete, Attachment attachment, Map<String, String> headers) throws WSSecurityException { - final InputStream attachmentInputStream; + final InputStream attachmentInputStream; //NOPMD if (complete) { - try (ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream()) { - OutputStreamWriter outputStreamWriter = new OutputStreamWriter(byteArrayOutputStream, StandardCharsets.US_ASCII); + try (ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); + OutputStreamWriter outputStreamWriter = new OutputStreamWriter(byteArrayOutputStream, StandardCharsets.US_ASCII)) { Iterator<Map.Entry<String, String>> iterator = headers.entrySet().iterator(); while (iterator.hasNext()) { @@ -606,7 +606,7 @@ public final class AttachmentUtils { } final ByteArrayInputStream ivInputStream = new ByteArrayInputStream(cipher.getIV()); - final CipherInputStream cipherInputStream = new CipherInputStream(attachmentInputStream, cipher); + final CipherInputStream cipherInputStream = new CipherInputStream(attachmentInputStream, cipher); //NOPMD return new MultiInputStream(ivInputStream, cipherInputStream); } @@ -636,9 +636,9 @@ public final class AttachmentUtils { ); } Attachment attachment = attachments.get(0); - InputStream inputStream = attachment.getSourceStream(); - - return JavaUtils.getBytesFromStream(inputStream); + try (InputStream inputStream = attachment.getSourceStream()) { + return JavaUtils.getBytesFromStream(inputStream); + } } catch (UnsupportedCallbackException | IOException e) { throw new WSSecurityException(WSSecurityException.ErrorCode.FAILED_CHECK, e); } diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/util/CommaDelimiterRfc2253Name.java b/ws-security-common/src/main/java/org/apache/wss4j/common/util/CommaDelimiterRfc2253Name.java index 9b9926d09..eafb4daba 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/util/CommaDelimiterRfc2253Name.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/util/CommaDelimiterRfc2253Name.java @@ -81,7 +81,7 @@ public class CommaDelimiterRfc2253Name { private String convertToDoubleQuotes(String rdnString) { StringBuilder quotedString = new StringBuilder(); - int indexEquals = rdnString.indexOf("="); + int indexEquals = rdnString.indexOf('='); String firstPart = rdnString.substring(0, indexEquals + 1); String lastPart = rdnString.substring(indexEquals + 1); String secondPart = unEscapeRfc2253RdnSubPart(lastPart); diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/util/KeyUtils.java b/ws-security-common/src/main/java/org/apache/wss4j/common/util/KeyUtils.java index de0805c8b..59374ace2 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/util/KeyUtils.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/util/KeyUtils.java @@ -209,7 +209,7 @@ public final class KeyUtils { new Object[]{"No such algorithm: \"" + RSA_ECB_OAEPWITH_SHA1_AND_MGF1_PADDING + "\""}); } } else { - if (e instanceof NoSuchAlgorithmException) { + if (e instanceof NoSuchAlgorithmException) { //NOPMD throw new WSSecurityException( WSSecurityException.ErrorCode.UNSUPPORTED_ALGORITHM, e, "unsupportedKeyTransp", new Object[]{"No such algorithm: \"" + keyAlgorithm + "\""}); diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/util/UsernameTokenUtil.java b/ws-security-common/src/main/java/org/apache/wss4j/common/util/UsernameTokenUtil.java index 9988278e2..c6d1861db 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/util/UsernameTokenUtil.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/util/UsernameTokenUtil.java @@ -120,7 +120,7 @@ public final class UsernameTokenUtil { saltValue = generateNonce(16); } catch (WSSecurityException ex) { LOG.debug(ex.getMessage(), ex); - return null; + return new byte[0]; } if (useForMac) { saltValue[0] = 0x01; diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/util/XMLUtils.java b/ws-security-common/src/main/java/org/apache/wss4j/common/util/XMLUtils.java index 2d43b729f..5497e9007 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/util/XMLUtils.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/util/XMLUtils.java @@ -24,6 +24,7 @@ import java.io.IOException; import java.io.OutputStream; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import javax.xml.XMLConstants; @@ -402,7 +403,7 @@ public final class XMLUtils { // lookup // if (startNode == null) { - return null; + return Collections.emptyList(); } Node startParent = startNode.getParentNode(); Node processedNode = null; diff --git a/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/NameConstraintsTest.java b/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/NameConstraintsTest.java index 6727359f2..f9b4a3b23 100644 --- a/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/NameConstraintsTest.java +++ b/ws-security-common/src/test/java/org/apache/wss4j/common/crypto/NameConstraintsTest.java @@ -129,8 +129,8 @@ public class NameConstraintsTest { Merlin merlin = new Merlin(); X509Certificate[] certificates = getTestCertificateChain(INTERMEDIATE_SIGNED); - assertNull(merlin.getNameConstraints(certificates[0])); - assertNull(merlin.getNameConstraints(certificates[1])); + assertThat(merlin.getNameConstraints(certificates[0]).length, equalTo(0)); + assertThat(merlin.getNameConstraints(certificates[1]).length, equalTo(0)); byte[] nameConstraints = merlin.getNameConstraints(certificates[2]); assertNotNull(nameConstraints); diff --git a/ws-security-dom/src/main/java/org/apache/wss4j/dom/str/SignatureSTRParser.java b/ws-security-dom/src/main/java/org/apache/wss4j/dom/str/SignatureSTRParser.java index c4e9a135c..d0752e53b 100644 --- a/ws-security-dom/src/main/java/org/apache/wss4j/dom/str/SignatureSTRParser.java +++ b/ws-security-dom/src/main/java/org/apache/wss4j/dom/str/SignatureSTRParser.java @@ -187,7 +187,7 @@ public class SignatureSTRParser implements STRParser { parserResult.setSecretKey(secretKey); } else { X509Certificate[] foundCerts = secRef.getKeyIdentifier(crypto); - if (foundCerts == null) { + if (foundCerts == null || foundCerts.length == 0) { // The reference may be to a BST in the security header rather than in the keystore if (SecurityTokenReference.SKI_URI.equals(valueType)) { byte[] skiBytes = secRef.getSKIBytes(); @@ -196,7 +196,7 @@ public class SignatureSTRParser implements STRParser { for (WSSecurityEngineResult bstResult : resultsList) { X509Certificate[] certs = (X509Certificate[])bstResult.get(WSSecurityEngineResult.TAG_X509_CERTIFICATES); - if (certs != null + if (certs != null && certs.length > 0 && Arrays.equals(skiBytes, crypto.getSKIBytesFromCert(certs[0]))) { parserResult.setPrincipal((Principal)bstResult.get(WSSecurityEngineResult.TAG_PRINCIPAL)); foundCerts = certs; @@ -210,7 +210,7 @@ public class SignatureSTRParser implements STRParser { for (WSSecurityEngineResult bstResult : resultsList) { X509Certificate[] certs = (X509Certificate[])bstResult.get(WSSecurityEngineResult.TAG_X509_CERTIFICATES); - if (certs != null) { + if (certs != null && certs.length > 0) { try { byte[] digest = KeyUtils.generateDigest(certs[0].getEncoded()); if (Arrays.equals(org.apache.xml.security.utils.XMLUtils.decode(kiValue), digest)) { @@ -227,7 +227,7 @@ public class SignatureSTRParser implements STRParser { } } } - if (foundCerts != null) { + if (foundCerts != null && foundCerts.length > 0) { parserResult.setCerts(new X509Certificate[]{foundCerts[0]}); } } @@ -309,7 +309,7 @@ public class SignatureSTRParser implements STRParser { ); } X509Certificate[] foundCerts = keyInfo.getCerts(); - if (foundCerts != null) { + if (foundCerts != null && foundCerts.length > 0) { parserResult.setCerts(new X509Certificate[]{foundCerts[0]}); } parserResult.setSecretKey(keyInfo.getSecret()); diff --git a/ws-security-dom/src/test/java/org/apache/wss4j/dom/saml/SamlTokenTest.java b/ws-security-dom/src/test/java/org/apache/wss4j/dom/saml/SamlTokenTest.java index 185eddad2..148e9dd47 100644 --- a/ws-security-dom/src/test/java/org/apache/wss4j/dom/saml/SamlTokenTest.java +++ b/ws-security-dom/src/test/java/org/apache/wss4j/dom/saml/SamlTokenTest.java @@ -132,7 +132,7 @@ public class SamlTokenTest { (SamlAssertionWrapper) actionResult.get(WSSecurityEngineResult.TAG_SAML_ASSERTION); assertNotNull(receivedSamlAssertion); assertFalse(receivedSamlAssertion.isSigned()); - assertNull(receivedSamlAssertion.getSignatureValue()); + assertTrue(receivedSamlAssertion.getSignatureValue().length == 0); } /** @@ -154,7 +154,7 @@ public class SamlTokenTest { (SamlAssertionWrapper) actionResult.get(WSSecurityEngineResult.TAG_SAML_ASSERTION); assertNotNull(receivedSamlAssertion); assertFalse(receivedSamlAssertion.isSigned()); - assertNull(receivedSamlAssertion.getSignatureValue()); + assertTrue(receivedSamlAssertion.getSignatureValue().length == 0); } /**